Throw an error message when Scala 2.12.4 is used

[lefou]

We have various test cases in Mill, which fail when we update zinc version from 1.5.7 to 1.5.8.

steps

On example reproducer is:

• checkout mill repository and bump zinc to 1.5.8

diff --git a/build.sc b/build.sc
index c415e742..a9b29871 100755
--- a/build.sc
+++ b/build.sc
@@ -121,7 +121,7 @@ object Deps {
   val upickle = ivy"com.lihaoyi::upickle:1.4.2"
   val utest = ivy"com.lihaoyi::utest:0.7.10"
   val windowsAnsi = ivy"io.github.alexarchambault.windows-ansi:windows-ansi:0.0.3"
-  val zinc = ivy"org.scala-sbt::zinc:1.5.7"
+  val zinc = ivy"org.scala-sbt::zinc:1.5.8"
   val bsp = ivy"ch.epfl.scala:bsp4j:2.0.0"
   val fansi = ivy"com.lihaoyi::fansi:0.2.14"
   val jarjarabrams = ivy"com.eed3si9n.jarjarabrams::jarjar-abrams-core:1.8.0"

• or alternatively, checkout this scala-steward-branch: https://github.com/scala-steward/mill/tree/update/zinc-1.5.8

• run this test suite

mill contrib.buildinfo.test

problem

Compilation with zinc aborts with an java.lang.StackOverflowError.

  java.lang.StackOverflowError
    xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.handleClassicTreeNode(ExtractUsedNames.scala:251)
    xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.traverse(ExtractUsedNames.scala:170)
    xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3(ExtractUsedNames.scala:294)
    xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3$adapted(ExtractUsedNames.scala:294)
    xsbt.Compat$OriginalTreeTraverser$Reflective$.traverseOriginal(Compat.scala:49)
    xsbt.Compat.processOriginalTreeAttachment(Compat.scala:25)
    xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.handleClassicTreeNode(ExtractUsedNames.scala:294)
    xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.traverse(ExtractUsedNames.scala:170)
    xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3(ExtractUsedNames.scala:294)
    xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3$adapted(ExtractUsedNames.scala:294)
    xsbt.Compat$OriginalTreeTraverser$Reflective$.traverseOriginal(Compat.scala:49)
    xsbt.Compat.processOriginalTreeAttachment(Compat.scala:25)
...

expectation

Compilation succeedes as it did with older zinc versions.

notes

I choosen this test as it is fairly minimal. The effective mill build simulated in this test case would basically look like this:

  object BuildInfo extends BuildInfoModule {
    override def scalaVersion = "2.12.4"
    override def buildInfoMembers = T {
      Map(
        "scalaVersion" -> scalaVersion()
      )
    }
  }

It seems, this failure is sensitive to the target scala version. e.g. it fails for: 2.12.4

But from some quick experiments, it seems to succeed for many other scala versions, e.g.: 2.12.3, 2.12.5, 2.12.7, 2.13.7

The code probably causing the StackOverflowError, xsbt.Compat.OriginalTreeTraverser, was newly added in zinc 1.5.8 (v1.5.7...v1.5.8). My wild guess is, that it was never tested with Scala 2.12.4 and the issue is only present for this exact Scala version.

This is a (soft) blocker for updating zinc in Mill. See PR com-lihaoyi/mill#1613

[eed3si9n]

Thanks for the report.

@dwijnand Looks like Zinc 1.5.8 shipped #997, and caught some infinite loop of walking the "original tree". I am guessing that there's some node where the original tree points to itself or something.

Also stack trace shows macros related:

	at scala.reflect.macros.Attachments.matchesTag(Attachments.scala:38)
	at scala.reflect.macros.Attachments.$anonfun$get$1(Attachments.scala:42)
	at scala.reflect.macros.Attachments.$anonfun$get$1$adapted(Attachments.scala:42)
	at scala.collection.immutable.Set$Set1.find(Set.scala:104)
	at scala.reflect.macros.Attachments.get(Attachments.scala:42)
	at xsbt.Compat$OriginalTreeTraverser$Reflective$.traverseOriginal(Compat.scala:48)
	at xsbt.Compat.processOriginalTreeAttachment(Compat.scala:25)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.handleClassicTreeNode(ExtractUsedNames.scala:294)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.traverse(ExtractUsedNames.scala:170)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3(ExtractUsedNames.scala:294)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3$adapted(ExtractUsedNames.scala:294)
	at xsbt.Compat$OriginalTreeTraverser$Reflective$.traverseOriginal(Compat.scala:49)
	at xsbt.Compat.processOriginalTreeAttachment(Compat.scala:25)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.handleClassicTreeNode(ExtractUsedNames.scala:294)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.traverse(ExtractUsedNames.scala:170)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3(ExtractUsedNames.scala:294)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3$adapted(ExtractUsedNames.scala:294)
	at xsbt.Compat$OriginalTreeTraverser$Reflective$.traverseOriginal(Compat.scala:49)
	at xsbt.Compat.processOriginalTreeAttachment(Compat.scala:25)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.handleClassicTreeNode(ExtractUsedNames.scala:294)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.traverse(ExtractUsedNames.scala:170)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3(ExtractUsedNames.scala:294)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3$adapted(ExtractUsedNames.scala:294)
	at xsbt.Compat$OriginalTreeTraverser$Reflective$.traverseOriginal(Compat.scala:49)
	at xsbt.Compat.processOriginalTreeAttachment(Compat.scala:25)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.handleClassicTreeNode(ExtractUsedNames.scala:294)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.traverse(ExtractUsedNames.scala:170)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3(ExtractUsedNames.scala:294)
	at xsbt.ExtractUsedNames$ExtractUsedNamesTraverser.$anonfun$handleClassicTreeNode$3$adapted(ExtractUsedNames.scala:294)
	at xsbt.Compat$OriginalTreeTraverser$Reflective$.traverseOriginal(Compat.scala:49)
	at xsbt.Compat.processOriginalTreeAttachment(Compat.scala:25)

This could mean that original tree might be used for multiple purposes (constant folding, macros etc) and testing with one scenario might work but doesn't for the other? Also if previous version of Zinc didn't have this issue, maybe it's a problem specific to 2.12?

[dwijnand]

Here's my hypothesis: OriginalTreeAttachment was added in 2.12.3, something was changed in 2.12.4 which introduced some cycle, which was either removed or guarded against in 2.12.5+ as well as 2.13+. 2.12.4 is Oct 2017 and 2.12.5 is Mar 2018, so this seems pretty low priority to me. But apologies for introducing the bug!

[lefou]

Sound reasonable. How about guarding that Scala version with a better error message, indicating that simply selecting some other version might fix the issue? I don't need this fix for me; but want to provide a nice experience for the users of Mill and zinc. If we claim to compile Scala 2.12 code but we know that certain versions may not work, we could at least communicate that to the user and avoid lots of frustrations.

[eed3si9n]

https://github.com/sbt/zinc/releases/tag/v1.5.9 is out without #997.

[dwijnand]

Why?

[eed3si9n]

I didn't want to create a Trolley problem of log4shell and unpredictable (works only on some 2.12.x?) stackoverflow in 1.5.x branch.

If sbt 1.6.0 eventually finalizes this would still be part of 1.6.x.

[dwijnand]

You still have a trolley problem because you just introduced under-compilation involving constants for all versions of scala 2.12.x just because it (sometimes?) stackoverflows for scala 2.12.4. Which is the wrong choice IMO.

[Friendseeker]

Referring to sjrd's comment

Attempting to compile any project using Scala 2.12.4 and sbt
1.6.0+, including a Hello World, results in a StackOverflow.
Therefore, no one must be using Scala 2.12.4 at this point.

[eed3si9n]

We should probably throw an error message from Zinc saying the version not supported.

[Friendseeker]

Note to myself: Probably want to add new type of exception specifically for unsupported compile setup.

Can throw same exception for #1352