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.

Sign up for GitHub

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

Jump to bottom

summonInline in a function in a quoted block causes compiler crash #19436

Closed
KuceraMartin opened this issue Jan 12, 2024 · 5 comments · Fixed by #20837
Closed

summonInline in a function in a quoted block causes compiler crash #19436

KuceraMartin opened this issue Jan 12, 2024 · 5 comments · Fixed by #20837
Assignees
@odersky @EugeneFlesselle
Labels
area:metaprogramming:quotes Issues related to quotes and splices area:pickling itype:bug itype:crash
Milestone
3.5.1

Comments

@KuceraMartin
Copy link
Contributor

Compiler version

3.3.1, 3.4.0-RC1-bin-20240109-91db06a-NIGHTLY-git-91db06a

Minimized code

// main.scala

trait SomeImplicits:
  given int: Int

def fn: Unit =
  Macro.testSummon
// macro.scala

import scala.quoted.*
import scala.compiletime.summonInline

object Macro:

  transparent inline def testSummon = ${ testSummonImpl }

  private def testSummonImpl(using Quotes): Expr[SomeImplicits => Int] =
    import quotes.reflect.*
    '{
      (x: SomeImplicits) =>
        import x.given
        summonInline[Int]
    }

Output (click arrow to expand)

  unhandled exception while running pickler on /Users/martin/development/macro-test/main.scala

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

     while compiling: /Users/martin/development/macro-test/main.scala
        during phase: pickler
                mode: Mode(ImplicitsEnabled)
     library version: version 2.13.12
    compiler version: version 3.4.0-RC1-bin-20240109-91db06a-NIGHTLY-git-91db06a
            settings: -classpath /Users/martin/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala3-library_3/3.4.0-RC1-bin-20240109-91db06a-NIGHTLY/scala3-library_3-3.4.0-RC1-bin-20240109-91db06a-NIGHTLY.jar:/Users/martin/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.12/scala-library-2.13.12.jar -d /Users/martin/development/macro-test/.scala-build/macro-test_3200b05eac-8a7bba9ae5/classes/main -java-output-version 17 -sourceroot /Users/martin/development/macro-test


  Exception while compiling /Users/martin/development/macro-test/main.scala, /Users/martin/development/macro-test/macro.scala

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

     while compiling: <no file>
        during phase: parser
                mode: Mode()
     library version: version 2.13.12
    compiler version: version 3.4.0-RC1-bin-20240109-91db06a-NIGHTLY-git-91db06a
            settings: -classpath /Users/martin/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala3-library_3/3.4.0-RC1-bin-20240109-91db06a-NIGHTLY/scala3-library_3-3.4.0-RC1-bin-20240109-91db06a-NIGHTLY.jar:/Users/martin/Library/Caches/Coursier/v1/https/repo1.maven.org/maven2/org/scala-lang/scala-library/2.13.12/scala-library-2.13.12.jar -d /Users/martin/development/macro-test/.scala-build/macro-test_3200b05eac-8a7bba9ae5/classes/main -java-output-version 17 -sourceroot /Users/martin/development/macro-test

Exception in thread "main" java.lang.AssertionError: assertion failed: unresolved symbols: parameter x (line 10) #32164 when pickling /Users/martin/development/macro-test/main.scala
	at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:8)
	at dotty.tools.dotc.core.tasty.TreePickler.pickle(TreePickler.scala:863)
	at dotty.tools.dotc.transform.Pickler.run$$anonfun$1$$anonfun$1(Pickler.scala:125)
	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.immutable.List.foreach(List.scala:333)
	at dotty.tools.dotc.transform.Pickler.run$$anonfun$1(Pickler.scala:180)
	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.immutable.List.foreach(List.scala:333)
	at dotty.tools.dotc.transform.Pickler.run(Pickler.scala:180)
	at dotty.tools.dotc.core.Phases$Phase.runOn$$anonfun$1(Phases.scala:354)
	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.immutable.List.foreach(List.scala:333)
	at dotty.tools.dotc.core.Phases$Phase.runOn(Phases.scala:360)
	at dotty.tools.dotc.transform.Pickler.runOn(Pickler.scala:197)
	at dotty.tools.dotc.Run.runPhases$1$$anonfun$1(Run.scala:315)
	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:15)
	at scala.runtime.function.JProcedure1.apply(JProcedure1.java:10)
	at scala.collection.ArrayOps$.foreach$extension(ArrayOps.scala:1323)
	at dotty.tools.dotc.Run.runPhases$1(Run.scala:337)
	at dotty.tools.dotc.Run.compileUnits$$anonfun$1(Run.scala:348)
	at dotty.tools.dotc.Run.compileUnits$$anonfun$adapted$1(Run.scala:357)
	at dotty.tools.dotc.util.Stats$.maybeMonitored(Stats.scala:69)
	at dotty.tools.dotc.Run.compileUnits(Run.scala:357)
	at dotty.tools.dotc.Run.compileUnits(Run.scala:267)
	at dotty.tools.dotc.Driver.finish(Driver.scala:58)
	at dotty.tools.dotc.Driver.doCompile(Driver.scala:38)
	at dotty.tools.dotc.Driver.process(Driver.scala:197)
	at dotty.tools.dotc.Driver.process(Driver.scala:165)
	at dotty.tools.dotc.Driver.process(Driver.scala:177)
	at dotty.tools.dotc.Driver.main(Driver.scala:207)
	at dotty.tools.dotc.Main.main(Main.scala)
Compilation failed
@KuceraMartin KuceraMartin added itype:bug itype:crash stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 12, 2024
@jchyb jchyb added area:pickling area:metaprogramming:quotes Issues related to quotes and splices and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Jan 15, 2024
This was referenced Jan 19, 2024
Closed
Closed
@mbovel
Copy link
Member

mbovel commented Mar 15, 2024

This issue was picked for the Issue Spree of March 19th, 2024. @EugeneFlesselle, @KuceraMartin, @nmcb will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here.

@nicolasstucki
Copy link
Contributor

Issue might be the same as in #19493

@lihaoyi
Copy link
Contributor

lihaoyi commented May 13, 2024

@Gedochao @EugeneFlesselle any updates here? This is blocking https://github.com/com-lihaoyi/scalasql from supporting Scala 3, which in future would block a number of experiments around using named tuples etc. to model database queries and the like

@EugeneFlesselle
Copy link
Contributor

any updates here?

I'm still working on it -- and am pretty familiar with was is going wrong -- but I am yet to find a way to fix it.

This is blocking https://github.com/com-lihaoyi/scalasql from supporting Scala 3

Maybe this is obvious but just in case, the situation that fails is one where summonInline isn't needed, so it could be replaced with a normal summon; unless it is necessary for it to be able to resolve an implicit from both the ctx of the caller and the ctx of the body, depending on usages.

@lihaoyi
Copy link
Contributor

lihaoyi commented May 21, 2024

Thanks @EugeneFlesselle ! As long as you're working on it that's fine, just worried it got dropped.

As far as I know, we needed summonInline in order to resolve implicits from the context of the caller

@mrdziuban mrdziuban mentioned this issue May 23, 2024
Merged
lihaoyi pushed a commit to com-lihaoyi/scalasql that referenced this issue May 25, 2024
@mrdziuban
Scala 3 support (#11)
7cbc5cc 
Closes #2 

## Macros

Just like @KuceraMartin in #3, I ran into scala/scala3#19493 and
scala/scala3#19436 when trying to resolve a `TypeMapper` by importing
from a `DialectTypeMappers`. As a workaround, I introduced [additional
`implicit def`s in the `TableMapper` companion
object](https://github.com/com-lihaoyi/scalasql/blob/a7d6c531bf7b9cc2f5e2c175906d2a1e961de206/scalasql/core/src/TypeMapper.scala#L58-L121)
that instead rely on an implicit instance of `DialectTypeMappers`, i.e.
in a macro:

```scala
// bad, causes a compiler crash
// TableMacro.scala
(dialect: DialectTypeMappers) => {
  import dialect.*
  summonInline[TypeMapper[t]]
}

// good
// TypeMapper.scala
implicit def stringFromDialectTypeMappers(implicit d: DialectTypeMappers): TypeMapper[String] = d.StringType

// TableMacro.scala
(dialect: DialectTypeMappers) => {
  given d: DialectTypeMappers = dialect
  summonInline[TypeMapper[t]]
}
```

## Supporting changes

In addition to building out the macros in Scala 3, the following changes
were necessary:

1. Update the generated code to ensure `def`s aren't too far to the left
-- this is to silence Scala 3 warnings
2. Convert `CharSequence`s to `String`s explicitly -- see the [error the
Scala 3 compiler reported
here](9ffeb06)
3. Remove `try` block without a corresponding `catch` -- see the
[warning the Scala 3 compiler reported
here](011c3f6)
4. Add types to implicit definitions
5. Mark `renderSql` as `private[scalasql]` instead of `protected` -- see
the [error the Scala 3 compiler reported
here](8e767e3)
6. Use Scala 3.4 -- this is a little unfortunate since it's not the LTS
but it's necessary for the Scala 3 macros to [match on higher kinded
types like
this](https://github.com/com-lihaoyi/scalasql/blob/a7d6c531bf7b9cc2f5e2c175906d2a1e961de206/scalasql/query/src-3/TableMacro.scala#L48-L52).
This type of match doesn't work in Scala 3.3
7. Replace `_` wildcards with `?` -- this is to silence Scala 3 warnings
8. Replace `Foo with Bar` in types with `Foo & Bar` -- this is to
silence Scala 3 warnings
9. Add the `-Xsource:3` compiler option for Scala 2 -- this is necessary
to use the language features mentioned in points 7 and 8
10. Add a number of type annotations to method overrides -- this is to
silence warnings reported by the Scala 2 compiler as a result of
enabling `-Xsource:3`. All of the warnings relate to the inferred type
of the method changing between Scala 2 and 3
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jun 27, 2024
@EugeneFlesselle
Also handle imports on parameters of lambdas returned from inline defs
c4e703f 
Both i19493 and i19436 require mapping the type of
the expr in an `ImportType` which is itself the info of a `TermRef`.
In the first issue, for the substitution of an inline def parameter proxy.
In the second issue, for the parameter of a lambda returned from an inline def.

Both can be handled in `TypeMap` by mapping over references to `ImportType`s.
The second case also requires modifying `TreeTypeMap#mapType` such that
the logic mapping over imports is done within a `TypeMap` doing the symbol substitutions.

Fixes scala#19436
@EugeneFlesselle EugeneFlesselle mentioned this issue Jun 27, 2024
Merged
EugeneFlesselle added a commit to dotty-staging/dotty that referenced this issue Jul 2, 2024
@EugeneFlesselle
Also handle imports on parameters of lambdas returned from inline defs
ff003fd 
Both i19493 and i19436 require mapping the type of
the expr in an `ImportType` which is itself the info of a `TermRef`.
In the first issue, for the substitution of an inline def parameter proxy.
In the second issue, for the parameter of a lambda returned from an inline def.

Both can be handled in `TypeMap` by mapping over references to `ImportType`s.
The second case also requires modifying `TreeTypeMap#mapType` such that
the logic mapping over imports is done within a `TypeMap` doing the symbol substitutions.

Fixes scala#19436
odersky added a commit that referenced this issue Jul 2, 2024
@odersky
Map over refs of ImportTypes in TypeMap (#20837)
cd8c5ed 
The inliner replaces references to parameters by their corresponding
proxys, including in singleton types.
It did not handle the mapping over import types, the symbols of which
way have depended on parameters.

Both i19493 and i19436 require mapping the type of the expr in an
`ImportType` which is itself the info of a `TermRef`.
In the first issue, for the substitution of an inline def parameter
proxy.
In the second issue, for the parameter of a lambda returned from an
inline def.

Both can be handled in `TypeMap` by mapping over references to
`ImportType`s.
The second case also requires modifying `TreeTypeMap#mapType` such that
the logic mapping over imports is done within a `TypeMap` doing the
symbol substitutions.

Also note these mappings are only necessary for `summonInline`s (which
could have just been made non-inline) resolving
post-inlining to givens imported within the inline definition.

Fix #19493
Fix #19436
WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this issue Jul 2, 2024
@EugeneFlesselle @WojciechMazur
Also handle imports on parameters of lambdas returned from inline defs
a16ed38 
Both i19493 and i19436 require mapping the type of
the expr in an `ImportType` which is itself the info of a `TermRef`.
In the first issue, for the substitution of an inline def parameter proxy.
In the second issue, for the parameter of a lambda returned from an inline def.

Both can be handled in `TypeMap` by mapping over references to `ImportType`s.
The second case also requires modifying `TreeTypeMap#mapType` such that
the logic mapping over imports is done within a `TypeMap` doing the symbol substitutions.

Fixes scala#19436
@Kordyjan Kordyjan added this to the 3.5.1 milestone Jul 3, 2024
WojciechMazur pushed a commit that referenced this issue Jul 10, 2024
@EugeneFlesselle @WojciechMazur
Also handle imports on parameters of lambdas returned from inline defs
1b9b913 
Both i19493 and i19436 require mapping the type of
the expr in an `ImportType` which is itself the info of a `TermRef`.
In the first issue, for the substitution of an inline def parameter proxy.
In the second issue, for the parameter of a lambda returned from an inline def.

Both can be handled in `TypeMap` by mapping over references to `ImportType`s.
The second case also requires modifying `TreeTypeMap#mapType` such that
the logic mapping over imports is done within a `TypeMap` doing the symbol substitutions.

Fixes #19436

[Cherry-picked ff003fd]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@odersky odersky

@EugeneFlesselle EugeneFlesselle

Labels
area:metaprogramming:quotes Issues related to quotes and splices area:pickling itype:bug itype:crash
Projects
None yet
Milestone
3.5.1
Development

Successfully merging a pull request may close this issue.

Map over refs of ImportTypes in TypeMap dotty-staging/dotty
8 participants
@mbovel @odersky @lihaoyi @KuceraMartin @nicolasstucki @Kordyjan @EugeneFlesselle @jchyb