REPL: chained extension methods on opaque types can only be called once

[bishabosha]

Minimized code

Only in the REPL, it seems like calling a chain of extension methods has a strange side effect that prevents it from happening again.

First, compile this source:

// IArr.scala
package example
import scala.reflect.ClassTag

object opaques:
  opaque type IArr[+T] = Array[_ <: T]
  given arrayOps: AnyRef with
    extension [T](arr: IArr[T]) def reverse: IArr[T] =
      genericArrayOps(arr).reverse

    extension [T](arr: IArr[T]) def sorted(using math.Ordering[T]): IArr[T] =
      genericArrayOps(arr).sorted
end opaques

type IArr[+T] = opaques.IArr[T]

object IArr:
  inline def apply(inline x: Int, inline xs: Int*): IArr[Int] = Array(x, xs: _*).asInstanceOf

Then run the repl task with IArr on the classpath

scala> import example.IArr

scala> IArr(1,2,3).reverse.sorted                                                                                   
val res0: example.opaques.IArr[Int] = Array(1, 2, 3)

scala> IArr(1,2,3).reverse.sorted                                                                                   
1 |IArr(1,2,3).reverse.sorted
  |^^^^^^^^^^^^^^^^^^^^^^^^^^
  |value sorted is not a member of example.opaques.IArr[Int].
  |An extension method was tried, but could not be fully constructed:
  |
  |    genericArrayOps[T](
  |      example.opaques.arrayOps.reverse[Int](example.IArr.apply(1, [2,3 : Int]*))
  |    )

scala> 

Expectation

this is ok when they are called one after the other in a source file

[OlivierBlanvillain]

@anatoliykmetyuk Not sure who to assign this to, are you the most knowledgeable person on REPL at the moment?

[som-snytt]

The type seems to change subtly.

scala> IArray(1,2,3).reverse
val res0: opaques.IArray[Int] = Array(3, 2, 1)

scala> IArray(1,2,3).reverse
val res1: scala.opaques.IArray[Int] = Array(3, 2, 1)

Also

scala> IArray.emptyIntIArray.reverse
val res0: opaques.IArray[Int] = Array()

scala> IArray.emptyIntIArray.reverse
1 |IArray.emptyIntIArray.reverse
  |^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |value reverse is not a member of opaques.IArray[Int].
  |An extension method was tried, but could not be fully constructed:
  |
  |    opaques.arrayOps.reverse[T](IArray.emptyIntIArray)

[odersky]

Seems to work in 3.0-M3

[som-snytt]

Still broken on M3 and also current master

[success] Total time: 126 s (02:06), completed Dec 27, 2020, 9:32:57 AM
Starting scala3 REPL...
scala>

scala> IArray(1,2,3).reverse.sorted
val res0: opaques.IArray[Int] = Array(1, 2, 3)

scala> IArray(1,2,3).reverse.sorted
1 |IArray(1,2,3).reverse.sorted
  |^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |value sorted is not a member of scala.opaques.IArray[Int].
  |An extension method was tried, but could not be fully constructed:
  |
  |    copyArrayToImmutableIndexedSeq[T](
  |      opaques.arrayOps.reverse[Int](
  |        Array.apply(1, [2,3 : Int]:Int*:Int*).asInstanceOf[opaques.IArray[Int]]:
  |          IArray[Int]
  |      )
  |    )

scala>  

also per previous comment

scala> IArray(1,2,3).reverse
val res0: opaques.IArray[Int] = Array(3, 2, 1)

scala> IArray(1,2,3).reverse
val res1: scala.opaques.IArray[Int] = Array(3, 2, 1)

scala> IArray(1,2,3).reverse.sorted
1 |IArray(1,2,3).reverse.sorted
  |^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |value sorted is not a member of scala.opaques.IArray[Int].
  |An extension method was tried, but could not be fully constructed:
  |
  |    genericArrayOps[T](
  |      opaques.arrayOps.reverse[Int](
  |        Array.apply(1, [2,3 : Int]:Int*:Int*).asInstanceOf[opaques.IArray[Int]]:
  |          IArray[Int]
  |      )
  |    )

scala> (IArray(1,2,3).reverse: opaques.IArray[Int]).sorted
val res2: scala.opaques.IArray[Int] = Array(1, 2, 3)

scala> (IArray(1,2,3).reverse: scala.opaques.IArray[Int]).sorted
val res3: scala.opaques.IArray[Int] = Array(1, 2, 3)

scala> (IArray(1,2,3).reverse: IArray[Int]).sorted
val res4: scala.opaques.IArray[Int] = Array(1, 2, 3)

scala> 

[bishabosha]

This seems to be fixed for IArray since #11354 , but it can be reproduced by compiling the the old definition under a new name and adding to the classpath in the repl task

[nicolasstucki]

@bishabosha could you include the code you needed to reproduce it?

[bishabosha]

@nicolasstucki I have updated the original post

[mpilquist]

Is this the same issue or something new?

➜  cs launch scala3-repl
scala> object Foo:
     |   opaque type Bar = Int
     |   object Bar:
     |     extension (b: Bar) def flip: Bar = -b
     |     def apply(x: Int): Bar = x
     |
// defined object Foo

scala> val a = Foo.Bar(42)
val a: Foo.Bar = 42

scala> val b = a.flip
val b: Foo.Bar = -42

scala> val c = b.flip
val c: Foo.Bar = 42

scala> val d = c.flip
1 |val d = c.flip
  |        ^^^^^^
  |value flip is not a member of Foo.Bar, but could be made available as an extension method.
  |
  |The following import might fix the problem:
  |
  |  import Foo.Bar.flip
  |

scala>

[griggt]

The original example now has a different problem on master, the second invocation of the extension method works but the result type is no longer opaque:

Welcome to Scala 3.1.0-RC1-bin-SNAPSHOT-git-7d1cddb (1.8.0_292, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.

scala> import example.IArr

scala> IArr(1,2,3).reverse.sorted
val res0: example.opaques.IArr[Int] = Array(1, 2, 3)

scala> IArr(1,2,3).reverse.sorted
val res1: Array[? <: Int] = Array(1, 2, 3)

This behavior seems to have started with #13260

Interestingly, both the original example and the one from @mpilquist seem to work correctly if #11300 is reverted, but I suspect that's just papering over the real issue.

[griggt]

Using a customized REPL where :rawtype is :type without the pretty-printing, and :dealiased is :rawtype with .dealias:

scala> object Foo:
     |   opaque type Bar = Int
     |   object Bar:
     |     extension (b: Bar) def flip: Bar = -b
     |     def apply(x: Int): Bar = x
     | 
// defined object Foo

scala> val a = Foo.Bar(42)
val a: Foo.Bar = 42

scala> val b0 = a.flip
val b0: Foo.Bar = -42

scala> val b1 = a.flip
val b1: Foo.Bar = -42

scala> :dealiased b0
TypeRef(TermRef(TermRef(ThisType(TypeRef(NoPrefix,module class repl$)),object rs$line$1),object Foo),type Bar)

scala> :dealiased b1
TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),object scala),class Int)

scala> :rawtype b0
TypeRef(TermRef(TermRef(ThisType(TypeRef(NoPrefix,module class repl$)),object rs$line$1),object Foo),type Bar)

scala> :rawtype b1
TypeRef(ThisType(TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,module class repl$)),module class rs$line$1$)),module class Foo$)),type Bar)

scala> val c = b0.flip
val c: Foo.Bar = 42

scala> val d = c.flip
-- Error:
1 |val d = c.flip
  |        ^^^^^^
  |        value flip is not a member of Foo.Bar, but could be made available as an extension method.
  |
  |        The following import might fix the problem:
  |
  |          import Foo.Bar.flip
  |

scala> :dealiased c
TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),object scala),class Int)

scala> :rawtype c
TypeRef(ThisType(TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,module class repl$)),module class rs$line$1$)),module class Foo$)),type Bar)

[SethTisue]

fwiw, #13260 doesn't affect Michael's case either way.

my intuition is that Michael's case should be investigated first, and then we'd circle back to the original report

a plausible next investigation step here would be to analyze why, in Tom's last comment, b0 and b1 already have different types — that seems like the moment where things begin to go wrong. b1 has already seen through the opaque type.

[SethTisue]

Some side notes that may or may not prove helpful to understanding this...

First, note that only one val is actually necessary (though naming the intermediates may of course be helpful when investigating):

scala> val a = Foo.Bar(42)
val a: Foo.Bar = 42
                                                                                                                        
scala> a.flip
val res0: Foo.Bar = -42
                                                                                                                        
scala> a.flip.flip
-- [E008] Not Found Error: -----------------------------------------------------
1 |a.flip.flip
  |^^^^^^^^^^^
  |value flip is not a member of Foo.Bar
1 error found

Second, note that the problem goes away if you import Foo.Bar.flip:

scala> val a = Foo.Bar(42)
val a: Foo.Bar = 42
                                                                                                                        
scala> import Foo.Bar.flip
                                                                                                                        
scala> a.flip
val res0: Foo.Bar = -42
                                                                                                                        
scala> a.flip.flip
val res1: Foo.Bar = 42

that's because when typer erroneously infers Foo$#Bar instead of Foo.Bar, that interferes with implicit search, but if flip is brought into scope by the import, then we don't need implicit search in order to find it.

(But it doesn't matter I guess, because we want to fix the wrongly inferred type.)

[SethTisue]

One more aside: even though the REPL doesn't show it without Tom's patches, you can detect that the problem is an incorrectly inferred Foo$#Bar type by explicitly annotating the type on c and the problem goes away:

scala> val a = Foo.Bar(42)
val a: Foo.Bar = 42
                                                                                                                        
scala> val b = a.flip
val b: Foo.Bar = -42
                                                                                                                        
scala> val c: Foo.Bar = b.flip
val c: Foo.Bar = 42
                                                                                                                        
scala> val d = c.flip
val d: Foo.Bar = -42

It makes me wonder if it's actually a bug in the type printer that it prints Foo$#Bar as Foo.Bar, concealing the underlying inference bug from view. Like if Foo$#Bar actually printed as Foo$#Bar so we could see the wrong type, would any tests fail? 🤔

[SethTisue]

It seems peculiar to me that the compiler accepts this:

scala> val a = Foo.Bar(42)
val a: Foo.Bar = 42
                                                                                                                        
scala> val b0 = a.flip
val b0: Foo.Bar = -42
                                                                                                                        
scala> val b1 = a.flip
val b1: Foo.Bar = -42
                                                                                                                        
scala> b1: Foo.Bar
val res0: Foo.Bar = -42

if b1 has the too-general type Foo$#Bar, why does the ascription to the more specific type succeed? 🤔

[smarter]

possibly because of https://github.com/lampepfl/dotty/blob/8246a7b3dba68f27b12e7f9fd8ffaddf5e58440c/compiler/src/dotty/tools/dotc/core/TypeComparer.scala#L289-L291

[SethTisue]

ha, interesting

as Dale and I were discussing, perhaps we're being too harsh to classify Foo$#Bar as outright "wrong", if slop between Foo$#Bar and Foo.Bar is at least sometimes acceptable, as in the piece of code Guillaume shared... but regardless of whether Foo$#Bar is "wrong", the fact that b0 and b1 get different types at all is just crazy, clearly something has gone haywire that there's any discrepancy at all

we'll keep digging

[SethTisue]

as Dale notes, this seems like a resident compiler bug rather than a "REPL bug" per se, it's just that the REPL is the usual way a resident compiler gets involved

[dwijnand]

We suspect this has to do with stale denotations. From Tom's script we found out that not only do the types of val b and val b2 dealias differently, they also differ in "underlying" types, with val b having Foo.Bar as underlying type and val b2 having Foo$.this#Bar as underlying type. We suspect it might be a missing bit of cleanup in "bringForward" or "updateValidity" in denotations (which seem to be the new names for adaptToNewRun, in nsc).

In terms of process we eventually found a reasonable debugging setup in IntelliJ that allowed us to step through the logic in Typer or in objects, which was sometimes faster than println debugging.

Here's the branch where Seth and I almost lost our minds trying to force IntelliJ to set a break point: https://github.com/lampepfl/dotty/compare/master...dwijnand:opaque-extensions-on-life-support?expand=1

[SethTisue]

Seth and I almost lost our minds trying to force IntelliJ to set a break point

haha yeah if some of the diffs look like bizarre flailing that's why, in general the debugger works well, but there is a specific pain point in the Scala 3 support: sometimes it simply won't let you set a breakpoint on a certain line, and we couldn't tell what the conditions are for that, so then we'd semi-randomly rearrange the code until it would let us set one where we needed it

also conditional breakpoints didn't seem to be reliable but we just substituted "poor man's conditional breakpoints" where you add if (condition) print("") to the code and then breakpoint the print

[SethTisue]

a random tidbit is that once we've enabled tracing in some part of the compiler testCompilation becomes unusable because you end with a mountain of debug spew during compilation of scala3-library-bootstrapped, so we bypassed that by using scala3-compiler/testOnly dotty.tools.repl.ScriptedTests -- -Ddotty.tests.filter=i10044 instead

[griggt]

Let me attempt to explain what's happening here.

I'm going to focus on the observation above that b0 and b1 from the example code below get different types. It seems like that's where things first go wrong and the rest of the issues follow as a consequence.

scala> object Foo:                      // (Run 1)
  opaque type Bar = Int
  object Bar:
    extension (b: Bar) def flip: Bar = -b
    def apply(x: Int): Bar = x

scala> val a = Foo.Bar(42)              // (Run 2)

scala> val b0 = a.flip                  // (Run 3)

scala> val b1 = a.flip                  // (Run 4)

scala> val c = b1.flip                  // (Run 5)

Now let's look at what happens during the compilation runs, one for each "line" entered into the REPL.
The lines prefixed by >> are debugging printlns, and the numeric identifiers are hash codes.

In Run 1

A SymDenotation is created for the symbol Bar.

>> Creating SymDenotation [991559582] Bar: sym=NakedSymbol#6255, owner=module class Foo$,
    initInfo=class dotty.tools.dotc.typer.Namer$Completer

In Run 2

A TermRef is created for object Bar with prefix Foo.

>> Creating CachedTermRef {-1124047355} object Bar 
    prefix=TermRef(TermRef(ThisType(TypeRef(NoPrefix,module class <empty>)),object rs$line$1),object Foo)

In Run 3

During phase typer:

Implicit search is performed. The TermRef {-1124047355} is added as a companion while computing the implicit scope, and its denotation is computed. It is important to note here that because of the opaque type, the denotation info depends on the prefix, i.e. it needs asSeenFrom, and so a UniqueRefDenotation is created.

>> created UniqueRefDenotation [404858632] from SymDenotation [991559582] for object Bar
    initInfo = TypeRef(TermRef(TermRef(ThisType(TypeRef(NoPrefix,module class <empty>)),object rs$line$1),object Foo),module class Bar$)
    prefix = TermRef(TermRef(ThisType(TypeRef(NoPrefix,module class <empty>)),object rs$line$1),object Foo)
    initValidFor = Period(1..82, run = 3)

>> addCompanion sym=object Bar, 
      adding TermRef {-1124047355} TermRef(TermRef(TermRef(ThisType(TypeRef(NoPrefix,module class <empty>)),object rs$line$1),object Foo),object Bar)

>> NamedType#setDenot for {-1124047355} Bar to [404858632], validFor=Period(1..82, run = 3)

During phase genBCode:

At some point genLoad is called with tree=Ident(Bar). This eventually leads to a call to DottyBackendInterface#cachedDesugarIdent which in turn calls tpd.desugarIdent:

https://github.com/lampepfl/dotty/blob/8f99dad4664ff28b4d77e0ab0b1e6fe7c590524a/compiler/src/dotty/tools/dotc/ast/tpd.scala#L1376-L1380

>> tpd.desugarIdent tree=Ident(Bar) qual=Ident(Foo) tree.symbol=object Bar

The problem is with the subsequent call to qual.select:

https://github.com/lampepfl/dotty/blob/8f99dad4664ff28b4d77e0ab0b1e6fe7c590524a/compiler/src/dotty/tools/dotc/ast/tpd.scala#L885-L892

which calls TermRef.apply specifying a denotation computed using asSeenFrom.

The gotcha is that we are calling this from the backend, but after erasure, asSeenFrom is the identity! Even after ElimOpaque we would run into trouble here.

So in computing the asSeenFrom, SingleDenotation#computeAsSeenFrom calls SymDenotation#membersNeedAsSeenFrom on the owner, which is false here, because both:

• this.seesOpaques = false and
• ctx.erasedTypes = true

so the result of our asSeenFrom ends up being the original SymDenotation [991559582] rather than the UniqueRefDenotation [404858632].

Then the implementation of TermRef.apply in effect replaces the denotation on the existing CachedTermRef.

>> NamedType#setDenot for {-1124047355} Bar to [991559582], validFor=Period(1..82, run = 3)

Based on the validity period of this SymDenotation, it will be brought forward into the next run, and used during typer without being recomputed!

In Run 4:

During phase typer:

The SymDenotation [991559582] is used for the CachedTermRef {-1124047355} rather than the UniqueRefDenotation [404858632]. This results in b1 being mis-typed.

In Run 5:

During phase typer:

The mis-typing of b1 during run 4 prevents the proper scope being computed for implicit search, and the extension method flip is not found.

[smarter]

Based on the validity period of this SymDenotation, it will be brought forward into the next run, and used during typer without being recomputed!

That seems to be the real issue, the validity period shouldn't include phases where the denotation isn't actually valid, otherwise problems like this could pop up in any random place

[smarter]

Then the implementation of TermRef.apply in effect replaces the denotation on the existing CachedTermRef.

or perhaps that's the real culprit, I always found our usage of setDenot suspicious and this seems to confirm that we can't just assume that a TermRef with a specific denotation can stand in for any TermRef with the same prefix and designator, even if the denotation is still considered valid.

[odersky]

Yes, I think we need to handle travelling back in time wrt phases specially. A denotation established at a later phase cannot be re-used for the same NamedType in previous phases, even if the denotation is still valid.

[odersky]

@griggt Thank you for such a great diagnosis!