Evaluating function value can lead to ClassCastException, when function type has by-name parameter

[heritamas]

Compiler version

3.3.0, 3.2.2, 3.2.1, 3.2.0

Minimized code

For reproduction, use Scala REPL, the issue seems to be REPL-related.

scala> val f: ( => Int) => Int = i => i ; f(1)
scala> f(1)

Output

scala> val f: ( => Int) => Int = i => i ; f(1)
val f: (=> Int) => Int = Lambda$1323/0x00007f9db44ac208@2dbf2da
val res0: Int = 1
                                                                                                                                                                     
scala> f(1)
java.lang.ClassCastException: class java.lang.Integer cannot be cast to class scala.Function0 (java.lang.Integer is in module java.base of loader 'bootstrap'; scala.Function0 is in unnamed module of loader java.net.URLClassLoader @4a951911)
  at scala.Function1.apply$mcII$sp(Function1.scala:69)
  ... 34 elided

Expectation

As f is basically the identity (for Ints), f(1) shall evaluate to 1.
Note: if the assignment and the evaluation entered on the same line, or entered in the same entry, such as:

scala> val f: ( => Int) => Int = i => i
     | f(1)
     | 
val f: (=> Int) => Int = Lambda$1324/0x00007f3d904a4208@7e38e254
val res0: Int = 1

the issue exhibits itself only when f is evaluated upon a different entry.

scala> f(2)
java.lang.ClassCastException: class java.lang.Integer cannot be cast to class scala.Function0 (java.lang.Integer is in module java.base of loader 'bootstrap'; scala.Function0 is in unnamed module of loader java.net.URLClassLoader @4a951911)
  at scala.Function1.apply$mcII$sp(Function1.scala:69)
  ... 34 elided

[heritamas]

Scala version 3.1.0 seems ok.

[nicolasstucki]

It seems that elimByName is not transforming one one the infos from (=> T): U into (() ?=> T): U.

[nicolasstucki]

I did not manage to fix this issue. Someone else should try to find the cause of this issue. #18761 has some insights and tests.

[SethTisue]

reported again today on Discord

fingers crossed that @smarter can figure it out

[smarter]

Here's a minimal change that fixes this particular issue:

diff --git compiler/src/dotty/tools/dotc/core/Denotations.scala compiler/src/dotty/tools/dotc/core/Denotations.scala
index bd92fa814a6..56e4b1b5e4d 100644
--- compiler/src/dotty/tools/dotc/core/Denotations.scala
+++ compiler/src/dotty/tools/dotc/core/Denotations.scala
@@ -1104,7 +1104,9 @@ object Denotations {
            && (derivedInfo eq info) && !needsPrefix then
           this
         else
-          derivedSingleDenotation(symbol, derivedInfo, pre)
+          val d = derivedSingleDenotation(symbol, derivedInfo, pre)
+          d.validFor = ctx.period
+          d
       end derived

       // Tt could happen that we see the symbol with prefix `this` as a member a different class

The issue is that when we create a new denotation from an existing one, the validity period of that denotation is set based on the validity of the original denotation. When I run:

scala> val f: ( => Int) => Int = i => i ; f(1)
...
scala> val f: ( => Int) => Int = i => i ; f(1)
ClassCastException

The difference is that in the second run, the denotation for f.apply is created from the symdenotation for Function1#apply which already exists and is known to be valid in every phase, but that doesn't mean that the derived denotation for f.apply has the same validity: unlike the symdenotation it needs to be transformed (in particular to run the ElimByName transformer).

It seems that this problem is pervasive in the codebase and not just limited to derivedSingleDenotation/newLikeThis, for example:

scala3/compiler/src/dotty/tools/dotc/core/Types.scala

Line 908 in 3d5cf9c

new UniqueRefDenotation(d.symbol, tp.memberInfo(d.symbol), d.validFor, pre)

The conservative approach would be to go through all denotation creation sites and use ctx.period as the validity period unless we can be sure it's not needed, but maybe we can do something like:

val validFor =
 if newInfo == oldDenot.info || !infoDependsOnPrefix(...) then
   oldDenot.validFor
 else
   ctx.period

/cc @odersky

[odersky]

We can try to change all denotation creation sites to currentStablePeriod. If that works and compile speed is not affected that would be simplest.

[odersky]

I did a quick scan. We already do the right thing for SymDenotation. For the others, it seems to be essentially the newLikeThis calls and maybe on instance in goSuper.