zinc icon indicating copy to clipboard operation
zinc copied to clipboard

Throw an error message when Scala 2.12.4 is used

Open lefou opened this issue 3 years ago • 10 comments

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 (https://github.com/sbt/zinc/compare/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 https://github.com/com-lihaoyi/mill/pull/1613

lefou avatar Dec 12 '21 21:12 lefou

Thanks for the report.

@dwijnand Looks like Zinc 1.5.8 shipped https://github.com/sbt/zinc/pull/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?

eed3si9n avatar Dec 13 '21 20:12 eed3si9n

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!

dwijnand avatar Dec 14 '21 10:12 dwijnand

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.

lefou avatar Dec 14 '21 10:12 lefou

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

eed3si9n avatar Dec 15 '21 15:12 eed3si9n

Why?

dwijnand avatar Dec 15 '21 16:12 dwijnand

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.

eed3si9n avatar Dec 15 '21 16:12 eed3si9n

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.

dwijnand avatar Dec 15 '21 21:12 dwijnand

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.

Friendseeker avatar Dec 24 '23 00:12 Friendseeker