From 8aa15e846c236c1950ed22a61798f445fc85c3b2 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Thu, 21 Aug 2025 19:39:23 +0200 Subject: [PATCH 1/2] Prevent opaque types leaking from transparent inline methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../dotty/tools/dotc/inlines/Inliner.scala | 40 +++++++++++++ .../dotty/tools/dotc/inlines/Inlines.scala | 10 +++- .../src/dotty/tools/dotc/typer/Typer.scala | 14 ++++- .../other-new-features/opaques-details.md | 39 ++++++++++++ tests/neg/i13461.scala | 9 +++ tests/pos/i13461-c.scala | 23 +++++++ tests/pos/i13461.scala | 12 ++++ tests/run/i13461-b.check | 4 ++ tests/run/i13461-b.scala | 60 +++++++++++++++++++ 9 files changed, 207 insertions(+), 4 deletions(-) create mode 100644 tests/neg/i13461.scala create mode 100644 tests/pos/i13461-c.scala create mode 100644 tests/pos/i13461.scala create mode 100644 tests/run/i13461-b.check create mode 100644 tests/run/i13461-b.scala diff --git a/compiler/src/dotty/tools/dotc/inlines/Inliner.scala b/compiler/src/dotty/tools/dotc/inlines/Inliner.scala index 2b2f012c688c..f61d85c3a18f 100644 --- a/compiler/src/dotty/tools/dotc/inlines/Inliner.scala +++ b/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, diff --git a/compiler/src/dotty/tools/dotc/inlines/Inlines.scala b/compiler/src/dotty/tools/dotc/inlines/Inlines.scala index 59386dd9bd4d..802db5bf68b0 100644 --- a/compiler/src/dotty/tools/dotc/inlines/Inlines.scala +++ b/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 diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index def6fac0556e..1f0908b29d17 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/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 diff --git a/docs/_docs/reference/other-new-features/opaques-details.md b/docs/_docs/reference/other-new-features/opaques-details.md index d285ec8e8325..e6c2ff80649e 100644 --- a/docs/_docs/reference/other-new-features/opaques-details.md +++ b/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 diff --git a/tests/neg/i13461.scala b/tests/neg/i13461.scala new file mode 100644 index 000000000000..70ec689601f1 --- /dev/null +++ b/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 + diff --git a/tests/pos/i13461-c.scala b/tests/pos/i13461-c.scala new file mode 100644 index 000000000000..ee5eff463b5a --- /dev/null +++ b/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] diff --git a/tests/pos/i13461.scala b/tests/pos/i13461.scala new file mode 100644 index 000000000000..6391681c07b1 --- /dev/null +++ b/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 diff --git a/tests/run/i13461-b.check b/tests/run/i13461-b.check new file mode 100644 index 000000000000..62c33240b000 --- /dev/null +++ b/tests/run/i13461-b.check @@ -0,0 +1,4 @@ +35s +35m +15s, 20m, 15m, 20s +15s, 15m, 15s, 20m diff --git a/tests/run/i13461-b.scala b/tests/run/i13461-b.scala new file mode 100644 index 000000000000..e384168158d2 --- /dev/null +++ b/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: Int): Seconds = + s"${n}s" + + def min(n: Int): Minutes = + s"${n}m" + + def mixed(t1: Time, t2: Time): Mixed = + s"${t1}, ${t2}" + + extension (t: Units) + def number: Int = + (t : String).init.toInt + + 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) From 1b2c3fbb72011684c9d3a4082ef381848a24dff5 Mon Sep 17 00:00:00 2001 From: Jan Chyb Date: Tue, 26 Aug 2025 15:19:46 +0200 Subject: [PATCH 2/2] Fix additional test case Certain macros could return nodes typed with incorrect ThisTypes, which would reference module types outside of their scope. We remap those problematic nodes to TermRefs pointing to objects, and then possibly manually cast the returned node to the remapped type, as the ensureConforms method would just leave the previous incorrect type after confirming that the remapped type is a subtype of the previous incorrect one. --- .../dotty/tools/dotc/inlines/Inlines.scala | 36 ++++++++++++++++--- tests/pos-macros/i13461-d/Macro_1.scala | 8 +++++ tests/pos-macros/i13461-d/Test_2.scala | 8 +++++ 3 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 tests/pos-macros/i13461-d/Macro_1.scala create mode 100644 tests/pos-macros/i13461-d/Test_2.scala diff --git a/compiler/src/dotty/tools/dotc/inlines/Inlines.scala b/compiler/src/dotty/tools/dotc/inlines/Inlines.scala index 802db5bf68b0..b34a3a6c9a71 100644 --- a/compiler/src/dotty/tools/dotc/inlines/Inlines.scala +++ b/compiler/src/dotty/tools/dotc/inlines/Inlines.scala @@ -574,17 +574,43 @@ object Inlines: val inlined = tpd.Inlined(call, bindings, expansion) val hasOpaquesInResultFromCallWithTransparentContext = + val owners = call.symbol.ownersIterator.toSet call.tpe.widenTermRefExpr.existsPart( - part => part.typeSymbol.is(Opaque) && call.symbol.ownersIterator.contains(part.typeSymbol.owner) + 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 + * See test i113461-d + * */ + def fixThisTypeModuleClassReferences(tpe: Type): Type = + val owners = ctx.owner.ownersIterator.toSet + println("owners " + owners) + 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 = + val (target, forceCast) = if inlinedMethod.is(Transparent) then - call.tpe & unpackProxiesFromResultType(inlined) - else call.tpe - inlined.ensureConforms(target) + 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 diff --git a/tests/pos-macros/i13461-d/Macro_1.scala b/tests/pos-macros/i13461-d/Macro_1.scala new file mode 100644 index 000000000000..52e83cee0d2a --- /dev/null +++ b/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 } diff --git a/tests/pos-macros/i13461-d/Test_2.scala b/tests/pos-macros/i13461-d/Test_2.scala new file mode 100644 index 000000000000..11202540f107 --- /dev/null +++ b/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] \ No newline at end of file