Add QuotePattern AST #17935

nicolasstucki · 2023-06-07T09:25:59Z

We add the definition of the QuotePattern AST. In this first step, keep the typing of quote patterns untouched and decode the resulting unapply into a QuotePattern. This AST is used to perform all transformations until the staging phase, here it is encoded into an unapply and a Quote AST. We also encode the AST into an unapply when we pickle the tree to keep the same pickled representation as before.

Based on #17956

When we encode quote patterns into `unapply` methods, we need to create a copy of each type variable. One copy is kept within the quote(in the next stage) and the other is used in the `unapply` method to define the usual pattern type variables. When creating the latter we copied the symbols but did not update the infos. This implies that if type variables would be bounded by each other, the bounds of the copies would be the original types instead of the copies. We need to update those references. To update the info we now create all the symbols in one pass and the update all their infos in a second pass. This also implies that we cannot use the `newPatternBoundSymbol` to create the symbol as this constructor will register the info into GADT bounds. Instead we use the plain `newSymbol`. Then in the second pass, when we have updated the infos, we register the symbol into GADT bounds. Note that the code in the added test does compiles correctly, but it had the inconsistent bounds. This test is added in case we need to manually inspect the bounds latter. This test does fail to compile in scala#17935 if this fix is not applied.

smarter · 2023-06-21T12:52:56Z

compiler/src/dotty/tools/dotc/staging/CrossStageSafety.scala

@@ -104,6 +105,19 @@ class CrossStageSafety extends TreeMapWithStages {
      case _: DefDef if tree.symbol.isInlineMethod =>
        tree

+      case tree: CaseDef if level == 0 =>


What would go wrong if we instead directly did case tree: QuotePattern if level == 0 here instead of a few lines below in this case?

The important this in this transformation is to introduce the new pattern variables (implicit Type[t]) in the pattern scope. These are needed to transform the RHS of the case. We need to encode the patterns before we look for the patterns variables to make sure they are in the context when needed.

An alternative would be implement this with a variant of the CaseDef case in TreeMapWithImplicits that collects the scope a second time after transforming the pattern. This would also need to include the context update in the CaseDef case of TreeMapWithStages. I chose not to do this because any change in the super-classes would need to be reflected here. This might be missed easily.

smarter · 2023-06-21T12:57:14Z

compiler/src/dotty/tools/dotc/ast/TreeInfo.scala

        case _ => foldOver(syms, tree)
      }
+      private object quotePatVars extends TreeAccumulator[List[Symbol]] {


Instead of using a separate object, could we instead add a case SplitPattern(pat, _) in the original TreeAccumulator that checks the level?

We could. I was trying to keep keep the cases of patVars to a minimum for the case where we do not have quoted patterns(most of them). Should I change this?

Your call, I would optimize for what you think is the most readable

I tried with levels. While that one is more concise and simple to read, I believe the current version makes the distinction between the two different traversal modes clearer.

smarter · 2023-06-21T13:16:03Z

compiler/src/dotty/tools/dotc/ast/Trees.scala

+    type ThisTree[+T <: Untyped] = QuotePattern[T]
+
+    /** Type of the quoted pattern */
+    def bodyType(using Context): Type =


This method and the one on Quote only make sense on typed trees, so to make them safe they should take a (using T <:< Type) parameter or be defined in tpd.scala

That would not work here

https://github.com/lampepfl/dotty/blob/d59048cb8fa36af65596c4de26db1bfdd18c3d30/compiler/src/dotty/tools/dotc/typer/ReTyper.scala#L115-L118

Should I rename it to bodyTypeOpt? And do the same for Tree.Quote.

You could use promote(tree).bodyType there

I did have to use a cast is another similar case in the refined printer

smarter · 2023-06-21T13:19:29Z

compiler/src/dotty/tools/dotc/ast/Trees.scala

+   *
+   *   @param  bindings  Type variable definitions (`Bind` tree)
+   *   @param  body      Quoted pattern (without type variable definitions)
+   *   @param  quotes    A reference to the given `Quotes` instance in scope


Does this actually need to be stored in the tree or could we look for the given when we need it?

In theory, we could look for it again. But I preferred to keep track of it to ensure we do not change the instance. Maybe a macro-generated quote pattern could go wrong. I need to analyze this in depth if we want to remove this instance. Would be nicer though.

When we encode quote patterns into `unapply` methods, we need to create a copy of each type variable. One copy is kept within the quote(in the next stage) and the other is used in the `unapply` method to define the usual pattern type variables. When creating the latter we copied the symbols but did not update the infos. This implies that if type variables would be bounded by each other, the bounds of the copies would be the original types instead of the copies. We need to update those references. To update the info we now create all the symbols in one pass and the update all their infos in a second pass. This also implies that we cannot use the `newPatternBoundSymbol` to create the symbol as this constructor will register the info into GADT bounds. Instead we use the plain `newSymbol`. Then in the second pass, when we have updated the infos, we register the symbol into GADT bounds. Note that the code in the added test does compiles correctly, but it had the inconsistent bounds. This test is added in case we need to manually inspect the bounds latter. This test does fail to compile in scala#17935 if this fix is not applied.

We add the definition of the `QuotePattern` AST. In this first step, we keep the typing of quote patterns untouched and decode the resulting unapply into a `QuotePattern`. This AST is used to preform all transformations until the `staging` phase, where it is encoded into an `unapply` and a `Quote` AST. We also encode the AST into an `unapply` when we pickle the tree to keep the same pickled representation as before.

smarter · 2023-06-26T15:02:50Z

compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala

@@ -734,7 +734,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
        keywordStr("macro ") ~ toTextGlobal(call)
      case tree @ Quote(body, tags) =>
        val tagsText = (keywordStr("<") ~ toTextGlobal(tags, ", ") ~ keywordStr(">")).provided(tree.tags.nonEmpty)
-        val exprTypeText = (keywordStr("[") ~ toTextGlobal(tree.bodyType) ~ keywordStr("]")).provided(printDebug && tree.typeOpt.exists)
+        val exprTypeText = (keywordStr("[") ~ toTextGlobal(tpd.bodyType(tree.asInstanceOf[tpd.Quote])) ~ keywordStr("]")).provided(printDebug && tree.typeOpt.exists)


What happens when I pretty-print an untyped Quote node?

The provided condition makes sure that the type exists. If it does not, the text and tpd.bodyType is not evaluated as it is a by-name prefix.

The other solution to this would be to create bodyTypeOpt. But this is the only use case for it.

Ah I missed the provided, ok.

nicolasstucki self-assigned this Jun 7, 2023

nicolasstucki force-pushed the quote-pattern-ast branch 17 times, most recently from bc086fd to 9a79ec7 Compare June 12, 2023 06:09

nicolasstucki mentioned this pull request Jun 12, 2023

Fix bounds of encoded type variables in quote patterns #17956

Closed

nicolasstucki force-pushed the quote-pattern-ast branch 3 times, most recently from a4be2a7 to d59048c Compare June 13, 2023 10:53

nicolasstucki marked this pull request as ready for review June 14, 2023 11:01

nicolasstucki requested a review from smarter June 14, 2023 11:01

nicolasstucki assigned smarter and unassigned nicolasstucki Jun 14, 2023

smarter requested changes Jun 21, 2023

View reviewed changes

smarter assigned nicolasstucki and unassigned smarter Jun 21, 2023

nicolasstucki assigned smarter and unassigned nicolasstucki Jun 23, 2023

nicolasstucki force-pushed the quote-pattern-ast branch from b783d0b to 2a03623 Compare June 26, 2023 12:47

nicolasstucki added 3 commits June 26, 2023 16:49

Only allow bodyType on typed quotes

351456e

nicolasstucki force-pushed the quote-pattern-ast branch from 2a03623 to 351456e Compare June 26, 2023 14:49

smarter reviewed Jun 26, 2023

View reviewed changes

smarter approved these changes Jun 28, 2023

View reviewed changes

nicolasstucki enabled auto-merge June 28, 2023 13:07

nicolasstucki merged commit 08de2ba into scala:main Jun 28, 2023

nicolasstucki deleted the quote-pattern-ast branch June 28, 2023 14:33

Kordyjan added this to the 3.4.0 milestone Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add QuotePattern AST #17935

Add QuotePattern AST #17935

Uh oh!

nicolasstucki commented Jun 7, 2023 •

edited

Loading

Uh oh!

smarter Jun 21, 2023

Uh oh!

nicolasstucki Jun 23, 2023

Uh oh!

smarter Jun 21, 2023

Uh oh!

nicolasstucki Jun 23, 2023

Uh oh!

smarter Jun 23, 2023

Uh oh!

nicolasstucki Jun 26, 2023

Uh oh!

smarter Jun 21, 2023

Uh oh!

nicolasstucki Jun 23, 2023

Uh oh!

smarter Jun 23, 2023 •

edited

Loading

Uh oh!

nicolasstucki Jun 26, 2023

Uh oh!

smarter Jun 21, 2023

Uh oh!

nicolasstucki Jun 23, 2023

Uh oh!

smarter Jun 26, 2023

Uh oh!

nicolasstucki Jun 28, 2023 •

edited

Loading

Uh oh!

smarter Jun 28, 2023

Uh oh!

Uh oh!

Add QuotePattern AST #17935

Add QuotePattern AST #17935

Uh oh!

Conversation

nicolasstucki commented Jun 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smarter Jun 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolasstucki Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nicolasstucki commented Jun 7, 2023 •

edited

Loading

smarter Jun 23, 2023 •

edited

Loading

nicolasstucki Jun 28, 2023 •

edited

Loading