Handle opaque types when inlining.

odersky · odersky · commit 279c185ac1f3 · 2021-06-14T23:48:49.000+02:00
The idea is described in part in the comments to PR #12815. 1. If a this-proxy definition contains references to classes that see opaque types, replace those references by other proxies that expose the opaque alias in a refinement type of the original reference type. 2. Make sure this proxies exist even for static moules containing opaque types, so that step 1 can be applied. 3. With steps 1 and 2, we can drop the restriction that inline methods may not be defined where opaque aliases are visible.
diff --git a/compiler/src/dotty/tools/dotc/typer/Inliner.scala b/compiler/src/dotty/tools/dotc/typer/Inliner.scala
@@ -556,18 +556,92 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
             ref(lastSelf).outerSelect(lastLevel - level, selfSym.info)
           else
             inlineCallPrefix
-      val binding = ValDef(selfSym.asTerm, QuoteUtils.changeOwnerOfTree(rhs, selfSym)).withSpan(selfSym.span)
+      val binding = accountForOpaques(
+        ValDef(selfSym.asTerm, QuoteUtils.changeOwnerOfTree(rhs, selfSym)).withSpan(selfSym.span))
       bindingsBuf += binding
       inlining.println(i"proxy at $level: $selfSym = ${bindingsBuf.last}")
       lastSelf = selfSym
       lastLevel = level
     }
   }
 
+  /** A list of pairs between TermRefs appearing in thisProxy bindings that
+   *  refer to objects with opaque type aliases and local proxy symbols
+   *  that contain refined versions of these TermRefs where the aliases
+   *  are exposed.
+   */
+  private val opaqueProxies = new mutable.ListBuffer[(TermRef, TermRef)]
+
+  /** Map first halfs of opaqueProxies pairs to second halfs, using =:= as equality */
+  def mapRef(ref: TermRef): Option[TermRef] =
+    opaqueProxies
+      .find((from, to) => from.symbol == ref.symbol && from =:= ref)
+      .map(_._2)
+
+  /** 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:
+   *
+   *  object refined:
+   *    opaque type Positive = Int
+   *    inline def Positive(value: Int): Positive = f(value)
+   *    def f(x: Positive): Positive = x
+   *  def run: Unit = { val x = 9; val nine = refined.Positive(x) }
+   *
+   *  This generates the following proxies:
+   *
+   *    val $proxy1: refined.type{type Positive = Int} =
+   *      refined.$asInstanceOf$[refined.type{type Positive = Int}]
+   *    val refined$_this: ($proxy1 : refined.type{Positive = Int}) =
+   *      $proxy1
+   *
+   *  and every reference to `refined` in the inlined expression is replaced by
+   *  `refined_$this`.
+   */
+  def accountForOpaques(binding: ValDef)(using Context): ValDef =
+    binding.symbol.info.foreachPart {
+      case ref: TermRef =>
+        for cls <- ref.widen.classSymbols do
+          if cls.containsOpaques && mapRef(ref).isEmpty then
+            def openOpaqueAliases(selfType: Type): List[(Name, Type)] = selfType match
+              case RefinedType(parent, rname, TypeAlias(alias)) =>
+                val opaq = cls.info.member(rname).symbol
+                if opaq.isOpaqueAlias then
+                  (rname, alias.stripLazyRef.asSeenFrom(ref, cls))
+                  :: openOpaqueAliases(parent)
+                else Nil
+              case _ =>
+                Nil
+            val refinements = openOpaqueAliases(cls.givenSelfType)
+            val refinedType = refinements.foldLeft(ref: Type) ((parent, refinement) =>
+              RefinedType(parent, refinement._1, TypeAlias(refinement._2))
+            )
+            val refiningSym = newSym(InlineBinderName.fresh(), Synthetic, refinedType).asTerm
+            val refiningDef = ValDef(refiningSym, tpd.ref(ref).cast(refinedType)).withSpan(binding.span)
+            inlining.println(i"add opaque alias proxy $refiningDef")
+            bindingsBuf += refiningDef
+            opaqueProxies += ((ref, refiningSym.termRef))
+      case _ =>
+    }
+    if opaqueProxies.isEmpty then binding
+    else
+      val mapType = new TypeMap:
+        override def stopAt = StopAt.Package
+        def apply(t: Type) = mapOver {
+          t match
+            case ref: TermRef => mapRef(ref).getOrElse(ref)
+            case _ => t
+        }
+      binding.symbol.info = mapType(binding.symbol.info)
+      val mapTree = TreeTypeMap(typeMap = mapType)
+      mapTree.transform(binding).asInstanceOf[ValDef]
+        .showing(i"transformed this binding exposing opaque aliases: $result", inlining)
+  end accountForOpaques
+
   private def canElideThis(tpe: ThisType): Boolean =
-    inlineCallPrefix.tpe == tpe && ctx.owner.isContainedIn(tpe.cls) ||
-    tpe.cls.isContainedIn(inlinedMethod) ||
-    tpe.cls.is(Package)
+    inlineCallPrefix.tpe == tpe && ctx.owner.isContainedIn(tpe.cls)
+    || tpe.cls.isContainedIn(inlinedMethod)
+    || tpe.cls.is(Package)
 
   /** Very similar to TreeInfo.isPureExpr, but with the following inliner-only exceptions:
    *  - synthetic case class apply methods, when the case class constructor is empty, are
@@ -666,12 +740,25 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
     case _ =>
   }
 
+  private val registerTypes = new TypeTraverser:
+    override def stopAt = StopAt.Package
+    // Only register ThisType prefixes that see opaques. No need to register the others
+    // since they are static prefixes.
+    def registerStaticPrefix(t: Type): Unit = t match
+      case t: ThisType if t.cls.seesOpaques => registerType(t)
+      case t: NamedType => registerStaticPrefix(t.prefix)
+      case _ =>
+    override def traverse(t: Type) = t match
+      case t: NamedType if t.currentSymbol.isStatic =>
+        registerStaticPrefix(t.prefix)
+      case t =>
+        registerType(t)
+        traverseChildren(t)
+
   /** Register type of leaf node */
-  private def registerLeaf(tree: Tree): Unit = tree match {
-    case _: This | _: Ident | _: TypeTree =>
-      tree.typeOpt.foreachPart(registerType, StopAt.Static)
+  private def registerLeaf(tree: Tree): Unit = tree match
+    case _: This | _: Ident | _: TypeTree => registerTypes.traverse(tree.typeOpt)
     case _ =>
-  }
 
   /** Make `tree` part of inlined expansion. This means its owner has to be changed
    *  from its `originalOwner`, and, if it comes from outside the inlined method
@@ -797,6 +884,8 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
     val inliner = new InlinerMap(
       typeMap =
         new DeepTypeMap {
+          override def stopAt =
+            if opaqueProxies.isEmpty then StopAt.Static else StopAt.Package
           def apply(t: Type) = t match {
             case t: ThisType => thisProxy.getOrElse(t.cls, t)
             case t: TypeRef => paramProxy.getOrElse(t, mapOver(t))
@@ -843,7 +932,16 @@ class Inliner(call: tpd.Tree, rhsToInline: tpd.Tree)(using Context) {
 
     // Apply inliner to `rhsToInline`, split off any implicit bindings from result, and
     // make them part of `bindingsBuf`. The expansion is then the tree that remains.
-    val expansion = inliner.transform(rhsToInline)
+    val expansion0 = inliner.transform(rhsToInline)
+    val expansion =
+      if opaqueProxies.nonEmpty && !inlinedMethod.is(Transparent) then
+        expansion0.cast(call.tpe)(using ctx.withSource(expansion0.source))
+          // the cast makes 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
+          // the opaque type itself. An example is in pos/opaque-inline1.scala.
+      else
+        expansion0
 
     def issueError() = callValueArgss match {
       case (msgArg :: Nil) :: Nil =>
diff --git a/compiler/src/dotty/tools/dotc/typer/PrepareInlineable.scala b/compiler/src/dotty/tools/dotc/typer/PrepareInlineable.scala
@@ -259,8 +259,6 @@ object PrepareInlineable {
     }
 
   private def checkInlineMethod(inlined: Symbol, body: Tree)(using Context): body.type = {
-    if (inlined.owner.isClass && inlined.owner.seesOpaques)
-      report.error(em"Implementation restriction: No inline methods allowed where opaque type aliases are in scope", inlined.srcPos)
     if Inliner.inInlineMethod(using ctx.outer) then
       report.error(ex"Implementation restriction: nested inline methods are not supported", inlined.srcPos)
 
diff --git a/tests/neg/i6662.scala b/tests/neg/i6662.scala
diff --git a/tests/pos/i6662.scala b/tests/pos/i6662.scala
@@ -0,0 +1,20 @@
+object opt:
+    opaque type Opt[A >: Null] = A
+    object Opt:
+        inline def unOpt[A >: Null](x: Opt[A]): A = x
+        inline def apply[A >: Null](x: A): Opt[A] = x
+        inline def some[A >: Null](x: A): Opt[A] = x
+        inline def none[A >: Null]: Opt[A] = null
+        inline def fromOption[A >: Null](x: Option[A]) = x.orNull
+
+import opt.Opt
+extension [A >: Null](x: Opt[A])
+    inline def nonEmpty : Boolean = x.get != null
+    inline def isEmpty  : Boolean = x.get == null
+    inline def isDefined: Boolean = x.nonEmpty
+    inline def get      : A       = Opt.unOpt(x)
+
+@main def Test =
+  val x: Opt[String] = Opt.some("abc")
+  assert(x.nonEmpty)
+  val y: String = Opt.unOpt(x)
diff --git a/tests/pos/i6854.scala b/tests/pos/i6854.scala
@@ -7,6 +7,6 @@ object Lib {
   opaque type IArray2[+T] = Array[_ <: T]
 
   object IArray2 {
-    inline def apply(x: =>Int): IArray2[Int] = Array(x) // error
+    inline def apply(x: =>Int): IArray2[Int] = Array(x)
   }
 }
diff --git a/tests/pos/inline3.scala b/tests/pos/inline3.scala
@@ -4,15 +4,13 @@ object K0 {
 
   opaque type ProductInstances[F[_], T] = ErasedProductInstances[F[T]]
 
-  inline def summonAsArray[F[_], T]: Array[Any] = ??? // error: Implementation restriction: No inline methods allowed
-
-  inline def mkProductInstances[F[_], T]: ProductInstances[F, T] = // error: Implementation restriction: No inline methods allowed
+  inline def summonAsArray[F[_], T]: Array[Any] = ???
+  inline def mkProductInstances[F[_], T]: ProductInstances[F, T] =
     new ErasedProductInstances(summonAsArray[F, T]).asInstanceOf[ProductInstances[F, T]]
 
   val x: T = ""
 
-  inline def foo(x: T): T = "foo".asInstanceOf[T] // error: Implementation restriction: No inline methods allowed
-
+  inline def foo(x: T): T = "foo".asInstanceOf[T]
 }
 
 final class ErasedProductInstances[FT](is0: => Array[Any])
@@ -21,7 +19,6 @@ trait Monoid[A]
 case class ISB(i: Int)
 
 object Test {
-  //val K0 = new K0
   K0.foo(K0.x)
   K0.mkProductInstances[Monoid, ISB]
 
diff --git a/tests/pos/opaque-inline.scala b/tests/pos/opaque-inline.scala
@@ -0,0 +1,20 @@
+
+object refined:
+  opaque type Positive = Int
+
+  object Positive extends PositiveFactory
+
+  trait PositiveFactory:
+    inline def apply(value: Int): Positive = value
+
+    def f(x: Positive): Positive = x
+    inline def fapply(value: Int): Positive =
+      val vv = (value, value) // error: implementation restriction
+      f(vv._1)
+
+@main def run: Unit =
+  import refined.*
+  val x = 9
+  val nine = Positive.apply(x)
+  val nine1 = Positive.fapply(x)
+
diff --git a/tests/pos/opaque-inline1-transparent.scala b/tests/pos/opaque-inline1-transparent.scala
@@ -0,0 +1,11 @@
+
+object refined:
+  opaque type Positive = Int
+  transparent inline def Positive(value: Int): Positive = f(value)
+  def f(x: Positive): Positive = x
+
+object test:
+  def run: Unit =
+    val x = 9
+    val nine = refined.Positive(x)
+
diff --git a/tests/pos/opaque-inline1.scala b/tests/pos/opaque-inline1.scala
@@ -0,0 +1,11 @@
+
+object refined:
+  opaque type Positive = Int
+  inline def Positive(value: Int): Positive = f(value)
+  def f(x: Positive): Positive = x
+
+object test:
+  def run: Unit =
+    val x = 9
+    val nine = refined.Positive(x)
+
diff --git a/tests/pos/opaque-inline2-transparent.scala b/tests/pos/opaque-inline2-transparent.scala
@@ -0,0 +1,28 @@
+
+import compiletime.*
+
+object refined:
+  opaque type Positive = Int
+
+  object Positive extends PositiveFactory
+
+  trait PositiveFactory:
+    transparent inline def apply(inline value: Int): Positive =
+      inline if value < 0 then error(codeOf(value) + " is not positive.")
+      else value
+
+    transparent inline def safe(value: Int): Positive | IllegalArgumentException =
+      if value < 0 then IllegalArgumentException(s"$value is not positive")
+      else value: Positive
+
+@main def Test: Unit =
+  import refined.*
+  val eight = Positive(8)
+  // val negative = Positive(-1) // This correctly produces a compile error "-1 is not positive."
+  // val random = Positive(scala.util.Random.nextInt()) // This correctly produces a compile error about being unable to inline the method call
+  val random = Positive.safe(scala.util.Random.nextInt())
+  val safeNegative = Positive.safe(-1)
+  val safeFive = Positive.safe(5)
+  println(eight)
+  println(random)
+  println(safeFive)
diff --git a/tests/pos/opaque-inline2.check b/tests/pos/opaque-inline2.check
@@ -0,0 +1,3 @@
+8
+java.lang.IllegalArgumentException: -1502782350 is not positive
+5
diff --git a/tests/pos/opaque-inline2.scala b/tests/pos/opaque-inline2.scala
@@ -0,0 +1,28 @@
+
+import compiletime.*
+
+object refined:
+  opaque type Positive = Int
+
+  object Positive extends PositiveFactory
+
+  trait PositiveFactory:
+    inline def apply(inline value: Int): Positive =
+      inline if value < 0 then error(codeOf(value) + " is not positive.")
+      else value
+
+    transparent inline def safe(value: Int): Positive | IllegalArgumentException =
+      if value < 0 then IllegalArgumentException(s"$value is not positive")
+      else value: Positive
+
+@main def Test: Unit =
+  import refined.*
+  val eight = Positive(8)
+  // val negative = Positive(-1) // This correctly produces a compile error "-1 is not positive."
+  // val random = Positive(scala.util.Random.nextInt()) // This correctly produces a compile error about being unable to inline the method call
+  val random = Positive.safe(scala.util.Random.nextInt())
+  val safeNegative = Positive.safe(-1)
+  val safeFive = Positive.safe(5)
+  println(eight)
+  println(random)
+  println(safeFive)
diff --git a/tests/run/opaque-inline/EmailAddress.scala b/tests/run/opaque-inline/EmailAddress.scala
@@ -0,0 +1,10 @@
+import scala.quoted.*
+
+opaque type EmailAddress = String
+object EmailAddress extends EmailAddressOps[EmailAddress]:
+
+  given (using s: ToExpr[String]): ToExpr[EmailAddress] = s
+
+  def parse(s: String): Either[String, EmailAddress] =
+    if (s contains "@") Right(s)
+    else Left("No @ symbol")
diff --git a/tests/run/opaque-inline/EmailAddressOps.scala b/tests/run/opaque-inline/EmailAddressOps.scala
@@ -0,0 +1,33 @@
+import scala.quoted.*
+
+trait EmailAddressOps[EmailAddressTransparent <: String]:
+
+  inline def apply(inline s: String): EmailAddress =
+    ${ EmailAddressOps.applyImpl('s) }
+
+  private val pattern = java.util.regex.Pattern.compile("([^@]*)@([^@]*)")
+
+  extension (value: EmailAddressTransparent)
+    inline def localPart: String =
+      val matcher = pattern.matcher(value: String)
+      matcher.matches
+      matcher.group(1)
+    inline def domainPart: String =
+      val matcher = pattern.matcher(value: String)
+      matcher.matches
+      matcher.group(2)
+
+object EmailAddressOps {
+  def applyImpl(expr: Expr[String])(using Quotes): Expr[EmailAddress] =
+    import quotes.reflect.*
+    expr.asTerm match
+      case Inlined(_, _, Literal(StringConstant(s))) =>
+        EmailAddress.parse(s) match
+          case Right(email) => Expr(email)
+          case Left(err) =>
+            report.error(s"Not a valid email address: $err", expr)
+            '{???}
+      case _ =>
+        report.error(s"Not a constant", expr)
+        '{???}
+}
diff --git a/tests/run/opaque-inline/Test.scala b/tests/run/opaque-inline/Test.scala
@@ -0,0 +1,2 @@
+@main def Test =
+  (new TestEmail).run
diff --git a/tests/run/opaque-inline/TestEmail.scala b/tests/run/opaque-inline/TestEmail.scala
diff --git a/tests/run/opaque-inline2.scala b/tests/run/opaque-inline2.scala

Original file line number	Diff line number	Diff line change
`@@ -259,8 +259,6 @@ object PrepareInlineable {`
`259`	`259`	`}`
`260`	`260`
`261`	`261`	`private def checkInlineMethod(inlined: Symbol, body: Tree)(using Context): body.type = {`
`262`		`- if (inlined.owner.isClass && inlined.owner.seesOpaques)`
`263`		`- report.error(em"Implementation restriction: No inline methods allowed where opaque type aliases are in scope", inlined.srcPos)`
`264`	`262`	`if Inliner.inInlineMethod(using ctx.outer) then`
`265`	`263`	`report.error(ex"Implementation restriction: nested inline methods are not supported", inlined.srcPos)`
`266`	`264`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,6 @@ object Lib {`
`7`	`7`	`opaque type IArray2[+T] = Array[_ <: T]`
`8`	`8`
`9`	`9`	`object IArray2 {`
`10`		`- inline def apply(x: =>Int): IArray2[Int] = Array(x) // error`
	`10`	`+ inline def apply(x: =>Int): IArray2[Int] = Array(x)`
`11`	`11`	`}`
`12`	`12`	`}`