scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.89k stars 1.06k forks source link

Selection on term defined in current clause unexpectedly fails #17242

Closed Sporarum closed 2 weeks ago

Sporarum commented 1 year ago

Compiler version

Scala 3.2.2

Minimized code and Ouptut

class local(predicate: Int) extends annotation.StaticAnnotation

def failing1(x: Int, z: Int @local(x + x)) = ??? //error: undefined: x.+ # -1: TermRef(TermParamRef(x),+) at typer

Scastie which also contains the examples below: https://scastie.scala-lang.org/jfNlEM8sR26DI2priR6mWQ

Expectation

Assuming following definitions in scope

class local(num: Int) extends annotation.StaticAnnotation
class local2(predicate: Boolean) extends annotation.StaticAnnotation

The above should compile since the reference to x is totally valid here, for example the following compiles:

def working2(x: Int, z: Int @local(x)) = ???
def working3(x: Int, z: Int & x.type ) = ???

You can see in particular that it is the selection that causes the problem, and not the reference to x

In dependent types, the selection doesn't cause the same problem:

def expectedlyFailing2(x: String, z: x.length.type) = ??? //error: Int is not a valid singleton type, since it is not an immutable path

Notice that the error mentions Int explicitly, so it has to know x.length is both a valid selection, and of type Int

I do not advocate for changing the following however:

def expectedlyFailing1(x: Int @local(x == x)) = ??? //error: Cyclic reference involving val x

Finally, here is another failing example, even though the selection in question is == which is defined on (pretty much?) every Scala type:

def failing2(x: Int, z: Int @local2(x == x)) = ??? //error: undefined: x.== # -1: TermRef(TermParamRef(x),==) at typer
Sporarum commented 1 year ago

Following today's investigation, here is what I found: This results from fromSymbols return expression, where it calls tl.integrate, which replaces the parameters (x, z) in the type of z by corresponding TermParamRefs, which have an empty underlying When the compiler later tries to select .+ on x, it fails, as it was replaced during the step described above

Note that tl has already found a type with a correct underlying, so this should definitely be possible

Sporarum commented 1 year ago

I added the tests in the PR above

mbovel commented 1 year ago

This results from fromSymbols return expression, where it calls tl.integrate, which replaces the parameters (x, z) in the type of z by corresponding TermParamRefs, which have an empty underlying

Yup, binder.paramInfos is null there. We are actually computing it at this point; the problematic call goes through the application of paramInfosExp:

https://github.com/lampepfl/dotty/blob/083027e456adef0f17af4a31cba309c963e0ca81/compiler/src/dotty/tools/dotc/core/Types.scala#L3933

Complete stack trace ``` dotty.tools.dotc.core.Types$ParamRef.underlying(Types.scala:4552) dotty.tools.dotc.core.Types$Type.go$1(Types.scala:720) dotty.tools.dotc.core.Types$Type.findMember(Types.scala:873) dotty.tools.dotc.core.Types$Type.memberBasedOnFlags(Types.scala:673) dotty.tools.dotc.core.Types$Type.nonPrivateMember(Types.scala:663) dotty.tools.dotc.core.Types$NamedType.memberDenot(Types.scala:2401) dotty.tools.dotc.core.Types$NamedType.reload$1(Types.scala:2733) dotty.tools.dotc.core.Types$NamedType.withPrefix(Types.scala:2747) dotty.tools.dotc.core.Types$NamedType.derivedSelect(Types.scala:2681) dotty.tools.dotc.core.Substituters$.subst2(Substituters.scala:49) dotty.tools.dotc.core.Substituters$Subst2Map.apply(Substituters.scala:176) dotty.tools.dotc.core.Annotations$Annotation$$anon$1.apply(Annotations.scala:64) dotty.tools.dotc.core.Annotations$Annotation$$anon$1.apply(Annotations.scala:61) dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1551) dotty.tools.dotc.core.Annotations$Annotation$$anon$1.apply(Annotations.scala:65) dotty.tools.dotc.core.Annotations$Annotation$$anon$1.apply(Annotations.scala:61) dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.fold$1(Trees.scala:1532) dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.apply(Trees.scala:1534) dotty.tools.dotc.core.Annotations$Annotation.mapWith(Annotations.scala:66) dotty.tools.dotc.core.Types$TypeMap.mapOver(Types.scala:5724) dotty.tools.dotc.core.Substituters$.subst2(Substituters.scala:54) dotty.tools.dotc.core.Types$Type.subst(Types.scala:1789) dotty.tools.dotc.core.Types$LambdaType.integrate(Types.scala:3662) dotty.tools.dotc.core.Types$LambdaType.integrate$(Types.scala:3606) dotty.tools.dotc.core.Types$MethodOrPoly.integrate(Types.scala:3692) dotty.tools.dotc.core.Types$MethodTypeCompanion.fromSymbols$$anonfun$2$$anonfun$1(Types.scala:4040) scala.collection.immutable.List.map(List.scala:250) dotty.tools.dotc.core.Types$MethodTypeCompanion.fromSymbols$$anonfun$2(Types.scala:4040) dotty.tools.dotc.core.Types$MethodType.(Types.scala:3933) dotty.tools.dotc.core.Types$CachedMethodType.(Types.scala:3953) dotty.tools.dotc.core.Types$MethodTypeCompanion.apply(Types.scala:4045) dotty.tools.dotc.core.Types$MethodTypeCompanion.fromSymbols(Types.scala:4041) dotty.tools.dotc.core.NamerOps$.recur$1(NamerOps.scala:52) dotty.tools.dotc.core.NamerOps$.methodType(NamerOps.scala:56) dotty.tools.dotc.typer.Namer.wrapMethType$1(Namer.scala:1775) dotty.tools.dotc.typer.Namer.defDefSig$$anonfun$4(Namer.scala:1781) dotty.tools.dotc.typer.Namer.schema$lzyINIT1$1(Namer.scala:1802) dotty.tools.dotc.typer.Namer.schema$1(Namer.scala:1802) dotty.tools.dotc.typer.Namer.$anonfun$34(Namer.scala:1827) scala.collection.LinearSeqOps.foldLeft(LinearSeq.scala:183) scala.collection.LinearSeqOps.foldLeft$(LinearSeq.scala:179) scala.collection.immutable.List.foldLeft(List.scala:79) dotty.tools.dotc.typer.Namer.inferredResultType(Namer.scala:1831) dotty.tools.dotc.typer.Namer.inferredType$1(Namer.scala:1683) dotty.tools.dotc.typer.Namer.valOrDefDefSig(Namer.scala:1690) dotty.tools.dotc.typer.Namer.defDefSig(Namer.scala:1781) dotty.tools.dotc.typer.Namer$Completer.typeSig(Namer.scala:791) dotty.tools.dotc.typer.Namer$Completer.completeInCreationContext(Namer.scala:926) dotty.tools.dotc.typer.Namer$Completer.complete(Namer.scala:814) dotty.tools.dotc.core.SymDenotations$SymDenotation.completeFrom(SymDenotations.scala:174) dotty.tools.dotc.core.Denotations$Denotation.completeInfo$1(Denotations.scala:187) dotty.tools.dotc.core.Denotations$Denotation.info(Denotations.scala:189) dotty.tools.dotc.core.SymDenotations$SymDenotation.ensureCompleted(SymDenotations.scala:393) dotty.tools.dotc.typer.Typer.retrieveSym(Typer.scala:2932) dotty.tools.dotc.typer.Typer.typedNamed$1(Typer.scala:2957) dotty.tools.dotc.typer.Typer.typedUnadapted(Typer.scala:3053) dotty.tools.dotc.typer.Typer.typed(Typer.scala:3126) dotty.tools.dotc.typer.Typer.typed(Typer.scala:3130) dotty.tools.dotc.typer.Typer.traverse$1(Typer.scala:3152) dotty.tools.dotc.typer.Typer.typedStats(Typer.scala:3198) dotty.tools.dotc.typer.Typer.typedClassDef(Typer.scala:2612) dotty.tools.dotc.typer.Typer.typedTypeOrClassDef$1(Typer.scala:2979) dotty.tools.dotc.typer.Typer.typedNamed$1(Typer.scala:2983) dotty.tools.dotc.typer.Typer.typedUnadapted(Typer.scala:3053) dotty.tools.dotc.typer.Typer.typed(Typer.scala:3126) dotty.tools.dotc.typer.Typer.typed(Typer.scala:3130) dotty.tools.dotc.typer.Typer.traverse$1(Typer.scala:3152) dotty.tools.dotc.typer.Typer.typedStats(Typer.scala:3198) dotty.tools.dotc.typer.Typer.typedPackageDef(Typer.scala:2755) dotty.tools.dotc.typer.Typer.typedUnnamed$1(Typer.scala:3024) dotty.tools.dotc.typer.Typer.typedUnadapted(Typer.scala:3054) dotty.tools.dotc.typer.Typer.typed(Typer.scala:3126) ```
mbovel commented 1 year ago

17256 demonstrates that the problem is indeed the nested reading of paramInfos while its being computed. The quick and dirty fix there is however not a viable solution as @sjrd mentioned.

I am wondering what other approaches we could take to solve this problem. Seems like we should generally avoid TermParamRef.underlying returning NoType, but I don't see yet how. Could we somehow return a backup type there when binder.paramInfos is null? If yes, what should it be?

mbovel commented 1 year ago

Another possibility could be to disable the check in TypeAssigner.assignType(tree: untpd.Apply, ... in some cases:

diff --git a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
index 98e9cb638c1..8f3a18f38f3 100644
--- a/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
+++ b/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
@@ -289,6 +289,10 @@ trait TypeAssigner {
           else fntpe.resultType // fast path optimization
         else
           errorType(em"wrong number of arguments at ${ctx.phase.prev} for $fntpe: ${fn.tpe}, expected: ${fntpe.paramInfos.length}, found: ${args.length}", tree.srcPos)
+      case NoType if fn.tpe.isInstanceOf[TermRef] =>
+        // TermRef is not bound yet. Wait silently.
+        // See https://github.com/lampepfl/dotty/issues/17242.
+        fn.tpe
       case t =>
         if (ctx.settings.Ydebug.value) new FatalError("").printStackTrace()
         errorType(err.takesNoParamsMsg(fn, ""), tree.srcPos)

But this is probably too general (makes tests/neg-custom-args/fatal-warnings/i15503e.scala fail for example). It seems that in our case, we can only substitute for a more precise type, so we might avoid the check, but that's not the case in general.

mbovel commented 1 year ago

This compiles:

class local(predicate: Int) extends annotation.StaticAnnotation
def working4(x: Int, y: Int @local((() => x + x)())) = ()
Sporarum commented 1 year ago

This might be related to the following:

def foo[A, B](x: A, y: B = x) = () //error: not found: x

def bar[A, B](x: A)(y: B = x) = ()

(This also showcases why it's useful that default parameters are checked at call-site)