Fix HK quoted pattern type variables #16907
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicolasstucki
force-pushed
the
fix-hk-quoted-pattern-type-variables
branch
from
25ddf81 to
b0dd16b
Compare
nicolasstucki
added
the
needs-minor-release
nicolasstucki
commented
Feb 14, 2023
| @@ -40,5 +42,12 @@ trait QuoteMatching: | |||
| * @param pattern `Type[?]` containing the pattern tree | |||
| * @return None if it did not match, `Some(tup)` if it matched where `tup` contains `Type[Ti]`` | |||
| */ | |||
| def unapply[TypeBindings <: Tuple, Tup <: Tuple](scrutinee: Type[?])(using pattern: Type[?]): Option[Tup] | |||
| // FIXME: is this change TASTy binary compatible? Or we need to create a new version of the interface? | |||
| def unapply[TypeBindings, Tup <: Tuple](scrutinee: Type[?])(using pattern: Type[?]): Option[Tup] | |||
There was a problem hiding this comment.
If there's no way it could have been overridden in a subclass, then it is indeed a backward compatible change. You can enlarge the bounds of a type parameter.
There was a problem hiding this comment.
This method is implemented in QuotesImpl in the compiler. I guess this means that it is not a TASTy compatible change.
There was a problem hiding this comment.
In second thought, this class can only be extended by the compiler. It must be mixed in Quotes, but users do not have access to this class. It should be fine to change it.
The only scenario where this could break is if we recompile the compiler from TASTy but use a newer version of the standard library. This is not something that we would ever want to do.
nicolasstucki
force-pushed
the
fix-hk-quoted-pattern-type-variables
branch
5 times, most recently
from
b15546f to
7f7b0b5
Compare
smarter
reviewed
Feb 16, 2023
| @@ -27,7 +27,7 @@ trait QuoteMatching: | |||
| * @param pattern `Expr[Any]` containing the pattern tree | |||
| * @return None if it did not match, `Some(tup)` if it matched where `tup` contains `Expr[Ti]`` | |||
| */ | |||
| def unapply[TypeBindings <: Tuple, Tup <: Tuple](scrutinee: Expr[Any])(using pattern: Expr[Any]): Option[Tup] | |||
| def unapply[TypeBindings, Tup <: Tuple](scrutinee: Expr[Any])(using pattern: Expr[Any]): Option[Tup] | |||
There was a problem hiding this comment.
TypeBindings does not appear anywhere in the signature of the method. How is it used?
There was a problem hiding this comment.
It is used as a placeholder for type-binding definitions. We must create them all in TypeBindings to make them available in Tup.
Also see https://github.com/lampepfl/dotty/blob/main/compiler/src/dotty/tools/dotc/typer/QuotesAndSplices.scala#L352-L380
This doc needed an update.
There was a problem hiding this comment.
What happens if the user manually calls unapply and pass a random type to TypeBindings?
nicolasstucki
force-pushed
the
fix-hk-quoted-pattern-type-variables
branch
from
7f7b0b5 to
a91eefd
Compare
nicolasstucki
added a commit
to dotty-staging/dotty
that referenced
this pull request
Feb 17, 2023
Support explicit type variable definition in quoted patterns.
This allows users to set explicit bounds or use the binding twice.
Previously this was only possible on quoted expression patterns case '{ ... }.
```scala
case '[type x; `x`] =>
case '[type x; Map[`x`, `x`]] =>
case '[type x <: List[Any]; `x`] =>
```
In combination with scala#16907 it would also allow
```
case '[type f[X]; `f`] =>
case '[type f <: AnyKind; `f`] =>
```
and therefore solve scala#10864 and scala#11738
nicolasstucki
added a commit
to dotty-staging/dotty
that referenced
this pull request
Feb 17, 2023
Support explicit type variable definition in quoted patterns.
This allows users to set explicit bounds or use the binding twice.
Previously this was only possible on quoted expression patterns case '{ ... }.
```scala
case '[type x; `x`] =>
case '[type x; Map[`x`, `x`]] =>
case '[type x <: List[Any]; `x`] =>
```
In combination with scala#16907 it would also allow
```scala
case '[type f[X]; `f`] =>
case '[type f <: AnyKind; `f`] =>
```
and therefore solve scala#10864 and scala#11738
nicolasstucki
added a commit
to dotty-staging/dotty
that referenced
this pull request
Feb 17, 2023
Support explicit type variable definition in quoted patterns.
This allows users to set explicit bounds or use the binding twice.
Previously this was only possible on quoted expression patterns case '{ ... }.
```scala
case '[type x; `x`] =>
case '[type x; Map[`x`, `x`]] =>
case '[type x <: List[Any]; `x`] =>
```
In combination with scala#16907 it would also allow
```scala
case '[type f[X]; `f`] =>
case '[type f <: AnyKind; `f`] =>
```
and therefore solve scala#10864 and scala#11738
The issue was in the encoding into `{ExprMatchModule,TypeMatchModule}.unapply`.
Specifically with the `TypeBindings` argument. This arguments holds the
list of type variable definitions (`tpd.Bind` trees). We used a `Tuple`
to list all the types inside. The problem is that higher-kinded type
variables do not conform with the upper bounds of the tuple elements.
The solution is to use an HList with any-kinded elements.
nicolasstucki
force-pushed
the
fix-hk-quoted-pattern-type-variables
branch
from
a91eefd to
0ce5c32
Compare
nicolasstucki
commented
Feb 20, 2023
Kordyjan
added
the
backport:accepted
smarter
removed
the
needs-minor-release
smarter
approved these changes
Feb 20, 2023
Kordyjan
added a commit
that referenced
this pull request
Feb 21, 2023
The issue was in the encoding into
`{ExprMatchModule,TypeMatchModule}.unapply`. Specifically with the
`TypeBindings` argument. This arguments holds the list of type variable
definitions (`tpd.Bind` trees). We used a `Tuple` to list all the types
inside. The problem is that higher-kinded type variables do not conform
with the upper bounds of the tuple elements. The solution is to use an
HList with any-kinded elements.
Backport of #16907
Kordyjan
added
backport:done
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue was in the encoding into
{ExprMatchModule,TypeMatchModule}.unapply. Specifically with theTypeBindingsargument. This arguments holds the list of type variable definitions (tpd.Bindtrees). We used aTupleto list all the types inside. The problem is that higher-kinded type variables do not conform with the upper bounds of the tuple elements. The solution is to use an HList with any-kinded elements.