Skip to content

Add missing case to TypeComparer #3713

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

Merged
merged 4 commits into from
Dec 30, 2017
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Dec 30, 2017

The missing case was for

hk-type ref <:< hk type lambda

E.g.

type Y[X]
Foo[Y] <:< Foo[[T] => Y[T]]

The missing case was for

   hk-type ref <:< hk type lambda

E.g.

   type Y[X]
   Foo[Y] <:< Foo[[T] => Y[T]]
@odersky odersky requested a review from smarter December 30, 2017 17:59
@smarter
Copy link
Member

smarter commented Dec 30, 2017

Missing testcases? I assume this should fix #3658 and/or #2989 ?

@odersky
Copy link
Contributor Author

odersky commented Dec 30, 2017

Just added the test case for #2989. #3658 still fails.

if (tp1.isHK) {
val tparams1 = tp1.typeParams
return isSubType(
HKTypeLambda.fromParams(tparams1, tp1.appliedTo(tparams1.map(_.paramRef))),
Copy link
Member

Choose a reason for hiding this comment

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

Can be written EtaExpansion(tp1) I think.

Copy link
Contributor Author

@odersky odersky Dec 30, 2017

Choose a reason for hiding this comment

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

EtaExpansion only works for class type constructors. I tried to change that, but it would be quite involved, so I thought it was simpler to write the code directly here.

@@ -1065,7 +1065,7 @@ object desugar {
AppliedTypeTree(ref(seqType), t),
New(ref(defn.RepeatedAnnotType), Nil :: Nil))
} else {
assert(ctx.mode.isExpr || ctx.reporter.hasErrors, ctx.mode)
assert(ctx.mode.isExpr || ctx.reporter.hasErrors || ctx.mode.is(Mode.Interactive), ctx.mode)
Copy link
Member

Choose a reason for hiding this comment

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

Any idea why we manage to hit this assertion in the IDE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea, I am afraid.

@odersky odersky merged commit 2164c42 into scala:master Dec 30, 2017
@allanrenucci allanrenucci deleted the fix-#2989 branch December 30, 2017 22:12
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