Skip to content
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

Add SplicePattern AST to parse and type quote pattern splices #17396

Merged
merged 1 commit into from
May 16, 2023

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented May 3, 2023

Splices in quoted expressions and quoted patterns are fundamentally different. SplicePattern makes this difference explicit and also handles HOAS arguments to the pattern splices.

@nicolasstucki nicolasstucki force-pushed the unencode-quote-patterns branch 2 times, most recently from 408904f to 0730eae Compare May 4, 2023 07:01
@nicolasstucki nicolasstucki self-assigned this May 4, 2023
@nicolasstucki nicolasstucki force-pushed the unencode-quote-patterns branch from 0730eae to 40d3e50 Compare May 4, 2023 08:00
@nicolasstucki nicolasstucki requested a review from smarter May 4, 2023 11:40
@nicolasstucki nicolasstucki marked this pull request as ready for review May 5, 2023 15:02
@nicolasstucki nicolasstucki force-pushed the unencode-quote-patterns branch 5 times, most recently from c43a302 to a626cfa Compare May 12, 2023 12:38
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM.

@@ -1566,6 +1587,8 @@ object Trees {
cpy.Quote(tree)(transform(body)(using quoteContext), transform(tags))
case tree @ Splice(expr) =>
cpy.Splice(tree)(transform(expr)(using spliceContext))
case tree @ SplicePattern(body, args) =>
cpy.SplicePattern(tree)(transform(body)(using spliceContext), transform(args))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't spliceContext used for the arguments?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguments of the SplicePattern are defined and references within the quoted pattern. The definition and references are at the same level.

case '{ (x: T) => $f(x) } =>

In this example f is the body, which is at a lower staging level and usable in the RHS of the =>. On the other hand x is defined in the pattern (x: T) and then referenced in the pattern within the SplicePattern in (x).

record("typedSplicePattern")
if (isFullyDefined(pt, ForceDegree.flipBottom)) {
def spliceOwner(ctx: Context): Symbol =
if (ctx.mode.is(Mode.QuotedPattern)) spliceOwner(ctx.outer) else ctx.owner
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't we always in mode QuotedPattern here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only in the first iteration. We want to find the first context that is outside of the quote pattern. Note the recursion spliceOwner(ctx.outer) and the end ctx.owner. I will refactor it to make this clearer.

val pat = typedPattern(tree.body, defn.QuotedExprClass.typeRef.appliedTo(patType))(
using spliceContext.retractMode(Mode.QuotedPattern).addMode(Mode.Pattern).withOwner(spliceOwner(ctx)))
val baseType = pat.tpe.baseType(defn.QuotedExprClass)
val argType = if baseType != NoType then baseType.argTypesHi.head else defn.NothingType
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val argType = if baseType != NoType then baseType.argTypesHi.head else defn.NothingType
val argType = if baseType.exists then baseType.argTypesHi.head else defn.NothingType

Comment on lines 126 to 127
report.error(em"Type must be fully defined.\nConsider annotating the splice using a type ascription:\n ($tree: XYZ).", tree.body.srcPos)
tree.withType(UnspecifiedErrorType)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually use errorTree(tree, ...) for this

Comment on lines +742 to +744
* Parser will only create `${ pattern }` and `$ident`, hence they will not have args.
* While typing, the `$ident(args*)` the args are identified and desugared into a `SplicePattern`
* containing them.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not have the parser take care of this since it's based purely on syntactic information?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the next PR I will introduce QuotePattern. At that point SplicePatterns will be kept after parser and until we encode the pattern into unapply and '{..}s.

@smarter smarter assigned nicolasstucki and unassigned smarter May 12, 2023
@nicolasstucki nicolasstucki force-pushed the unencode-quote-patterns branch from a626cfa to a0bdfe8 Compare May 16, 2023 07:52
@nicolasstucki nicolasstucki requested a review from smarter May 16, 2023 07:53
@nicolasstucki nicolasstucki removed their assignment May 16, 2023
@smarter smarter merged commit d7c0cb0 into scala:main May 16, 2023
@smarter smarter deleted the unencode-quote-patterns branch May 16, 2023 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants