Prevent opaque types leaking from transparent inline methods (#23792)

Fixes #13461
Before this PR, every transparent inline call returning an opaque type
would actually be typed with an intersection type DECLARED & ACTUAL,
where DECLARED was the declared return type of the transparent method,
and ACTUAL was the type actually returned in the expansion, with every
opaque type alias then dealiased. There was no way to guard against this
dealiasing. With the changes in this PR, users are now able to manually
ensure that they receive the types they want, although they might have
to manually annotate the returned type inside of the transparent inline
method body (as described in the added documentation section).

The previous dealiasing was caused by the proxy mechanism in inlining,
which would effectively deals every opaque type, that is transparent
from the perspective of the original method declaration. Now, we try to
map the results of the transparent inline back to the original (opaque)
types.

However all of this is only true for the outermost transparent inline
method calls. Nested calls will not be affected by this change. This is
because the type checker in the original context of the method will see
the opaque type as transparent (so it will type the rest of the method
according to that), and that typing must still hold after inlining the
method e.g.:

```scala
object Time:
  opaque type Time = String
  transparent inline makeTime(): Time = "1h"
  transparent inline listTime(): List[Time] = List[String](makeTime())
    // mapping the results of makeTime() back into opaque types outside
    // of the scope of Time will cause an inlining compilation error
    // (which we are generally trying to avoid, and which would be
    // considered a bug in the compiler).
```
This might cause the aliased type to still leak in a manner that may
feel unexpected. In the above example, even if the List does not have an
explicit type parameter, the type inference will still decide on
`String`, causing any call to listTime to leak that type. This is also
touched upon in the added docs.

This PR might cause some source/library incompatibilities connected to
the changed returned types (but I doubt it’s many, considering the
additional required effort of ignoring type inference if we want the
outputted type to be different).

Author: odersky

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

@@ -573,12 +573,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

@@ -109,6 +109,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?
    */
@@ -3029,7 +3036,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
     val rhs1 = excludeDeferredGiven(ddef.rhs, sym): rhs =>
       PrepareInlineable.dropInlineIfError(sym,
         if sym.isScala2Macro then typedScala2MacroBody(rhs)(using rhsCtx)
-        else typedExpr(rhs, tpt1.tpe.widenExpr)(using rhsCtx))
+        else
+          typedExpr(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
@@ -4694,7 +4704,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
             insertGadtCast(tree, wtp, pt)
           case CompareResult.OKwithOpaquesUsed if !tree.tpe.frozen_<:<(pt)(using ctx.withOwner(defn.RootClass)) =>
             // guard to avoid extra Typed trees, eg. from testSubType(O.T, O.T) which returns OKwithOpaquesUsed
-            Typed(tree, TypeTree(pt))
+            Typed(tree, TypeTree(pt)).withAttachment(InsertedTyped, ())
           case _ =>
             //typr.println(i"OK ${tree.tpe}\n${TypeComparer.explained(_.isSubType(tree.tpe, pt))}") // uncomment for unexpected successes
             tree

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