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

Compile quote patterns directly into QuotePattern AST #18133

Merged
merged 3 commits into from
Jul 25, 2023

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Jul 4, 2023

Fixes #14708
Fixes #16522
Fixes #18125
Fixes #18250

@nicolasstucki nicolasstucki self-assigned this Jul 4, 2023
@nicolasstucki nicolasstucki force-pushed the type-QuotePattern-directly branch 3 times, most recently from cb2bdff to 55b53ca Compare July 4, 2023 13:53
@nicolasstucki nicolasstucki force-pushed the type-QuotePattern-directly branch from 55b53ca to 62d64b7 Compare July 13, 2023 09:11
@nicolasstucki nicolasstucki force-pushed the type-QuotePattern-directly branch 7 times, most recently from a587838 to 49fd70c Compare July 20, 2023 07:45
@nicolasstucki nicolasstucki force-pushed the type-QuotePattern-directly branch 4 times, most recently from 77386e0 to edf30ad Compare July 20, 2023 12:50
@nicolasstucki nicolasstucki force-pushed the type-QuotePattern-directly branch from edf30ad to 10b1a74 Compare July 20, 2023 13:12
@nicolasstucki nicolasstucki marked this pull request as ready for review July 24, 2023 13:34
@nicolasstucki nicolasstucki requested a review from smarter July 24, 2023 13:35
val allTypeBindings = List.newBuilder[Bind]
for tpVar <- typeTypeVariables do
val sym = tpVar.symbol
sym.setFlag(Case)
Copy link
Member

Choose a reason for hiding this comment

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

Could this flag be set when creating the symbol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could not figure out how to do it cleanly in Namer. But I found that I could add the case modifier while desugaring.

}

override def transform(tree: Tree)(using Context) = tree match
case tree @ AppliedTypeTree(tpt, args) =>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe that doesn't matter for the transform you're doing, but this isn't complete, it's at least missing TypeBoundsTree, LambdaTypeTree as well as method parameters in DefTrees.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It might be a problem, but we must change this behavior in a minor release. This will not affect soundness of the pattern match, but it will change the behavior of existing programs. This is only used to decide which bound of the type we should return.

This is related to the type inference of type variable bounds and shares some of the same problems and limitations. At some point, we will need a way to allow users to state explicitly if they want to solve for a lower or upper bound. This inference would become redundant or a warning like the one we have for the type bounds.

@smarter smarter assigned nicolasstucki and unassigned smarter Jul 24, 2023
@nicolasstucki nicolasstucki added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Jul 25, 2023
@nicolasstucki nicolasstucki requested a review from smarter July 25, 2023 09:51
@nicolasstucki nicolasstucki removed their assignment Jul 25, 2023
@smarter smarter merged commit 3e00a0d into scala:main Jul 25, 2023
@smarter smarter deleted the type-QuotePattern-directly branch July 25, 2023 12:46
@Kordyjan Kordyjan added this to the 3.4.0 milestone Aug 1, 2023
@Kordyjan Kordyjan removed the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment