Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix inductive-implicits benchmark #22007

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mbovel
Copy link
Member

@mbovel mbovel commented Nov 22, 2024

The inductive-implicits benchmark fails on the benchmarks machine since April 2024:

image

This should have been caught by the tests at:

compileFilesInDir("tests/bench", defaultOptions.without("-Yno-deep-subtypes")),

When I run scala3-bootstrapped / testOnly dotty.tools.dotc.BootstrappedOnlyCompilationTests -- *posMacros locally, the tests are indeed failing and the following error is printed:

64 |  val sel = Selector[L, Boolean]
   |                                ^
   |No given instance of type shapeless.Selector[Test.L, Boolean] was found for parameter selector of method apply in object Selector.
   |I found:
   |
   |    shapeless.Selector.inTail[Int,
   |      Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |         Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int
   |         :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |        Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int
   |        :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |        Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: ...Int :: ... :: ... :: ...,
   |    Boolean](
   |      shapeless.Selector.inTail[Int,
   |        Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int
   |          :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |           Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |          Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |          Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |          Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: ...Int :: ... :: ... :: ...,
   |      Boolean](
   |        shapeless.Selector.inTail[Int,
   |          Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |            Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |            Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |            Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |            Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |            Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: ...Int :: ... :: ... :: ...,
   |        Boolean](
 
...

   |            )
   |          )
   |        )
   |      )
   |    )
   |
   |But method inTail in object Selector does not match type shapeless.Selector[
   |  Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int :: Int ::
   |     Boolean :: shapeless.HNil,
   |Boolean].
   |
   |Note: method inTail in object Selector was not considered because it was not imported with `import given`.

It is not clear to me why the test does not fail on the CI. I checked that dotty.tools.dotc.BootstrappedOnlyCompilationTests.posMacros indeed runs there, and I confirm it is the case.

Anyway, the fix is simple, we just need to import the implicits explicitly.

This PR additionally removes the duplicate tests/pos-deep-subtype/inductive-implicits-bench.scala.

@mbovel mbovel requested a review from hamzaremmal November 22, 2024 11:56
This is a duplicate of `tests/bench/inductive-implicits.scala`, which is already tested from `dotty.tools.dotc.BootstrappedOnlyCompilationTest`, also without `-Yno-deep-subtypes`.
@mbovel mbovel force-pushed the mb/benchs/fix-inductive-implicits branch from 1a05f7f to ceb7d70 Compare November 25, 2024 12:24
@mbovel
Copy link
Member Author

mbovel commented Nov 25, 2024

After having a deeper look with @hamzaremmal, we found that importing the given is actually not needed; the last line of the error message is incorrect. However, importing explicitly the given apparently diminishes the required stack size, which is why it made the test pass on my machine. This also explains why the test was not failing on the CI: the stack size was sufficient there.

I pushed a new fix which comments out some more lines of the summoned implicit argument type.

@mbovel
Copy link
Member Author

mbovel commented Nov 25, 2024

For the record, here is another error I got for the same benchmarks while working on another branch, with the import in the object:

object Test extends App {
  import Selector.given
  val sel = Selector[L, Boolean]
  ...
  Exception while compiling tests/bench/inductive-implicits.scala

  An unhandled exception was thrown in the compiler.
  Please file a crash report here:
  https://github.com/scala/scala3/issues/new/choose
  For non-enriched exceptions, compile with -Xno-enrich-error-messages.


     while compiling: <no file>pped / Test / testOnly 11s
        during phase: parser
                mode: Mode()
     library version: version 2.13.15
    compiler version: version 3.6.3-RC1-bin-SNAPSHOT-git-924ea08
            settings: -Xsemanticdb true -Xunchecked-java-output-version 9 -Xverify-signatures true -Ycheck List(all) -Yforce-sbt-phases true -Yno-double-bindings true -classpath /Users/mbovel/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.15/scala-library-2.13.15.jar:/Users/mbovel/dotty/library/../out/bootstrap/scala3-library-bootstrapped/scala-3.6.3-RC1-bin-SNAPSHOT-nonbootstrapped/scala3-library_3-3.6.3-RC1-bin-SNAPSHOT.jar:out/compilePosMacros/bench/inductive-implicits -color never -d out/compilePosMacros/bench/inductive-implicits -indent true -pagewidth 120

java.lang.StackOverflowError
        at dotty.tools.dotc.core.Scopes$MutableScope.enterAllInHash(Scopes.scala:310)
        at dotty.tools.dotc.core.Scopes$MutableScope.enterAllInHash(Scopes.scala:310)
        at dotty.tools.dotc.core.Scopes$MutableScope.enterAllInHash(Scopes.scala:310)
        at dotty.tools.dotc.core.Scopes$MutableScope.enterAllInHash(Scopes.scala:310)
        at dotty.tools.dotc.core.Scopes$MutableScope.enterAllInHash(Scopes.scala:310)
        at dotty.tools.dotc.core.Scopes$MutableScope.enterAllInHash(Scopes.scala:310)
        ...

Here the stack overflow error is not caught and crashes the compiler.

To avoid reaching the stack limit.
@mbovel mbovel force-pushed the mb/benchs/fix-inductive-implicits branch from ceb7d70 to d3b9d76 Compare November 25, 2024 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant