Merge pull request #8061 from dotty-staging/disallow-phase-inconsistent-inline-params

Disallow phase inconsistent inline parameters

Author: nicolasstucki

community-build/community-projects/scalatest

@@ -88,8 +88,6 @@ class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages(
     tree match {
       case Quoted(_) | Spliced(_)  =>
         tree
-      case tree: RefTree if tree.symbol.isAllOf(InlineParam) =>
-        tree
       case _: This =>
         assert(checkSymLevel(tree.symbol, tree.tpe, tree.sourcePos).isEmpty)
         tree
@@ -197,10 +195,8 @@ class PCPCheckAndHeal(@constructorOnly ictx: Context) extends TreeMapWithStages(
     case Some(l) =>
       l == level ||
         level == -1 && (
-            // here we assume that Splicer.canBeSpliced was true before going to level -1,
-            // this implies that all non-inline arguments are quoted and that the following two cases are checked
-            // on inline parameters or type parameters.
-            sym.is(Param) ||
+            // here we assume that Splicer.checkValidMacroBody was true before going to level -1,
+            // this implies that all arguments are quoted.
             sym.isClass // reference to this in inline methods
           )
     case None =>

compiler/src/dotty/tools/dotc/transform/PCPCheckAndHeal.scala

@@ -52,6 +52,9 @@ object Splicer {
       catch {
         case ex: CompilationUnit.SuspendException =>
           throw ex
+        case ex: scala.quoted.StopQuotedContext if ctx.reporter.hasErrors =>
+           // errors have been emitted
+          EmptyTree
         case ex: StopInterpretation =>
           ctx.error(ex.msg, ex.pos)
           EmptyTree
@@ -389,6 +392,8 @@ object Splicer {
           throw new StopInterpretation(sw.toString, pos)
         case ex: InvocationTargetException =>
           ex.getTargetException match {
+            case ex: scala.quoted.StopQuotedContext =>
+              throw ex
             case MissingClassDefinedInCurrentRun(sym) =>
               if (ctx.settings.XprintSuspension.value)
                 ctx.echo(i"suspension triggered by a dependency on $sym", pos)

compiler/src/dotty/tools/dotc/transform/Splicer.scala

@@ -19,7 +19,6 @@ import dotty.tools.dotc.util.Spans._
 import dotty.tools.dotc.util.{Property, SourcePosition}
 import dotty.tools.dotc.transform.SymUtils._
 import dotty.tools.dotc.typer.Implicits.SearchFailureType
-import dotty.tools.dotc.typer.Inliner
 
 import scala.collection.mutable
 import scala.annotation.constructorOnly

compiler/src/dotty/tools/dotc/transform/TreeMapWithStages.scala

@@ -356,11 +356,11 @@ again together with a program that calls `assert`.
 ```scala
 object Macros {
 
-  inline def assert(expr: => Boolean): Unit =
+  inline def assert(inline expr: Boolean): Unit =
     ${ assertImpl('expr) }
 
   def assertImpl(expr: Expr[Boolean]) =
-    '{ if !($expr) then throw new AssertionError(s"failed assertion: ${$expr}") }
+    '{ if !($expr) then throw new AssertionError("failed assertion: " + ${expr.show}) }
 }
 
 object App {
@@ -414,33 +414,28 @@ assume that both definitions are local.
 
 The `inline` modifier is used to declare a `val` that is
 either a constant or is a parameter that will be a constant when instantiated. This
-aspect is also important for macro expansion.  To illustrate this,
-consider an implementation of the `power` function that makes use of a
-statically known exponent:
+aspect is also important for macro expansion.
+
+To get values out of expressions containing constants `Expr` provides the method
+`getValue` (or `value`). This will convert the `Expr[T]` into a `Some[T]` (or `T`) when the
+expression contains value. Otherwise it will retrun `None` (or emit an error).
+To avoid having incidental val bindings generated by the inlining of the `def`
+it is recommended to use an inline parameter. To illustrate this, consider an
+implementation of the `power` function that makes use of a statically known exponent:
 ```scala
-inline def power(inline n: Int, x: Double) = ${ powerCode(n, 'x) }
+inline def power(x: Double, inline n: Int) = ${ powerCode('x, 'n) }
+
+private def powerCode(x: Expr[Double], n: Expr[Int])(given QuoteContext): Expr[Double] =
+  n.getValue match
+    case Some(m) => powerCode(x, m)
+    case None => '{ Math.pow($x, $y) }
 
-private def powerCode(n: Int, x: Expr[Double]): Expr[Double] =
+private def powerCode(x: Expr[Double], n: Int)(given QuoteContext): Expr[Double] =
   if (n == 0) '{ 1.0 }
   else if (n == 1) x
-  else if (n % 2 == 0) '{ val y = $x * $x; ${ powerCode(n / 2, 'y) } }
-  else '{ $x * ${ powerCode(n - 1, x) } }
-```
-The reference to `n` as an argument in `${ powerCode(n, 'x) }` is not
-phase-consistent, since `n` appears in a splice without an enclosing
-quote. Normally that would be a problem because it means that we need
-the _value_ of `n` at compile time, which is not available for general
-parameters. But since `n` is an inline parameter of a macro, we know
-that at the macro’s expansion point `n` will be instantiated to a
-constant, so the value of `n` will in fact be known at this
-point. To reflect this, we loosen the phase consistency requirements
-as follows:
-
- - If `x` is a inline value (or a inline parameter of an inline
-   function) of type Boolean, Byte, Short, Int, Long, Float, Double,
-   Char or String, it can be accessed in all contexts where the number
-   of splices minus the number of quotes between use and definition
-   is either 0 or 1.
+  else if (n % 2 == 0) '{ val y = $x * $x; ${ powerCode('y, n / 2) } }
+  else '{ $x * ${ powerCode(x, n - 1) } }
+```
 
 ### Scope Extrusion
 
@@ -472,7 +467,7 @@ that invokation of `run` in splices. Consider the following expression:
 '{ (x: Int) => ${ run('x); 1 } }
 ```
 This is again phase correct, but will lead us into trouble. Indeed, evaluating
-the splice will reduce the expression `('x).run` to `x`. But then the result
+the splice will reduce the expression `run('x)` to `x`. But then the result
 
 ```scala
 '{ (x: Int) => ${ x; 1 } }
@@ -590,12 +585,12 @@ inline method that can calculate either a value of type `Int` or a value of type
 `String`.
 
 ```scala
-inline def defaultOf(inline str: String) <: Any = ${ defaultOfImpl(str) }
+inline def defaultOf(inline str: String) <: Any = ${ defaultOfImpl('str) }
 
-def defaultOfImpl(str: String): Expr[Any] = str match {
-  case "int" => '{1}
-  case "string" => '{"a"}
-}
+def defaultOfImpl(strExpr: Expr[String])(given QuoteContext): Expr[Any] =
+  strExpr.value match
+    case "int" => '{1}
+    case "string" => '{"a"}
 
 // in a separate file
 val a: Int = defaultOf("int")
@@ -624,8 +619,10 @@ It is possible to deconstruct or extract values out of `Expr` using pattern matc
 In `scala.quoted.matching` contains object that can help extract values from `Expr`.
 
 * `scala.quoted.matching.Const`: matches an expression a literal value and returns the value.
+* `scala.quoted.matching.Value`: matches an expression a value and returns the value.
 * `scala.quoted.matching.ExprSeq`: matches an explicit sequence of expresions and returns them. These sequences are useful to get individual `Expr[T]` out of a varargs expression of type `Expr[Seq[T]]`.
 * `scala.quoted.matching.ConstSeq`:  matches an explicit sequence of literal values and returns them.
+* `scala.quoted.matching.ValueSeq`:  matches an explicit sequence of values and returns them.
 
 These could be used in the following way to optimize any call to `sum` that has statically known values.
 ```scala
@@ -661,7 +658,7 @@ optimize {
 ```
 
 ```scala
-def sum(args: =>Int*): Int = args.sum
+def sum(args: Int*): Int = args.sum
 inline def optimize(arg: Int): Int = ${ optimizeExpr('arg) }
 private def optimizeExpr(body: Expr[Int])(given QuoteContext): Expr[Int] = body match {
   // Match a call to sum without any arguments
@@ -695,7 +692,7 @@ private def sumExpr(args1: Seq[Expr[Int]])(given QuoteContext): Expr[Int] = {
 Sometimes it is necessary to get a more precise type for an expression. This can be achived using the following pattern match.
 
 ```scala
-def f(exp: Expr[Any]) =
+def f(exp: Expr[Any])(given QuoteContext) =
   expr match
     case '{ $x: $t } =>
       // If the pattern match succeeds, then there is some type `T` such that

docs/docs/reference/metaprogramming/macros.md

@@ -227,8 +227,8 @@ private[quoted] object Matcher {
           case (While(cond1, body1), While(cond2, body2)) =>
             cond1 =?= cond2 && body1 =?= body2
 
-          case (New(tpt1), New(tpt2)) =>
-            tpt1 =?= tpt2
+          case (New(tpt1), New(tpt2)) if tpt1.tpe.typeSymbol == tpt2.tpe.typeSymbol =>
+            matched
 
           case (This(_), This(_)) if scrutinee.symbol == pattern.symbol =>
             matched

library/src/scala/internal/quoted/Matcher.scala

@@ -18,6 +18,14 @@ class Expr[+T] private[scala] {
    */
   final def getValue[U >: T](given qctx: QuoteContext, valueOf: ValueOfExpr[U]): Option[U] = valueOf(this)
 
+  /** Return the value of this expression.
+   *
+   *  Emits an error error and throws if the expression does not contain a value or contains side effects.
+   *  Otherwise returns the value.
+   */
+  final def value[U >: T](given qctx: QuoteContext, valueOf: ValueOfExpr[U]): U =
+    valueOf(this).getOrElse(qctx.throwError(s"Expected a known value. \n\nThe value of: $show\ncould not be recovered using $valueOf", this))
+
   /** Pattern matches `this` against `that`. Effectively performing a deep equality check.
    *  It does the equivalent of
    *  ```

library/src/scala/quoted/Expr.scala

@@ -22,7 +22,7 @@ class QuoteContext(val tasty: scala.tasty.Reflection) {
     tpe.unseal.show(syntaxHighlight)
   }
 
-  /** Report an error */
+  /** Report an error at the position of the macro expansion */
   def error(msg: => String): Unit = {
     import tasty.{_, given}
     tasty.error(msg, rootPosition)(given rootContext)
@@ -34,6 +34,17 @@ class QuoteContext(val tasty: scala.tasty.Reflection) {
     tasty.error(msg, expr.unseal.pos)(given rootContext)
   }
 
+  /** Report an error at the position of the macro expansion and throws a StopQuotedContext */
+  def throwError(msg: => String): Nothing = {
+    error(msg)
+    throw new StopQuotedContext
+  }
+  /** Report an error at the on the position of `expr` and throws a StopQuotedContext */
+  def throwError(msg: => String, expr: Expr[_]): Nothing = {
+    error(msg, expr)
+    throw new StopQuotedContext
+  }
+
   /** Report a warning */
   def warning(msg: => String): Unit = {
     import tasty.{_, given}

library/src/scala/quoted/QuoteContext.scala

@@ -0,0 +1,4 @@
+package scala.quoted
+
+/** Stop code generation after an error has been reported */
+class StopQuotedContext extends Throwable