Skip to content

Selection on term defined in current clause unexpectedly fails #17242

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

Closed
Sporarum opened this issue Apr 13, 2023 · 7 comments · Fixed by #21941
Closed

Selection on term defined in current clause unexpectedly fails #17242

Sporarum opened this issue Apr 13, 2023 · 7 comments · Fixed by #21941

Comments

@Sporarum
Copy link
Contributor

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 Sporarum added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label area:typer stat:taken This project is already worked on by a student or as part of another volunteership program and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 13, 2023
@Sporarum Sporarum self-assigned this Apr 13, 2023
@Sporarum Sporarum added Spree Suitable for a future Spree and removed stat:taken This project is already worked on by a student or as part of another volunteership program labels Apr 13, 2023
@Sporarum Sporarum removed their assignment Apr 13, 2023
@Sporarum
Copy link
Contributor Author

Sporarum commented Apr 13, 2023

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 Sporarum linked a pull request Apr 13, 2023 that will close this issue
@Sporarum
Copy link
Contributor Author

I added the tests in the PR above

@Sporarum Sporarum mentioned this issue Apr 13, 2023
@mbovel
Copy link
Member

mbovel commented Apr 13, 2023

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.<init>(Types.scala:3933)
dotty.tools.dotc.core.Types$CachedMethodType.<init>(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
Copy link
Member

mbovel commented Apr 13, 2023

#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
Copy link
Member

mbovel commented Apr 14, 2023

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 mbovel removed the Spree Suitable for a future Spree label Apr 14, 2023
@mbovel
Copy link
Member

mbovel commented Apr 20, 2023

This compiles:

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

@Sporarum
Copy link
Contributor Author

Sporarum commented Aug 1, 2023

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants