summonAll after importing givens from a function parameter causes compiler crash #19493
Comments
KuceraMartin
added
itype:bug
itype:crash
stat:needs triage
Miminizedimport compiletime.{summonInline, summonAll}
class DialectTypeMappers:
given aString: String = ???
inline def metadata(dialect: DialectTypeMappers): Unit =
import dialect.given
summonInline[String]
// summonAll[Tuple1[String]]
def f =
metadata((??? : DialectTypeMappers))
where |
nicolasstucki
added
area:inline
and removed
stat:needs triage
|
My best guess so far we are using the wrong context here https://github.com/lampepfl/dotty/blob/main/compiler/src/dotty/tools/dotc/inlines/Inlines.scala#L427-L437 |
|
This issue was picked for the Issue Spree of Februrary 27th, 2024. @natsukagami, @KuceraMartin and @jan-pieter will be working on it. If you have any insight into the issue or guidance on how to fix it, please leave it here. |
|
Some more context: Looking at |
|
@EugeneFlesselle, do you plan to work on this? |
lihaoyi
pushed a commit
to com-lihaoyi/scalasql
that referenced
this issue
May 25, 2024
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
The inliner replaces references to parameters by their corresponding proxys, including in singleton types. It did not, however, handle the mapping over import types, the symbols of which way have depended on parameters. Mapping imports correctly was necessary for i19493 since the `summonInline` resolves post inlining to a given imported within the inline definition. Fix scala#19493
odersky
added a commit
that referenced
this issue
Jul 2, 2024
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
The inliner replaces references to parameters by their corresponding proxys, including in singleton types. It did not, however, handle the mapping over import types, the symbols of which way have depended on parameters. Mapping imports correctly was necessary for i19493 since the `summonInline` resolves post inlining to a given imported within the inline definition. Fix scala#19493
WojciechMazur
pushed a commit
that referenced
this issue
Jul 10, 2024
The inliner replaces references to parameters by their corresponding proxys, including in singleton types. It did not, however, handle the mapping over import types, the symbols of which way have depended on parameters. Mapping imports correctly was necessary for i19493 since the `summonInline` resolves post inlining to a given imported within the inline definition. Fix #19493 [Cherry-picked 413d7b4]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Compiler version
3.4.0-RC2
Minimized code
Output (click arrow to expand)
note: could be related to #19436
The text was updated successfully, but these errors were encountered: