Clean up kindedness tests #4360
Merged
Conversation
This file contains hidden or 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
smarter
approved these changes
Apr 29, 2018
| @@ -213,7 +213,10 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisPhase | |||
| } | |||
| case tree: TypeApply => | |||
| val tree1 @ TypeApply(fn, args) = normalizeTypeArgs(tree) | |||
| Checking.checkBounds(args, fn.tpe.widen.asInstanceOf[PolyType]) | |||
| if (fn.symbol != defn.ChildAnnot.primaryConstructor) { | |||
There was a problem hiding this comment.
Instead of special-casing Child, the compiler could be changed to always use it with fully-applied types with wildcards, e.g. Child[Cons[_]]
We currently mostly use isLambdaSub which detects unapplied abstract and alias types but not unapplied generic classes. We should use `hasHigherKind` more often, which includes unapplied classes. The first commit fixes a problem with `hasHigherKind` where abstract types upper bounded by `AnyKind` where not classified as higher-kinded. It also changes checkSimpleKinded to use hasHigherKind instead if isLambdaSub.
Also disallow unapplied classes if bounds are first-kinded. This broke the Child annotation which should really be kind polymorphic. Fixed for now by making a special exception for Child.
hasHigherKind is misleading since it will cover AnyKind and Effect as well and it is not clear how these are "higher" kinds.
odersky
force-pushed
the
change-kindedness
branch
from
1152369 to
2470455
Compare
Blaisorblade
added a commit
to dotty-staging/dotty
that referenced
this pull request
May 17, 2018
This follows scala#4360. I'd love to add a test but scala#4360 has none, so I'm not 100% sure what it'd look like.
Blaisorblade
added a commit
to dotty-staging/dotty
that referenced
this pull request
May 25, 2018
This follows scala#4360. I'd love to add a test but scala#4360 has none, so I'm not 100% sure what it'd look like.
Blaisorblade
added a commit
to dotty-staging/dotty
that referenced
this pull request
Aug 16, 2018
This follows scala#4360. I'd love to add a test but scala#4360 has none, so I'm not 100% sure what it'd look like.
Blaisorblade
added a commit
to dotty-staging/dotty
that referenced
this pull request
Aug 16, 2018
This follows scala#4360. I'd love to add a test but scala#4360 has none, so I'm not 100% sure what it'd look like.
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.
We currently mostly use isLambdaSub which detects unapplied
abstract and alias types but not unapplied generic classes.
We should use
hasHigherKindmore often, which includesunapplied classes.
The first commit fixes a problem with
hasHigherKindwhereabstract types upper bounded by
AnyKindwhere not classifiedas higher-kinded.
It also changes checkSimpleKinded to use hasHigherKind instead
if isLambdaSub.
I thought it would be good to get this in before I proceed with effects, since
effects effectively introduce another kind.