Skip to content

Commit

Permalink
Inhibit typer to insert contextual arguments when it is inside argume…
Browse files Browse the repository at this point in the history
…nts of HOAS patterns (#18040)

This will close #17905.

## Current behavior and issue

Consider the following macro:

```scala
// Macro_1.scala
import scala.quoted.*

inline def testCtxParam(inline body: Any) = ${ testCtxParamImpl('body) }
def testCtxParamImpl(body: Expr[Any])(using Quotes): Expr[String] =
  body match
    case '{ def g(using s: String) = "placeholder"; $a(g): String } =>
      '{ $a((s: String) => s"(inside ${s})") }
    case _ => Expr("not matched")
```

```scala
// Test_2.scala
@main def Test: Unit =
  given String = "given"
  println(testCtxParam { def f(using t: String) = "placeholder"; f + " outside" })
```

In theory, running this code should allow the quote '{ def f(using t:
String)... } to match against the first clause of testCtxParamImpl,
binding $a to '{ g => g + " outside"}. As such, we'd expect Test_2.scala
to output (inside given) outside.

However, compiling Macro_1.scala results in the following error:

```
-- Error: macro_1.scala:6:56 ---------------------------------------------------
6 |    case '{ def g(using s: String) = "placeholder"; $a(g): String } =>
  |                                                        ^
  |    No given instance of type String was found for parameter s of method g
1 error found
```

The issue stems from the method symbol `g` in the HOAS pattern `$a(g)`.
Here, `g` should represent a symbol that can appear in the pattern
variable `$a`. It's not intended to mean a method call, yet the compiler
treats it as such, attempts to insert explicit contextual arguments, and
fails.

## Approach to fix this issue
It is `Typer.adaptNoArgs` that inserts explicit contextual arguments. I
added the following condition
`!ctx.mode.is(Mode.InQuotePatternHoasArgs)` to prevent it from inserting
contextual arguments.


https://github.com/lampepfl/dotty/pull/18040/files#diff-8c9ece1772bd78160fc1c31e988664586c9df566a1d22ff99ef99dd6d5627a90R4064

`Mode.InQuotePatternHoasArgs` is a new mode for typing arguments of a
HOAS pattern. This solution works, as all existing tests have passed.
However, considering that the number of Modes is limited to 32 in the
current implementation, it might not be the most optimal approach.

## Discussion: Matching against contextual/implicit methods
An aspect to consider is how the quote pattern match should treat
normal/contextual/implicit methods. For instance, consider this macro:

```scala
import scala.quoted.

inline def testMethods(inline body: Any) = ${ testMethodsImpl('body) }
def testMethodsImpl(body: Expr[Any])(using Quotes): Expr[String] =
  body match
    case '{ given i : Int = 0; def g(s: Int) = "placeholder"; g(i) } =>
      Expr("matched normal method")
    case '{ given i : Int = 0; def g(using s: Int) = "placeholder"; g } =>
      Expr("matched contextual method")
    case '{ given i : Int = 0; def g(implicit s: Int) = "placeholder"; g } =>
      Expr("matched implicit method")
    case _ => Expr("not matched")
```

If we run `testMethods { given Int = 0; def f(implicit s: Int) =
"placeholder"; f }`, how should it be handled?

If pattern matching is done exactly, it should match the third pattern.
However, given the similar behavior of using and implicit, it could
reasonably match the second pattern. Alternatively, the pattern matcher
can forget any information about context parameters, matching the first
pattern -- which is the current behavior.

In the current implementation (even without this fix), `testMethods {
given Int = 0; def f(implicit s: Any) = "placeholder"; f(10) }` expands
to `"matched normal method"`. This suggests that quote pattern matching
disregards whether method parameters are contextual or not.

This behavior has its merits; it removes the need to provide different
patterns to match both normal and contextual methods. However, changing
this behavior could disrupt macros dependent on the current behavior,
potentially breaking the backward compatibility of quote pattern
matching.

The question remains: should we maintain the current behavior, or alter
the quote pattern matcher to differentiate between normal and contextual
methods?
  • Loading branch information
nicolasstucki authored Jul 13, 2023
2 parents 2712dde + 8519dea commit 766d1cf
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 10 deletions.
8 changes: 8 additions & 0 deletions compiler/src/dotty/tools/dotc/core/Mode.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ object Mode {
/** Are we looking for cyclic references? */
val CheckCyclic: Mode = newMode(5, "CheckCyclic")

/** We are in arguments of HOAS pattern in quote pattern matching
* e.g. x, y, z in a quote pattern '{ ... $a(x, y, z) ... }
*
* This mode keep typer from inserting contextual parameters to a contextual method without arguments.
* (See tests/run-macros/i17905 for motivating examples)
*/
val InQuotePatternHoasArgs: Mode = newMode(6, "InQuotePatternHoasArgs")

/** We are in a pattern alternative */
val InPatternAlternative: Mode = newMode(7, "InPatternAlternative")

Expand Down
14 changes: 8 additions & 6 deletions compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala
Original file line number Diff line number Diff line change
Expand Up @@ -112,12 +112,14 @@ trait QuotesAndSplices {
if isFullyDefined(pt, ForceDegree.flipBottom) then
def patternOuterContext(ctx: Context): Context =
if (ctx.mode.is(Mode.QuotedPattern)) patternOuterContext(ctx.outer) else ctx
val typedArgs = tree.args.map {
case arg: untpd.Ident =>
typedExpr(arg)
case arg =>
report.error("Open pattern expected an identifier", arg.srcPos)
EmptyTree
val typedArgs = withMode(Mode.InQuotePatternHoasArgs) {
tree.args.map {
case arg: untpd.Ident =>
typedExpr(arg)
case arg =>
report.error("Open pattern expected an identifier", arg.srcPos)
EmptyTree
}
}
for arg <- typedArgs if arg.symbol.is(Mutable) do // TODO support these patterns. Possibly using scala.quoted.util.Var
report.error("References to `var`s cannot be used in higher-order pattern", arg.srcPos)
Expand Down
6 changes: 4 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4116,8 +4116,10 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
wtp match {
case wtp: ExprType =>
readaptSimplified(tree.withType(wtp.resultType))
case wtp: MethodType if wtp.isImplicitMethod &&
({ resMatch = constrainResult(tree.symbol, wtp, sharpenedPt); resMatch } || !functionExpected) =>
case wtp: MethodType
if wtp.isImplicitMethod
&& ({ resMatch = constrainResult(tree.symbol, wtp, sharpenedPt); resMatch} || !functionExpected)
&& !ctx.mode.is(Mode.InQuotePatternHoasArgs) =>
if (resMatch || ctx.mode.is(Mode.ImplicitsEnabled))
adaptNoArgsImplicitMethod(wtp)
else
Expand Down
8 changes: 6 additions & 2 deletions compiler/src/scala/quoted/runtime/impl/QuoteMatcher.scala
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,11 @@ import dotty.tools.dotc.util.optional
* '{ val x: T = e1; e2 } =?= '{ val y: P = p1; p2 } ===> withEnv(x -> y)('[T] =?= '[P] &&& '{e1} =?= '{p1} &&& '{e2} =?= '{p2})
*
* /* Match def */
* '{ def x0(x1: T1, ..., xn: Tn): T0 = e1; e2 } =?= '{ def y0(y1: P1, ..., yn: Pn): P0 = p1; p2 } ===> withEnv(x0 -> y0, ..., xn -> yn)('[T0] =?= '[P0] &&& ... &&& '[Tn] =?= '[Pn] &&& '{e1} =?= '{p1} &&& '{e2} =?= '{p2})
* '{ def x0(x1: T1, ..., xn: Tn)...(y1: U1, ..., ym: Um): T0 = e1; e2 } =?= '{ def y0(z1: P1, ..., zn: Pn)...(w1: Q1, ..., wn: Qn): P0 = p1; p2 } ===>
* /* Note that types of parameters can depend on earlier parameters */
* withEnv(x1 -> y1, ..., zn -> zn)(...withEnv(y1 -> w1, ..., ym -> wm)(
* ('[T1] =?= '[P1] &&& ... &&&'[T1] =?= '[P1]) &&& ... &&& ('[U1] =?= '[Q1] &&& ... &&&'[Um] =?= '[Qm])
* &&& '[T0] =?= '[P0] &&& '{e1} =?= '{p1} && '{e2} =?= '{p2})...)
*
* // Types
*
Expand Down Expand Up @@ -570,7 +574,7 @@ class QuoteMatcher(debug: Boolean) {
* f has a method type `(x: Int): Int` and `f` maps to `g`, `p` should hold
* `g.apply(0)` because the type of `g` is `Int => Int` due to eta expansion.
*/
case Apply(fun, args) if env.contains(tree.symbol) => transform(fun).select(nme.apply).appliedToArgs(args)
case Apply(fun, args) if env.contains(tree.symbol) => transform(fun).select(nme.apply).appliedToArgs(args.map(transform))
case tree: Ident => env.get(tree.symbol).flatMap(argsMap.get).getOrElse(tree)
case tree => super.transform(tree)
}.transform(tree)
Expand Down
3 changes: 3 additions & 0 deletions tests/run-macros/i17905.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
case 1: [matched 1st case] another_given outside
case 2: [matched 2nd case] given outside
case 3: [matched 1st case] another_given outside
10 changes: 10 additions & 0 deletions tests/run-macros/i17905/Macro_1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import scala.quoted.*

inline def testCtxParam(inline body: Any) = ${ testCtxParamImpl('body) }
def testCtxParamImpl(body: Expr[Any])(using Quotes): Expr[String] =
body match
case '{ given i: String = "given"; def g(using s: String) = "placeholder"; $a(g, i): String } =>
'{ $a(((s: String) ?=> s"[matched 1st case] ${s}"), "another_given") }
case '{ def g(using s: String) = "placeholder"; $a(g): String } =>
'{ $a((s: String) ?=> s"[matched 2nd case] ${s}") }
case _ => Expr("not matched")
6 changes: 6 additions & 0 deletions tests/run-macros/i17905/Test_2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
@main def Test: Unit =
println("case 1: " + testCtxParam { given String = "given"; def f(using t: String) = "placeholder"; f + " outside" })
given String = "given"
println("case 2: " + testCtxParam { def f(using t: String) = "placeholder"; f + " outside" })
/* This is expected to match the first case. The current QuoteMatcher identifies a function with a contextual function. */
println("case 3: " + testCtxParam { given i: String = "given"; def a(x: String) = "placeholder"; a(i) + " outside" } )

0 comments on commit 766d1cf

Please sign in to comment.