Merge pull request #601 from scala/backport-lts-3.3-23792

Backport "Prevent opaque types leaking from transparent inline methods" to 3.3 LTS

Author: tgodzik

compiler/src/dotty/tools/dotc/inlines/Inliner.scala

@@ -394,6 +394,11 @@ class Inliner(val call: tpd.Tree)(using Context):
       case (from, to) if from.symbol == ref.symbol && from =:= ref => to
     }
 
+  private def mapRefBack(ref: TermRef): Option[TermRef] =
+    opaqueProxies.collectFirst {
+      case (from, to) if to.symbol == ref.symbol && to =:= ref => from
+    }
+
   /** If `tp` contains TermRefs that refer to objects with opaque
    *  type aliases, add proxy definitions to `opaqueProxies` that expose these aliases.
    */
@@ -438,6 +443,22 @@ class Inliner(val call: tpd.Tree)(using Context):
           }
     )
 
+  /** Map back all TermRefs that match the right element in `opaqueProxies` to the
+   *  corresponding left element.
+   *  E.g. for a previously created
+   *   `val proxy$1 = Time {type OpaqueInt = Int}` as part of the ongoing inlining
+   *  a `List[proxy$1.OpaqueInt]` will be mapped back into a `List[Time.OpaqueInt]`.
+   */
+  protected val mapBackToOpaques = TreeTypeMap(
+    typeMap = new TypeMap:
+      override def stopAt = StopAt.Package
+      def apply(t: Type) = mapOver {
+        t match
+          case ref: TermRef => mapRefBack(ref).getOrElse(ref)
+          case _ => t
+      }
+  )
+
   /** If `binding` contains TermRefs that refer to objects with opaque
    *  type aliases, add proxy definitions that expose these aliases
    *  and substitute such TermRefs with theproxies. Example from pos/opaque-inline1.scala:
@@ -487,6 +508,48 @@ class Inliner(val call: tpd.Tree)(using Context):
 
   private def adaptToPrefix(tp: Type) = tp.asSeenFrom(inlineCallPrefix.tpe, inlinedMethod.owner)
 
+  def thisTypeProxyExists = !thisProxy.isEmpty
+
+  /** Maps a type that includes a thisProxy (e.g. `TermRef(NoPrefix,val Foo$_this)`)
+   *  by reading the defTree belonging to that thisProxy (`val Foo$_this: Foo.type = AnotherProxy`)
+   *  back into its original reference (`AnotherProxy`, which is directly or indirectly a refinement on `Foo`)
+   *
+   *  Usually when we end up with another proxy like this, we will be able to further unwrap it back
+   *  into `Foo` with mapBackToOpaques, but, for nested transparent inline calls, `AnotherProxy` will be
+   *  a proxy created by inlining the outer calls, that we might not be able to further unwrap this way
+   *  (as those proxies will not be a part of opaqueProxies created during this inlining).
+   *  We leave that as it is and treat this behavior as intended (see documentation with an example in
+   *  `Opaque Types in Transparent Inline Methods` section in `opaques-details.md`),
+   *  as we might need those opaques to have visible right hand sides for successful
+   *  typechecking of the outer inline call.
+   */
+  val thisTypeUnpacker =
+    TreeTypeMap(
+      typeMap = new TypeMap:
+        override def stopAt = StopAt.Package
+        def apply(t: Type) = mapOver {
+          t match
+            case a: TermRef if thisProxy.values.exists(_ == a) =>
+              a.termSymbol.defTree match
+                case untpd.ValDef(a, tpt, _) => tpt.tpe
+            case _ => t
+        }
+  )
+
+  /** Returns the result type of the Inlined code block after removing thisProxy and opaqueProxy TermRefs.
+   *  E.g. for an Inlined tree returning Block of type `Option[Foo$_this.OpaqueInt]`,
+   *  and for proxies:
+   *  ```
+   *  val $proxy1: Foo.type{type OpaqueInt = Int} = = ...
+   *  val Foo$_this: ($proxy1 : Foo.type{type OpaqueInt = Int}) = ...
+   *  ```
+   *  the method will return: `Foo.OpaqueInt`
+   */
+  def unpackProxiesFromResultType(inlined: Inlined): Type =
+    if thisTypeProxyExists then mapBackToOpaques.typeMap(thisTypeUnpacker.typeMap(inlined.expansion.tpe))
+    else inlined.tpe
+
+
   /** Populate `thisProxy` and `paramProxy` as follows:
    *
    *  1a. If given type refers to a static this, thisProxy binds it to corresponding global reference,

compiler/src/dotty/tools/dotc/inlines/Inlines.scala

@@ -532,12 +532,47 @@ object Inlines:
       // different for bindings from arguments and bindings from body.
       val inlined = tpd.Inlined(call, bindings, expansion)
 
-      if !hasOpaqueProxies then inlined
+      val hasOpaquesInResultFromCallWithTransparentContext =
+        val owners = call.symbol.ownersIterator.toSet
+        call.tpe.widenTermRefExpr.existsPart(
+          part => part.typeSymbol.is(Opaque) && owners.contains(part.typeSymbol.owner)
+        )
+
+      /** Remap ThisType nodes that are incorrect in the inlined context.
+       * Incorrect ThisType nodes can cause unwanted opaque type dealiasing later.
+       * E.g. if inlined in a `<root>.Foo` package (but outside of <root>.Foo.Bar object) we will map
+       *   `TermRef(ThisType(TypeRef(ThisType(TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),object Foo),Bar$)),MyOpaque$)),one)`
+       * into
+       *   `TermRef(TermRef(TermRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),object Foo),object Bar),object MyOpaque),val one)`
+       * See test i13461-d
+       */
+      def fixThisTypeModuleClassReferences(tpe: Type): Type =
+        val owners = ctx.owner.ownersIterator.toSet
+        TreeTypeMap(
+          typeMap = new TypeMap:
+            override def stopAt = StopAt.Package
+            def apply(t: Type) = mapOver {
+              t match
+                case ThisType(tref @ TypeRef(prefix, _)) if tref.symbol.flags.is(Module) && !owners.contains(tref.symbol) =>
+                  TermRef(apply(prefix), tref.symbol.companionModule)
+                case _ => mapOver(t)
+            }
+        ).typeMap(tpe)
+
+      if !hasOpaqueProxies && !hasOpaquesInResultFromCallWithTransparentContext then inlined
       else
-        val target =
-          if inlinedMethod.is(Transparent) then call.tpe & inlined.tpe
-          else call.tpe
-        inlined.ensureConforms(target)
+        val (target, forceCast) =
+          if inlinedMethod.is(Transparent) then
+            val unpacked = unpackProxiesFromResultType(inlined)
+            val withAdjustedThisTypes = if call.symbol.is(Macro) then fixThisTypeModuleClassReferences(unpacked) else unpacked
+            (call.tpe & withAdjustedThisTypes, withAdjustedThisTypes != unpacked)
+          else (call.tpe, false)
+        if forceCast then
+          // we need to force the cast for issues with ThisTypes, as ensureConforms will just
+          // check subtyping and then choose not to cast, leaving the previous, incorrect type
+          inlined.cast(target)
+        else
+          inlined.ensureConforms(target)
           // Make sure that the sealing with the declared type
           // is type correct. Without it we might get problems since the
           // expression's type is the opaque alias but the call's type is

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -102,6 +102,13 @@ object Typer {
    */
   private[typer] val HiddenSearchFailure = new Property.Key[List[SearchFailure]]
 
+
+  /** An attachment on a Typed node. Indicates that the Typed node was synthetically
+   *  inserted by the Typer phase. We might want to remove it for the purpose of inlining,
+   *  but only if it was not manually inserted by the user.
+   */
+  private[typer] val InsertedTyped = new Property.Key[Unit]
+
   /** Is tree a compiler-generated `.apply` node that refers to the
    *  apply of a function class?
    */
@@ -2579,7 +2586,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
     if sym.is(ExtensionMethod) then rhsCtx.addMode(Mode.InExtensionMethod)
     val rhs1 = PrepareInlineable.dropInlineIfError(sym,
       if sym.isScala2Macro then typedScala2MacroBody(ddef.rhs)(using rhsCtx)
-      else typedExpr(ddef.rhs, tpt1.tpe.widenExpr)(using rhsCtx))
+      else
+        typedExpr(ddef.rhs, tpt1.tpe.widenExpr)(using rhsCtx)) match
+          case typed @ Typed(outer, _) if typed.hasAttachment(InsertedTyped) => outer
+          case other => other
 
     if sym.isInlineMethod then
       if StagingLevel.level > 0 then

docs/_docs/reference/other-new-features/opaques-details.md

@@ -109,6 +109,45 @@ object obj:
 ```
 The opaque type alias `A` is transparent in its scope, which includes the definition of `x`, but not the definitions of `obj` and `y`.
 
+## Opaque Types in Transparent Inline Methods
+
+Additional care is required if an opaque type is returned from a transparent inline method, located inside a context where that opaque type is defined.
+Since the typechecking and type inference of the body of the method is done from the perspective of that context, the returned types might contain dealiased opaque types. Generally, this means that calls to those transparent methods will return a `DECLARED & ACTUAL`, where `DECLARED` is the return type defined in the method declaration, and `ACTUAL` is the type returned after the inlining, which might include dealiased opaque types.
+
+API designers can ensure that the correct type is returned by explicitly annotating it inside of the method body with `: ExpectedType` or by explicitly passing type parameters to the method being returned. Explicitly annotating like this will help for the outermost transparent inline method calls, but will not affect the nested calls, as, from the perspective of the new context into which we are inlining, those might still have to be dealiased to avoid compilation errors:
+
+```scala
+object Time:
+  opaque type Time = String
+  opaque type Seconds <: Time = String
+
+  // opaque type aliases have to be dealiased in nested calls,
+  // otherwise the resulting program might not be typed correctly
+  // in the below methods this will be typed as Seconds & String despite
+  // the explicit type declaration
+  transparent inline def sec(n: Double): Seconds =
+    s"${n}s": Seconds
+
+  transparent inline def testInference(): List[Time] =
+    List(sec(5)) // infers List[String] and returns List[Time] & List[String], not List[Seconds]
+  transparent inline def testGuarded(): List[Time] =
+    List(sec(5)): List[Seconds] // returns List[Seconds]
+  transparent inline def testExplicitTime(): List[Time] =
+    List[Seconds](sec(5)) // returns List[Seconds]
+  transparent inline def testExplicitString(): List[Time] =
+    List[String](sec(5)) // returns List[Time] & List[String]
+
+end Time
+
+@main def main() =
+  val t1: List[String] = Time.testInference() // returns List[Time.Time] & List[String]
+  val t2: List[Time.Seconds] = Time.testGuarded() // returns List[Time.Seconds]
+  val t3: List[Time.Seconds] = Time.testExplicitTime() // returns List[Time.Seconds]
+  val t4: List[String] = Time.testExplicitString() // returns List[Time.Time] & List[String]
+```
+
+Be careful especially if what is being inlined depends on the type of those nested transparent calls.
+```
 
 ## Relationship to SIP 35
 

tests/neg/i13461-a.scala

@@ -0,0 +1,9 @@
+package i13461:
+
+  opaque type Opaque = Int
+  transparent inline def op: Opaque = (123: Opaque)
+
+  object Main:
+    def main(args: Array[String]): Unit =
+      val o22: 123 = op // error
+

tests/pos-macros/i13461-d/Macro_1.scala

@@ -0,0 +1,8 @@
+import scala.quoted.*
+opaque type MyOpaque = Int
+object MyOpaque:
+  val one: MyOpaque = 1
+  transparent inline def apply(): MyOpaque = ${ applyMacro }
+  private def applyMacro(using Quotes): Expr[MyOpaque] =
+    import quotes.reflect.*
+    '{ one }

tests/pos-macros/i13461-d/Test_2.scala

@@ -0,0 +1,8 @@
+trait Leak[T]:
+  type Out
+given [T]: Leak[T] with
+  type Out = T
+extension [T](t: T)(using l: Leak[T]) def leak: l.Out = ???
+
+val x = MyOpaque().leak
+val shouldWork = summon[x.type <:< MyOpaque]

tests/pos/i13461-a.scala

@@ -0,0 +1,12 @@
+package i13461:
+
+  opaque type Opaque = Int
+  transparent inline def op: Opaque = 123
+  transparent inline def oop: i13461.Opaque = 123
+
+  object Main:
+    def main(args: Array[String]): Unit =
+      val o2: Opaque = op
+      val o3: Opaque = oop // needs to be unwrapped from Typed generated in adapt
+      val o22: 123 = op
+      val o23: 123 = oop

tests/pos/i13461-c.scala

@@ -0,0 +1,23 @@
+object Time:
+  opaque type Time = String
+  opaque type Seconds <: Time = String
+
+  transparent inline def sec(n: Double): Seconds =
+    s"${n}s": Seconds // opaque type aliases have to be dealiased in nested calls, otherwise the resulting program might not be typed correctly
+
+  transparent inline def testInference(): List[Time] =
+    List(sec(5)) // infers List[String] and returns List[Time] & List[String], not List[Seconds]
+  transparent inline def testGuarded(): List[Time] =
+    List(sec(5)): List[Seconds] // returns List[Seconds]
+  transparent inline def testExplicitTime(): List[Time] =
+    List[Seconds](sec(5)) // returns List[Seconds]
+  transparent inline def testExplicitString(): List[Time] =
+    List[String](sec(5)) // returns List[Time] & List[String]
+
+end Time
+
+@main def main() =
+  val t1: List[String] = Time.testInference() // returns List[Time.Time] & List[String]
+  val t2: List[Time.Seconds] = Time.testGuarded() // returns List[Time.Seconds]
+  val t3: List[Time.Seconds] = Time.testExplicitTime() // returns List[Time.Seconds]
+  val t4: List[String] = Time.testExplicitString() // returns List[Time.Time] & List[String]

tests/run/i13461-b.check

@@ -0,0 +1,4 @@
+35s
+35m
+15s, 20m, 15m, 20s
+15s, 15m, 15s, 20m