-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Compiler crash when quoting an inline argument of function type #4431
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
Comments
The issue here is that we do not support |
The same lack of support is cought correctly by the folowing assertion as stated in #4433 $ dotty-0.8.0-RC1/bin/dotr
Starting dotty REPL...
scala> inline def g(inline p: Int => Boolean): Boolean = ~{
if(p(5)) '(true)
else '(false)
}
def g(p: (Int => Boolean) @InlineParam): Boolean
scala>
scala> g(i => false)
Exception in thread "main" java.lang.AssertionError: assertion failed
at dotty.DottyPredef$.assertFail(DottyPredef.scala:35)
at dotty.tools.dotc.transform.Splicer$.$anonfun$4(Splicer.scala:61)
at scala.collection.immutable.List.map(List.scala:283)
at dotty.tools.dotc.transform.Splicer$.liftArgs$1(Splicer.scala:64)
at dotty.tools.dotc.transform.Splicer$.getLiftedArgs(Splicer.scala:72)
at dotty.tools.dotc.transform.Splicer$.splice(Splicer.scala:36)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.op1$1(ReifyQuotes.scala:568)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.transform(ReifyQuotes.scala:537)
at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1204)
at dotty.tools.dotc.transform.MacroTransform$Transformer.transform(MacroTransform.scala:54)
at dotty.tools.dotc.transform.MacroTransformWithImplicits$ImplicitsTransformer.transform(MacroTransformWithImplicits.scala:86)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.mapOverTree$1(ReifyQuotes.scala:539)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.op1$1(ReifyQuotes.scala:606)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.transform(ReifyQuotes.scala:537)
at dotty.tools.dotc.transform.MacroTransformWithImplicits$ImplicitsTransformer.traverse$1(MacroTransformWithImplicits.scala:53)
at dotty.tools.dotc.transform.MacroTransformWithImplicits$ImplicitsTransformer.transformStats(MacroTransformWithImplicits.scala:60)
at dotty.tools.dotc.transform.MacroTransform$Transformer.transform(MacroTransform.scala:60)
at dotty.tools.dotc.transform.MacroTransformWithImplicits$ImplicitsTransformer.transform(MacroTransformWithImplicits.scala:86)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.mapOverTree$1(ReifyQuotes.scala:539)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.op1$1(ReifyQuotes.scala:606)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.transform(ReifyQuotes.scala:537)
at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1211)
at dotty.tools.dotc.transform.MacroTransform$Transformer.transform(MacroTransform.scala:54)
at dotty.tools.dotc.transform.MacroTransformWithImplicits$ImplicitsTransformer.transform(MacroTransformWithImplicits.scala:86)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.mapOverTree$1(ReifyQuotes.scala:539)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.op1$1(ReifyQuotes.scala:606)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.transform(ReifyQuotes.scala:537)
at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform$$anonfun$2(Trees.scala:1231)
at scala.collection.immutable.List.mapConserve(List.scala:176)
at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1231)
at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transformStats(Trees.scala:1229)
at dotty.tools.dotc.ast.Trees$Instance$TreeMap.transform(Trees.scala:1217)
at dotty.tools.dotc.transform.MacroTransform$Transformer.transform(MacroTransform.scala:54)
at dotty.tools.dotc.transform.MacroTransformWithImplicits$ImplicitsTransformer.transform(MacroTransformWithImplicits.scala:86)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.mapOverTree$1(ReifyQuotes.scala:539)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.op1$1(ReifyQuotes.scala:606)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.transform(ReifyQuotes.scala:537)
at dotty.tools.dotc.transform.MacroTransform.run(MacroTransform.scala:22)
at dotty.tools.dotc.transform.ReifyQuotes.run(ReifyQuotes.scala:104)
at dotty.tools.dotc.core.Phases$Phase.runOn$$anonfun$1(Phases.scala:295)
at scala.collection.immutable.List.map(List.scala:283)
at dotty.tools.dotc.core.Phases$Phase.runOn(Phases.scala:297)
at dotty.tools.dotc.Run.runPhases$4$$anonfun$4(Run.scala:171)
at scala.compat.java8.JProcedure1.apply(JProcedure1.java:18)
at scala.compat.java8.JProcedure1.apply(JProcedure1.java:10)
at scala.collection.IndexedSeqOptimized.foreach(IndexedSeqOptimized.scala:32)
at scala.collection.IndexedSeqOptimized.foreach$(IndexedSeqOptimized.scala:29)
at scala.collection.mutable.ArrayOps$ofRef.foreach(ArrayOps.scala:191)
at dotty.tools.dotc.Run.runPhases$5(Run.scala:186)
at dotty.tools.dotc.Run.compileUnits$$anonfun$1(Run.scala:194)
at scala.compat.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:12)
at dotty.tools.dotc.util.Stats$.maybeMonitored(Stats.scala:88)
at dotty.tools.dotc.Run.compileUnits(Run.scala:201)
at dotty.tools.dotc.Run.compileUnits(Run.scala:141)
at dotty.tools.repl.ReplCompiler.runCompilationUnit(ReplCompiler.scala:188)
at dotty.tools.repl.ReplCompiler.compile(ReplCompiler.scala:197)
at dotty.tools.repl.ReplDriver.compile(ReplDriver.scala:229)
at dotty.tools.repl.ReplDriver.interpret(ReplDriver.scala:202)
at dotty.tools.repl.ReplDriver.loop$1(ReplDriver.scala:150)
at dotty.tools.repl.ReplDriver.runUntilQuit$$anonfun$1(ReplDriver.scala:154)
at dotty.tools.repl.ReplDriver.withRedirectedOutput$$anonfun$2$$anonfun$1(ReplDriver.scala:163)
at scala.util.DynamicVariable.withValue(DynamicVariable.scala:58)
at scala.Console$.withErr(Console.scala:192)
at dotty.tools.repl.ReplDriver.withRedirectedOutput$$anonfun$1(ReplDriver.scala:163)
at scala.util.DynamicVariable.withValue(DynamicVariable.scala:58)
at scala.Console$.withOut(Console.scala:163)
at dotty.tools.repl.ReplDriver.withRedirectedOutput(ReplDriver.scala:163)
at dotty.tools.repl.ReplDriver.runUntilQuit(ReplDriver.scala:154)
at dotty.tools.repl.Main$.main(Main.scala:6)
at dotty.tools.repl.Main.main(Main.scala) |
But my example doesn't need the value of the function at compile time, only its code. Also, the documentation states that
so
Code using inline functions is part of the documentation: inline def foreach(inline op: Int => Unit): Unit = {
var i = from
while (i < end) {
op(i)
i += 1
}
} I don't think this issue is the same as #4433, where the value of an |
True. The issue is that we may not be able to support |
I suppose you meant scala> inline def h(inline f: Int => String): String = ~{('(f))('(42))}
def h(f: (Int => String) @InlineParam): String
scala> but I don't see why that should be different than
I suppose this comment applies to #4433 only, where the function is actually run at compile time. Or do you mean that the |
Both. At compile time in a |
Unless they are quoted, no? Which is the case in this issue. After all, the example from my previous comment compiled. |
Yes. And all non inlined parameters must be quoted. Quoting an inline parameter is possible but not really useful. Your previous example will break at use site. |
Heh, you're right that it breaks at runtime, which is even worse than a compiler crash. Don't you agree that #4433 is a different issue, since there the inline parameter is not quoted? That's the use case I actually care about: moving some computation to compile time. |
For me there are two distinct manifestations of the same root issue. Disallowing it would ensure that the programs generated do not crash. |
@nicolasstucki it seems to me that inline def h(inline f: Int => String): String = ~ '(f(42)); h(x => x + 1) should work and produce something like @TomasMikula is that what you'd expect? However, one does need to decide whether this should beta-reduce or not, because that matters if the argument isn't a value. inline def h(inline f: Int => String): String = ~ '(println("start of h's generated code"); f { val x = 42; println(x); x })
// how many println's in the output? how are they ordered relative to the println in `h`?
h(x => x + x)
h(x => 0) // ditto The SIP-28 cited by But I guess @biboudis is the expert here, and the foundations he's investigating (will) offer principled answer to those questions. |
@TomasMikula do you want to execute a function in the macro that is defined in the call site of that macro? |
@Blaisorblade Yup, that would be an acceptable outcome.
If the issue is that
In #4433, yes, and I don't find it too crazy (with some restrictions). |
We already tried interpretation of source code mixed with reflective calls to evaluate the contents of the There is another way to evaluate functions that are passed as argument. We could JIT compile them and run them as long as they do not contain any free variables and only refer to static method that are already compiled. Currently, this is broken but I will make it work at some point, I will create some other issue with the details. The code might look something like this: inline def g(p: Int => Boolean): Boolean = ~{
val f = ('(p)).run
if(f(5)) '(true)
else '(false)
} |
@nicolasstucki I see why #4433 requires actual evaluation, but the code in this issue should only require inlining, just as it would be if the body of |
Maybe we can support it. But then the semantics of inline function would not be the same as the other inline parameters. It is cleaner to not do so and use the normal mechanism quoting an splicing to achive the same with consistent semantics. |
When I referred to the semantics I meant that the values of inline parameters are available at stage -1 (i.e. under the top level ~ in the inline method). These may need some more documentation, this is a consecuence of the last bullet point in relationship-with-inline-and-macros. |
What we are going to do is to disallow If in the future we have compelling usecases we can start supporting it then. Taking into acount that it is possible to support the same using quotes and splices. |
I might still be missing something and I'm unconvinced (other than on short-term implementation restrictions for compiler stability).
That's interesting, but I think that's at fault. It also means, again, that the true problem is not with this issue but with #4433, and it is relevant because inline params can be also evaluated per the bullet point you quote, IIUC that is:
But I think that property should be restricted depending on the type (arguably with a typeclass), since it's a form of cross-stage persistence "in reverse", since it requires evaluating at compile-time program expressions. That's easy for integers (the example), and for strings, and for other literals. But that's hard for functions, class instances and so on. So it seems that rule should not apply consistently. It's really just a non-principled way to provide a limited form of inspection—matching on the content of an expression, finding a literal, extracting its content.
As a short-term measure, fine.
That would be convincing, but how would that work? I think the proper way to do it is by writing import scala.quoted._
def hMacro(f: Expr[Int => String]): Expr[String] = f('(42))
inline def h1(f: Int => String): String = ~hMacro('(f))
inline def h2(inline f: Int => String): String = ~hMacro('(f))
val fArg: Int => String = _.toString
scala> h1(_.toString)
1 |h1(_.toString)
|^^^^^^^^^^^^^^
|Could not find interpreted class rs$line$5 in classpath
1 |
| ^
| access to value f from wrong staging level:
| - the definition is at level 0,
| - but the access is at level 1.
cala> h1(fArg)
1 |h1(fArg)
|^^^^^^^^
|Could not find interpreted class rs$line$4 in classpath
scala> h2(_.toString)
Exception in thread "main" java.lang.AssertionError: assertion failed
at scala.Predef$.assert(Predef.scala:204)
at dotty.tools.dotc.transform.Splicer$.$anonfun$getLiftedArgs$2(Splicer.scala:61)
at scala.collection.immutable.List.map(List.scala:283)
[...]
scala> h2(fArg)
Exception in thread "main" java.lang.AssertionError: assertion failed
at scala.Predef$.assert(Predef.scala:204)
at dotty.tools.dotc.transform.Splicer$.$anonfun$getLiftedArgs$2(Splicer.scala:61)
at scala.collection.immutable.List.map(List.scala:283)
at dotty.tools.dotc.transform.Splicer$.liftArgs$1(Splicer.scala:58)
at dotty.tools.dotc.transform.Splicer$.getLiftedArgs(Splicer.scala:72)
at dotty.tools.dotc.transform.Splicer$.splice(Splicer.scala:36)
at dotty.tools.dotc.transform.ReifyQuotes$Reifier.$anonfun$transform$2(ReifyQuotes.scala:562)
at dotty.tools.dotc.reporting.trace$.op1$3(trace.scala:32)
[...] |
In We could only implement it if we can somehow enforce that the closure passed to the inline function argument has no free variables and is fully inlined (not always the cases with We used |
I created a PR where the semantics are modified to allow inline functions as parameters. |
Why would the interpreter need to know how the language is compiled to JVM bytecode?
Limiting to only static methods seems too restrictive. Also, it would be nice to at least also be able to refer to methods that themselves have this property, i.e.
Just throwing in a suggestion:
I'm afraid it has to be restricted based on the actual value passed in for an |
See #4460 |
That's already required to be a "constant expression". FWIW, once we're adding inspection, extracting the literal could be supported by something like the Scala 2 macro idiom
or something more complicated on tasty trees. (Probably it'd need to be a simplifier for all constant expressions). But not sure what's preferable there. |
What does constant expression mean, exactly? Is a function literal a constant expression? |
Contant expression there are basically primitive value types Int, Boolean,...., String and a couple of singleton types like Unit and Null. Function literals are not constant expressions. |
Since our docs and errors talk about constant expressions, can we ensure we mean it in the sense of the spec (if that's not already the case?) and/or that we describe deviations? The basic definition is in SLS 6.24; SIP-23 essentially associates a singleton type with each constant expression. In fact, among platform-dependent constant expressions we also (seem to) have those computations that are constant-folded by the typer to obtain their singleton types — see the definition of constant value definitions in SLS 4.1. That also works for inline functions: scala> final val x = 1 + 2
val x: Int(3) = 3
scala> inline def f(inline w: Int) = w
def f(w: Int @InlineParam): Int
scala> f(x+2)
val res9: Int = 5
scala> val z = 1
val z: Int = 1
scala> f(z+2)
1 |f(z+2)
| ^^^
| argument to inline parameter must be a constant expression or a function |
Fix #4431: Change semantics of inline params in macro
I later confirmed this is the case; the error checking's done by |
Clarification following scala#4431. The error checking's done by `Checking.checkInlineConformant(tree, isFinal = false, "argument to inline parameter")`, and `checkInlineConformant` uses `ConstantType` which characterizes constant expressions.
Clarification following scala#4431. The error checking's done by `Checking.checkInlineConformant(tree, isFinal = false, "argument to inline parameter")`, and `checkInlineConformant` uses `ConstantType` which characterizes constant expressions.
The text was updated successfully, but these errors were encountered: