Skip to content

Simplify QuoteMatcher.&&& using optional #16953

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

Closed

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Feb 17, 2023

Use optional boundary abstraction in QuoteMatcher. Specifically to reduce the syntactic and runtime overhead of &&&. For this we define optional in the compiler utils to make it available in the compiler.

Also, use Seq concatenation instead of Tuple concatenation to have the types of elements.

@nicolasstucki nicolasstucki marked this pull request as ready for review February 20, 2023 07:35
Copy link
Contributor

@odersky odersky 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

@@ -121,19 +117,20 @@ object QuoteMatcher {

private def withEnv[T](env: Env)(body: Env ?=> T): T = body(using env)

def treeMatch(scrutineeTree: Tree, patternTree: Tree)(using Context): Option[Tuple] =
def treeMatch(scrutineeTree: Tree, patternTree: Tree)(using Context): Option[Seq[Expr[Any]]] =
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe make a type alias for Seq[Expr[Any]] and explain how it is used in a doc comment for it?

Copy link
Contributor Author

@nicolasstucki nicolasstucki Feb 28, 2023

Choose a reason for hiding this comment

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

I added type MatchingExprs with its documentation.

@odersky odersky assigned nicolasstucki and unassigned odersky Feb 27, 2023
@nicolasstucki
Copy link
Contributor Author

This commit is now included in #16951.

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