-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Prevent opaque types leaking from transparent inline methods #23792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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? | ||
*/ | ||
|
@@ -3031,7 +3038,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 | ||
Comment on lines
+3041
to
+3044
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part is connected to the i13461-a tests, where as an optimization the compiler added a Typed tree, which meant that we were not able to correctly handle result of the transparent tree, since we couldn't differentiate between Typed added by the compiler and Typed added by the user - the InsertedTyped exists for that purpose |
||
|
||
if sym.isInlineMethod then | ||
if StagingLevel.level > 0 then | ||
|
@@ -4690,7 +4700,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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
object Time: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test-case was made by me, based on i13461-b.scala. to show and test the behaviors of nested transparent calls. |
||
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] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
35s | ||
35m | ||
15s, 20m, 15m, 20s | ||
15s, 15m, 15s, 20m |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This, along with the withAdjustedThisTypes method, fixes the
i13461-d
test case. There, andIdent
was typed asTermRef(ThisType(TypeRef(ThisType(TypeRef(ThisType(TypeRef(ThisType(TypeRef(NoPrefix,module class <root>)),module class <empty>)),Macro_1$package$)),MyOpaque$)),one)
, and was then Inlined into a place where those ThisTypes stopped being valid. This caused the implicit resolution to incorrectly be able to see the opaque type rhs. To fix this, we make sure we don't refer incorrectly to objects as ThisTypes with the help of withAdjustedThisTypes method