Skip to content

Commit 04adb37

Browse files
committed
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).
1 parent 0a7f843 commit 04adb37

File tree

9 files changed

+207
-4
lines changed

9 files changed

+207
-4
lines changed

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,11 @@ class Inliner(val call: tpd.Tree)(using Context):
394394
case (from, to) if from.symbol == ref.symbol && from =:= ref => to
395395
}
396396

397+
private def mapRefBack(ref: TermRef): Option[TermRef] =
398+
opaqueProxies.collectFirst {
399+
case (from, to) if to.symbol == ref.symbol && to =:= ref => from
400+
}
401+
397402
/** If `tp` contains TermRefs that refer to objects with opaque
398403
* type aliases, add proxy definitions to `opaqueProxies` that expose these aliases.
399404
*/
@@ -438,6 +443,19 @@ class Inliner(val call: tpd.Tree)(using Context):
438443
}
439444
)
440445

446+
/** Map back all TermRefs that match the right element in `opaqueProxies` to the
447+
* corresponding left element.
448+
*/
449+
protected val mapBackToOpaques = TreeTypeMap(
450+
typeMap = new TypeMap:
451+
override def stopAt = StopAt.Package
452+
def apply(t: Type) = mapOver {
453+
t match
454+
case ref: TermRef => mapRefBack(ref).getOrElse(ref)
455+
case _ => t
456+
}
457+
)
458+
441459
/** If `binding` contains TermRefs that refer to objects with opaque
442460
* type aliases, add proxy definitions that expose these aliases
443461
* and substitute such TermRefs with theproxies. Example from pos/opaque-inline1.scala:
@@ -487,6 +505,28 @@ class Inliner(val call: tpd.Tree)(using Context):
487505

488506
private def adaptToPrefix(tp: Type) = tp.asSeenFrom(inlineCallPrefix.tpe, inlinedMethod.owner)
489507

508+
def thisTypeProxyExists = !thisProxy.isEmpty
509+
510+
// Unpacks `val ObjectDef$_this: ObjectDef.type = ObjectDef` reference back into ObjectDef reference
511+
// For nested transparent inline calls, ObjectDef will be an another proxy, but that is okay
512+
val thisTypeUnpacker =
513+
TreeTypeMap(
514+
typeMap = new TypeMap:
515+
override def stopAt = StopAt.Package
516+
def apply(t: Type) = mapOver {
517+
t match
518+
case a: TermRef if thisProxy.values.exists(_ == a) =>
519+
a.termSymbol.defTree match
520+
case untpd.ValDef(a, tpt, _) => tpt.tpe
521+
case _ => t
522+
}
523+
)
524+
525+
def unpackProxiesFromResultType(inlined: Inlined): Type =
526+
if thisTypeProxyExists then mapBackToOpaques.typeMap(thisTypeUnpacker.typeMap(inlined.expansion.tpe))
527+
else inlined.tpe
528+
529+
490530
/** Populate `thisProxy` and `paramProxy` as follows:
491531
*
492532
* 1a. If given type refers to a static this, thisProxy binds it to corresponding global reference,

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,10 +573,16 @@ object Inlines:
573573
// different for bindings from arguments and bindings from body.
574574
val inlined = tpd.Inlined(call, bindings, expansion)
575575

576-
if !hasOpaqueProxies then inlined
576+
val hasOpaquesInResultFromCallWithTransparentContext =
577+
call.tpe.widenTermRefExpr.existsPart(
578+
part => part.typeSymbol.is(Opaque) && call.symbol.ownersIterator.contains(part.typeSymbol.owner)
579+
)
580+
581+
if !hasOpaqueProxies && !hasOpaquesInResultFromCallWithTransparentContext then inlined
577582
else
578583
val target =
579-
if inlinedMethod.is(Transparent) then call.tpe & inlined.tpe
584+
if inlinedMethod.is(Transparent) then
585+
call.tpe & unpackProxiesFromResultType(inlined)
580586
else call.tpe
581587
inlined.ensureConforms(target)
582588
// Make sure that the sealing with the declared type

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,13 @@ object Typer {
109109
*/
110110
private[typer] val HiddenSearchFailure = new Property.Key[List[SearchFailure]]
111111

112+
113+
/** An attachment on a Typed node. Indicates that the Typed node was synthetically
114+
* inserted by the Typer phase. We might want to remove it for the purpose of inlining,
115+
* but only if it was not manually inserted by the user.
116+
*/
117+
private[typer] val InsertedTyped = new Property.Key[Unit]
118+
112119
/** Is tree a compiler-generated `.apply` node that refers to the
113120
* apply of a function class?
114121
*/
@@ -3029,7 +3036,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
30293036
val rhs1 = excludeDeferredGiven(ddef.rhs, sym): rhs =>
30303037
PrepareInlineable.dropInlineIfError(sym,
30313038
if sym.isScala2Macro then typedScala2MacroBody(rhs)(using rhsCtx)
3032-
else typedExpr(rhs, tpt1.tpe.widenExpr)(using rhsCtx))
3039+
else
3040+
typedExpr(rhs, tpt1.tpe.widenExpr)(using rhsCtx)) match
3041+
case typed @ Typed(outer, _) if typed.hasAttachment(InsertedTyped) => outer
3042+
case other => other
30333043

30343044
if sym.isInlineMethod then
30353045
if StagingLevel.level > 0 then
@@ -4678,7 +4688,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
46784688
insertGadtCast(tree, wtp, pt)
46794689
case CompareResult.OKwithOpaquesUsed if !tree.tpe.frozen_<:<(pt)(using ctx.withOwner(defn.RootClass)) =>
46804690
// guard to avoid extra Typed trees, eg. from testSubType(O.T, O.T) which returns OKwithOpaquesUsed
4681-
Typed(tree, TypeTree(pt))
4691+
Typed(tree, TypeTree(pt)).withAttachment(InsertedTyped, ())
46824692
case _ =>
46834693
//typr.println(i"OK ${tree.tpe}\n${TypeComparer.explained(_.isSubType(tree.tpe, pt))}") // uncomment for unexpected successes
46844694
tree

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,45 @@ object obj:
109109
```
110110
The opaque type alias `A` is transparent in its scope, which includes the definition of `x`, but not the definitions of `obj` and `y`.
111111

112+
## Opaque Types in Transparent Inline Methods
113+
114+
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.
115+
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.
116+
117+
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:
118+
119+
```scala
120+
object Time:
121+
opaque type Time = String
122+
opaque type Seconds <: Time = String
123+
124+
// opaque type aliases have to be dealiased in nested calls,
125+
// otherwise the resulting program might not be typed correctly
126+
// in the below methods this will be typed as Seconds & String despite
127+
// the explicit type declaration
128+
transparent inline def sec(n: Double): Seconds =
129+
s"${n}s": Seconds
130+
131+
transparent inline def testInference(): List[Time] =
132+
List(sec(5)) // infers List[String] and returns List[Time] & List[String], not List[Seconds]
133+
transparent inline def testGuarded(): List[Time] =
134+
List(sec(5)): List[Seconds] // returns List[Seconds]
135+
transparent inline def testExplicitTime(): List[Time] =
136+
List[Seconds](sec(5)) // returns List[Seconds]
137+
transparent inline def testExplicitString(): List[Time] =
138+
List[String](sec(5)) // returns List[Time] & List[String]
139+
140+
end Time
141+
142+
@main def main() =
143+
val t1: List[String] = Time.testInference() // returns List[Time.Time] & List[String]
144+
val t2: List[Time.Seconds] = Time.testGuarded() // returns List[Time.Seconds]
145+
val t3: List[Time.Seconds] = Time.testExplicitTime() // returns List[Time.Seconds]
146+
val t4: List[String] = Time.testExplicitString() // returns List[Time.Time] & List[String]
147+
```
148+
149+
Be careful especially if what is being inlined depends on the type of those nested transparent calls.
150+
```
112151
113152
## Relationship to SIP 35
114153

tests/neg/i13461.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package i13461:
2+
3+
opaque type Opaque = Int
4+
transparent inline def op: Opaque = (123: Opaque)
5+
6+
object Main:
7+
def main(args: Array[String]): Unit =
8+
val o22: 123 = op // error
9+

tests/pos/i13461-c.scala

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
object Time:
2+
opaque type Time = String
3+
opaque type Seconds <: Time = String
4+
5+
transparent inline def sec(n: Double): Seconds =
6+
s"${n}s": Seconds // opaque type aliases have to be dealiased in nested calls, otherwise the resulting program might not be typed correctly
7+
8+
transparent inline def testInference(): List[Time] =
9+
List(sec(5)) // infers List[String] and returns List[Time] & List[String], not List[Seconds]
10+
transparent inline def testGuarded(): List[Time] =
11+
List(sec(5)): List[Seconds] // returns List[Seconds]
12+
transparent inline def testExplicitTime(): List[Time] =
13+
List[Seconds](sec(5)) // returns List[Seconds]
14+
transparent inline def testExplicitString(): List[Time] =
15+
List[String](sec(5)) // returns List[Time] & List[String]
16+
17+
end Time
18+
19+
@main def main() =
20+
val t1: List[String] = Time.testInference() // returns List[Time.Time] & List[String]
21+
val t2: List[Time.Seconds] = Time.testGuarded() // returns List[Time.Seconds]
22+
val t3: List[Time.Seconds] = Time.testExplicitTime() // returns List[Time.Seconds]
23+
val t4: List[String] = Time.testExplicitString() // returns List[Time.Time] & List[String]

tests/pos/i13461.scala

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package i13461:
2+
3+
opaque type Opaque = Int
4+
transparent inline def op: Opaque = 123
5+
transparent inline def oop: i13461.Opaque = 123
6+
7+
object Main:
8+
def main(args: Array[String]): Unit =
9+
val o2: Opaque = op
10+
val o3: Opaque = oop // needs to be unwrapped from Typed generated in adapt
11+
val o22: 123 = op
12+
val o23: 123 = oop

tests/run/i13461-b.check

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
35s
2+
35m
3+
15s, 20m, 15m, 20s
4+
15s, 15m, 15s, 20m

tests/run/i13461-b.scala

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// TODO taken from issue
2+
object Time:
3+
opaque type Time = String
4+
opaque type Seconds <: Time = String
5+
opaque type Minutes <: Time = String
6+
opaque type Mixed <: Time = String
7+
8+
type Units = Seconds | Minutes
9+
10+
def sec(n: Int): Seconds =
11+
s"${n}s"
12+
13+
def min(n: Int): Minutes =
14+
s"${n}m"
15+
16+
def mixed(t1: Time, t2: Time): Mixed =
17+
s"${t1}, ${t2}"
18+
19+
extension (t: Units)
20+
def number: Int =
21+
(t : String).init.toInt
22+
23+
extension [T1 <: Time](inline a: T1)
24+
transparent inline def +[T2 <: Time](inline b: T2): Time =
25+
inline (a, b) match
26+
case x: (Seconds, Seconds) =>
27+
(sec(x._1.number + x._2.number))
28+
29+
case x: (Minutes, Minutes) =>
30+
(min(x._1.number + x._2.number))
31+
32+
case x: (Time, Time) =>
33+
(mixed(x._1, x._2))
34+
end +
35+
end Time
36+
37+
import Time.*
38+
39+
// Test seconds
40+
val a = sec(15)
41+
val b = sec(20)
42+
43+
// Test minutes
44+
val x = min(15)
45+
val y = min(20)
46+
47+
// Test mixes
48+
val m1 = a + y
49+
val m2 = x + b
50+
51+
// Test upper type
52+
val t1: Time = a
53+
val t2: Time = x
54+
val t3: Time = m1
55+
56+
@main def Test() =
57+
println(a + b)
58+
println(x + y)
59+
println(m1 + m2)
60+
println(t1 + t2 + t3)

0 commit comments

Comments
 (0)