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.

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

Wrong compiler error message while using contextual functions #12941

Closed
BarkingBad opened this issue Jun 25, 2021 · 11 comments · Fixed by #14043
Closed

Wrong compiler error message while using contextual functions #12941

BarkingBad opened this issue Jun 25, 2021 · 11 comments · Fixed by #14043
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc exp:intermediate itype:bug
Milestone

Comments

@BarkingBad
Copy link
Contributor

BarkingBad commented Jun 25, 2021

Compiler version

Example is at 3.0.0 but the problem still exists for the newest master (commit b3ade17)

Minimized code

class I:
  def runSth: Int = 1

abstract class A:
  def myFun(op: I ?=> Unit) =
    op(using I())
    1

class B extends A
  
@main def hello: Unit = 
  
  B().myFun {
    val res = summon[I].runSth
    org.junit.Assert.assertEquals("", 1, res, "asd")
    // org.junit.Assert.assertEquals("", 1, res)
    println("Hello!")
  }

Output

[error] -- Error: /home/aratajczak/Dokumenty/Praca/Scala/compiler-bug/src/main/scala/Main.scala:14:23 
[error] 14 |    val res = summon[I].runSth
[error]    |                       ^
[error]    |no implicit argument of type I was found for parameter x of method summon in object Predef
[error] one error found
[error] one error found
[error] (Compile / compileIncremental) Compilation failed
[error] Total time: 0 s, completed 25 cze 2021, 16:08:49

Expectation

Error should point to wrong assertEquals dispatch instead of no implicit argument of type I.
However, if we change commented line, Program succeedes

[info] running hello 
Hello!
[success] Total time: 0 s, completed 25 cze 2021, 16:09:29

I think it only exists for calling Java functions, however for scala function it returns BOTH no implicit and None of the overloaded alternatives of method assertEquals in object Assert with types errors which is somehow more correct. I'll link clean repo with reproduction.

@BarkingBad
Copy link
Contributor Author

BarkingBad commented Jun 25, 2021

Here the repo: https://github.com/BarkingBad/dotty-bug-12941/blob/master/src/main/scala/Main.scala

Apparently making A abstract and B extending it is not required, can reproduce for just class A. Interesting thing is that making myFun top level function makes the compiler fail with None of the overloaded alternatives of method assertEquals in object Assert with types... which is correct

@BarkingBad BarkingBad changed the title Wrong compiler error message Wrong compiler error message while using contextual functions Jun 25, 2021
@odersky
Copy link
Contributor

odersky commented Jun 28, 2021

Further minimized without the junit dependency:

class I:
  def runSth: Int = 1

abstract class A:
  def myFun(op: I ?=> Unit) =
    op(using I())
    1

class B extends A

def assertEquals(x: String, y: Int, z: Int): Unit = ()

@main def hello: Unit =

  B().myFun {
    val res = summon[I].runSth
    assertEquals("", 1, res, "asd")
    println("Hello!")
  }

@odersky
Copy link
Contributor

odersky commented Jun 28, 2021

Test output:

-- Error: test.scala:16:23 -----------------------------------------------------
16 |    val res = summon[I].runSth
   |                       ^
   |no implicit argument of type I was found for parameter x of method summon in object Predef
-- Error: test.scala:17:29 -----------------------------------------------------
17 |    assertEquals("", 1, res, "asd")
   |                             ^^^^^
   |too many arguments for method assertEquals: (x: String, y: Int, z: Int): Unit
2 errors found

The first error is unexpected.

@anatoliykmetyuk anatoliykmetyuk added area:reporting Error reporting including formatting, implicit suggestions, etc exp:intermediate labels Jul 20, 2021
@SethTisue
Copy link
Member

Beat our heads on this for a while at the spree, with Jiri and Gagandeep.

What happens here is that when the second error is encountered, the compiler attempts error recovery, and when it does that,

  1. the scope of the error recovery is the entire function literal, not just the erroneous line
  2. error recovery is attempted with the expected type for the function literal being WildcardType, rather than expecting a context function, and that's why summon[I] no longer compiles on the second try

the WildcardType comes from line 385 of ProtoTypes.scala; it calls typer.typed(norm(arg, idx)), and since only one argument is passed to typed, the second argument defaults to WildcardType

We aren't even sure what the right solution plan is here. Some approaches we considered:

  1. At the time compilation of summon[I] succeeds, can it somehow plant a flag, somehow "commit" that this is definitely a context function, the expected type is definitely I ?=> ....
  2. Or at the time we're about to do the second pass, can we somehow determine at that point that retrying compilation expecting WildcardType makes no sense to even try?
  3. Or is it actually right to proceed with WildcardType, and the bug is simply that the first error ought to be suppressed? (But how would we decide to suppress it?)

@SethTisue
Copy link
Member

I could be interested in pursuing this further, but I feel like we need expert advice on what the appropriate solution plan is. If we had a solution plan sort of sketched out for us, maybe we could try to implement it.

@SethTisue
Copy link
Member

Jiri, Gagandeep, could you leave your even-more-minimized version of the code here?

@gagandeepkalra
Copy link
Contributor

gagandeepkalra commented Nov 17, 2021

Minimized code-

    object A:
      def myFun(op: String ?=> Unit) = ()

    @main def func: Unit =
      A.myFun {
        val res = summon[String]
        println(ress)
      }

@odersky
Copy link
Contributor

odersky commented Nov 17, 2021

the compiler attempts error recovery, and when it does that,

the scope of the error recovery is the entire function literal, not just the erroneous line

Did you find out where that recovery is triggered?

@jirid
Copy link

jirid commented Nov 17, 2021

The error recovery is triggered in Applications.scala, line 985 with a call to tryWithImplicitOnQualifier in function realApply.

@jirid
Copy link

jirid commented Nov 18, 2021

I have investigated the minimized code that Gagandeep proposed above a little further. The compiler behaves slightly differently on this code than the original code, but I believe that the difference does not matter here.

The original code was calling a function with valid arguments, but the wrong number of them. The minimized code is calling a function with the correct number of arguments, but the argument itself is invalid. In this new code, the problematic recovery attempt does happen on the wrong line first, does not succeed and then it happens again on the entire function literal and produces the same unexpected diagnostic as the original code.

As I mentioned above, the error recovery happens in Applications.scala in function typedApply (line 879). typedApply defines 3 nested functions: realApply (line 881), simpleApply (line 895), and tryWithImplicitOnQualifier (line 910). The function realApply calls simpleApply (line 977) and when this fails, it attempts recovery by calling tryWithImplicitOnQualifier (line 985).

The first call to simpleApply (that we care about) happens with the following tree:

Apply(Select(Ident(A),myFun),List(Block(List(ValDef(res,TypeTree,TypeApply(Ident(summon),List(Ident(String))))),Apply(Ident(println),List(Ident(ress))))))

and this call eventually leads to a recursive call to simpleApply with this tree:

Apply(Ident(println),List(Ident(ress)))

tryWithImplicitOnQualifier is first called for the inner call to simpleApply, this try is unable to fix the issue, but does not emit the unexpected diagnostic, therefore there is no problem here.

The outer call to simpleApply also leads to a call to tryWithImplicitOnQualifier, this time the evaluation of the call produces the unexpected diagnostic.

The logic of this error recovery attempt is implemented in Typer.scala in function tryInsertImplicitOnQualifier (line 3155). In this function, the compiler concludes that it should not attempt this error recovery, unfortunately the evaluation of the condition based on which the compiler decides is what produces the unexpected diagnostic. The expression in this function responsible for the diagnostic is selProto.isMatchedBy(qual.tpe) (line 3163), where the values are:

selProto:

SelectionProto(myFun,FunProto(Block(List(ValDef(res,TypeTree,TypeApply(Ident(summon),List(Ident(String))))),Apply(Ident(println),List(Ident(ress)))) => class dotty.tools.dotc.typer.ProtoTypes$CachedIgnoredProto),dotty.tools.dotc.typer.ProtoTypes$NoViewsAllowed$@44ba768a,false)

qual.tpe:

TermRef(ThisType(TypeRef(NoPrefix,module class <empty>)),object A)

The line val res = summon[String] is processed by the compiler twice. The first time is succeeds and the second time it fails. The difference between the two cases manifests in the function typedUnadapted in Typer.scala (line 2743). The pt: Type argument to this function has an actual type during the first (successful) pass and WildcardType (as Seth described above) in the second (failing) pass. The Context seems to be the same in both calls, but the first call, due to the specified type, leads to a call to makeContextualFunction (line 2844) which creates a nested context that contains the information about the existence of the implicit argument which allows the compiler to typecheck the summon line. The presence of the WildcardType in the second pass triggers no creation of nested contexts and therefore the compiler does not see the given value defined when it processes the summon line for the second time.

Source references based on commit 5f2e3b6acf60c3d347cc553b2ff9157c578f894f.

odersky added a commit to dotty-staging/dotty that referenced this issue Dec 5, 2021
Don't retypecheck erroneous arguments when trying conversions or extensions
on the function part.

Generally, we should avoid typechecking untyped trees several times, this can quickly
explode exponentially.

Fixes scala#12941
@odersky
Copy link
Contributor

odersky commented Dec 5, 2021

Was exp:intermediate a joke?

odersky added a commit to dotty-staging/dotty that referenced this issue Dec 19, 2021
Don't retypecheck erroneous arguments when trying conversions or extensions
on the function part.

Generally, we should avoid typechecking untyped trees several times, this can quickly
explode exponentially.

Fixes scala#12941
odersky added a commit to dotty-staging/dotty that referenced this issue Dec 19, 2021
Don't retypecheck erroneous arguments when trying conversions or extensions
on the function part.

Generally, we should avoid typechecking untyped trees several times, this can quickly
explode exponentially.

Fixes scala#12941
odersky added a commit to dotty-staging/dotty that referenced this issue Jan 3, 2022
Don't retypecheck erroneous arguments when trying conversions or extensions
on the function part.

Generally, we should avoid typechecking untyped trees several times, this can quickly
explode exponentially.

Fixes scala#12941
odersky added a commit to dotty-staging/dotty that referenced this issue Jan 23, 2022
Don't retypecheck erroneous arguments when trying conversions or extensions
on the function part.

Generally, we should avoid typechecking untyped trees several times, this can quickly
explode exponentially.

Fixes scala#12941
odersky added a commit to dotty-staging/dotty that referenced this issue Jan 24, 2022
Don't retypecheck erroneous arguments when trying conversions or extensions
on the function part.

Generally, we should avoid typechecking untyped trees several times, this can quickly
explode exponentially.

Fixes scala#12941
olsdavis pushed a commit to olsdavis/dotty that referenced this issue Apr 4, 2022
Don't retypecheck erroneous arguments when trying conversions or extensions
on the function part.

Generally, we should avoid typechecking untyped trees several times, this can quickly
explode exponentially.

Fixes scala#12941
@Kordyjan Kordyjan added this to the 3.1.2 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc exp:intermediate itype:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants