Prevent opaque types leaking from transparent inline methods

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.:
```
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: jchyb

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,19 @@ class Inliner(val call: tpd.Tree)(using Context):
           }
     )
 
+  /** Map back all TermRefs that match the right element in `opaqueProxies` to the
+   *  corresponding left element.
+   */
+  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 +505,28 @@ class Inliner(val call: tpd.Tree)(using Context):
 
   private def adaptToPrefix(tp: Type) = tp.asSeenFrom(inlineCallPrefix.tpe, inlinedMethod.owner)
 
+  def thisTypeProxyExists = !thisProxy.isEmpty
+
+  // Unpacks `val ObjectDef$_this: ObjectDef.type = ObjectDef` reference back into ObjectDef reference
+  // For nested transparent inline calls, ObjectDef will be an another proxy, but that is okay
+  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
+        }
+  )
+
+  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,10 +573,16 @@ object Inlines:
       // different for bindings from arguments and bindings from body.
       val inlined = tpd.Inlined(call, bindings, expansion)
 
-      if !hasOpaqueProxies then inlined
+      val hasOpaquesInResultFromCallWithTransparentContext =
+        call.tpe.widenTermRefExpr.existsPart(
+          part => part.typeSymbol.is(Opaque) && call.symbol.ownersIterator.contains(part.typeSymbol.owner)
+        )
+
+      if !hasOpaqueProxies && !hasOpaquesInResultFromCallWithTransparentContext then inlined
       else
         val target =
-          if inlinedMethod.is(Transparent) then call.tpe & inlined.tpe
+          if inlinedMethod.is(Transparent) then
+            call.tpe & unpackProxiesFromResultType(inlined)
           else call.tpe
         inlined.ensureConforms(target)
           // Make sure that the sealing with the declared type

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
@@ -4678,7 +4688,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.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/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/pos/i13461.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/run/i13461-b.check

@@ -0,0 +1,4 @@
+35.0s
+35.0m
+15.0s, 20.0m, 15.0m, 20.0s
+15.0s, 15.0m, 15.0s, 20.0m

tests/run/i13461-b.scala

@@ -0,0 +1,60 @@
+// TODO taken from issue
+object Time:
+  opaque type Time = String
+  opaque type Seconds <: Time = String
+  opaque type Minutes <: Time = String
+  opaque type Mixed <: Time = String
+
+  type Units = Seconds | Minutes
+
+  def sec(n: Double): Seconds =
+    s"${n}s"
+
+  def min(n: Double): Minutes =
+    s"${n}m"
+
+  def mixed(t1: Time, t2: Time): Mixed =
+    s"${t1}, ${t2}"
+
+  extension (t: Units)
+    def number: Double =
+      (t : String).init.toDouble
+
+  extension [T1 <: Time](inline a: T1)
+    transparent inline def +[T2 <: Time](inline b: T2): Time =
+      inline (a, b) match
+        case x: (Seconds, Seconds) =>
+          (sec(x._1.number + x._2.number))
+
+        case x: (Minutes, Minutes) =>
+          (min(x._1.number + x._2.number))
+
+        case x: (Time, Time) =>
+          (mixed(x._1, x._2))
+    end +
+end Time
+
+import Time.*
+
+// Test seconds
+val a = sec(15)
+val b = sec(20)
+
+// Test minutes
+val x = min(15)
+val y = min(20)
+
+// Test mixes
+val m1 = a + y
+val m2 = x + b
+
+// Test upper type
+val t1: Time = a
+val t2: Time = x
+val t3: Time = m1
+
+@main def Test() =
+  println(a + b)
+  println(x + y)
+  println(m1 + m2)
+  println(t1 + t2 + t3)