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

[experimental feature] Support HOAS pattern with type variables for quote pattern matching #18271

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

zeptometer
Copy link
Contributor

@zeptometer zeptometer commented Jul 23, 2023

This PR extends higher-order patterns inside quote patterns to allow type parameters. When this PR is merged, we'll be able to write quote patterns like the following example with an experimental flag experimental.quotedPatternsWithPolymorphicFunctions.

def decomposePoly(x: Expr[Any])(using Quotes): Expr[Int] =
  x match
    case '{ [A] => (x: List[A]) => $y[A](x) : Int } =>
      '{ $y[Int](List(1, 2, 3)) }
    case _ => Expr(0)

You can see that the higher-order pattern $y[A](x) carries an type parameter A. It states that this pattern matches a code fragment with occurrences of A, and y is assigned a polymorphic function type [A] => List[A] => x.

This PR mainly changes two parts: type checker and quote pattern matcher. Those changes are based on the formalized type system defined in Nicolas Stucki's thesis, and one can expect the soundness of the implementation.

Type Dependency

If a higher-order pattern carries a value parameter with a type that has type parameters defined in the quoted pattern, those type parameters should also be captured in the higher-order pattern. For example, the following pattern will not be typed.

case '{ [A] => (x: List[A]) => $y(x) : Int } =>

In this case, x has the type List[A], which includes a type variable A that is defined in the pattern. However, the higher-order pattern $y(x) does not have any type parameters. This should be ill-typed. One can always avoid this kind of type errors by adding type parameters, like $y[A](x)

Implementation Restriction

Current implementation only allows type parameters that do not have bounds, because sound typing rules for such pattern is not clear yet.

case '{ [A] => (x: List[A]) => $y(x) : Int } => // Allowed
case '{ [A <: Int] => (x: List[A]) => $y(x) : Int } => // Disallowed

@zeptometer zeptometer force-pushed the hoas-with-tvar branch 2 times, most recently from 988087c to 6b345cc Compare July 24, 2023 07:09
@nicolasstucki nicolasstucki added the needs-minor-release This PR cannot be merged until the next minor release label Jul 25, 2023
@zeptometer zeptometer force-pushed the hoas-with-tvar branch 4 times, most recently from 282e4ad to fc15089 Compare July 31, 2023 07:51
* (NOTE: Needs non-trivial extension to type system)
*/
val bounds = ctx.gadt.fullBounds(typedTypearg.symbol)
if bounds != null && bounds != TypeBounds.empty then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might want to move this logic to checkPattern because we have fully-typed tree.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sync Aug 07: How to check type bounds with fully typed tree?

@zeptometer zeptometer force-pushed the hoas-with-tvar branch 2 times, most recently from 49c0cca to 6764d5e Compare August 7, 2023 05:36
* (NOTE: Needs non-trivial extension to type system)
*/
val bounds = ctx.gadt.fullBounds(typedTypearg.symbol)
if bounds != null && bounds != TypeBounds.empty then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sync Aug 07: How to check type bounds with fully typed tree?

compiler/src/scala/quoted/runtime/impl/QuoteMatcher.scala Outdated Show resolved Hide resolved
compiler/src/scala/quoted/runtime/impl/QuoteMatcher.scala Outdated Show resolved Hide resolved
Copy link
Contributor Author

@zeptometer zeptometer left a comment

Choose a reason for hiding this comment

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

  • Add comment with "Implementation restriction"
  • Add test cases with and without experimental flags
  • (potentailly) improve performance of type comparison under an environment
  • update stdlibExperimentalDefinitions.scala
  • Add documentation

body match
case '{ [A] => (x : A) => $b[A] : (A => A) } => // error
Expr("A higher-order pattern must carry value params")
case '{ [A] => (x : A) => $b(x) : (A => A) } => // error
Copy link
Contributor Author

Choose a reason for hiding this comment

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

better to have: Show example of expected code $b[A](x) in this case

tests/run-macros/quote-match-poly-function/Macro_1.scala Outdated Show resolved Hide resolved
compiler/src/scala/quoted/runtime/impl/QuoteMatcher.scala Outdated Show resolved Hide resolved
tests/neg-macros/hoas-pattern-with-type-params.scala Outdated Show resolved Hide resolved
@nicolasstucki nicolasstucki self-requested a review August 22, 2023 07:46
@zeptometer zeptometer marked this pull request as ready for review August 27, 2023 01:27
@zeptometer zeptometer changed the title WIP: Support HOAS pattern with type variables for quote pattern matching [experimental feature] Support HOAS pattern with type variables for quote pattern matching Aug 28, 2023
@nicolasstucki
Copy link
Contributor

I will rebase this PR to solve the small conflict in library/src/scala/runtime/stdLibPatches/language.scala.

nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Apr 22, 2024
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Apr 22, 2024
@Kordyjan Kordyjan requested a review from jchyb July 3, 2024 15:04
@Gedochao
Copy link
Contributor

This has been decided to be included in 3.6.0.

Copy link
Contributor

@jchyb jchyb left a comment

Choose a reason for hiding this comment

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

Rebased to resolve conflicts (all very simple). Also added some (also very simple) changes to make it work on top of #17396

@@ -0,0 +1,5 @@
//> using options -experimental
Copy link
Contributor

Choose a reason for hiding this comment

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

Added experimental here and in other tests

assert(targs.isEmpty, "unexpected type arguments in SPLICEPATTERN") // `targs` will be needed for #18271. Until this fearure is added they should be empty.
SplicePattern(pat, args, patType)
SplicePattern(pat, targs, args, patType)
Copy link
Contributor

Choose a reason for hiding this comment

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

Adjusted pickling and unpicking of SplicePatterns as it was previously prepared for this PR by Nicolas

case SplicePattern(pat, args) =>
val targs = Nil // SplicePattern `targs` will be added with #18271
case SplicePattern(pat, targs, args) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@jchyb jchyb merged commit 4c9cf0a into scala:main Jul 22, 2024
28 checks passed
@jchyb
Copy link
Contributor

jchyb commented Jul 22, 2024

Thank you so much @zeptometer! Fantastic work!

@zeptometer
Copy link
Contributor Author

@jchyb I'm glad to hear that this feature is merged 😄 Thank you for your effort in merging this PR!

@WojciechMazur WojciechMazur added this to the 3.6.0 milestone Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:experimental needs-minor-release This PR cannot be merged until the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants