Skip to content

Fix #2663: More refined handling of enum case apply results #3903

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
Jan 28, 2018

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jan 24, 2018

i2663 demonstrates that we do not always have a complete result type
for the apply method of an enum case. In that case our only recourse
is to add a generic widening method that upcasts the enum case class to the
enum base class.

@liufengyun
Copy link
Contributor

It doesn't work for the following case:

enum Foo[T](x: T) {
  case Bar[S, T](y: T) extends Foo(y)
}

While the following compiles without no problem:

class Foo[T](x: T)
case class Bar[S, T](y: T) extends Foo(y)

@liufengyun liufengyun assigned odersky and unassigned liufengyun Jan 24, 2018
@odersky
Copy link
Contributor Author

odersky commented Jan 27, 2018

@liufengyun Excellent catch! This turned out to be an even bigger can of worms than I thought.

i2663 demonstrates that we do not always have a complete result type
for the `apply` method of an enum case. In that case our only recourse
is to add a generic widening method that upcasts the enum case class to the
enum base class.
Widening methods need to be given derived versions of the type parameters of
the original enum class instead of the case class. This required some new
infrastructure to define a deribed type tree from a symbol instead of an
original TypeDef.
It turns out that simply renaming type parameters is not enough to generate
a correct eqInstance definition such as

    def eqInstance[S$1, S$1, S$2, T$2]eqInstance ...

This failed when a parameter `S` or `T` contained the other in its bound. i2663.scala
now contains a test for that.

The infix involes keeping track of renaming suffices when inter-parameter references are resolved.
@odersky odersky removed their assignment Jan 27, 2018
@liufengyun
Copy link
Contributor

liufengyun commented Jan 27, 2018

The fix looks good, but I found another related issue about the return type of apply:

enum Foo[T](x: T) {
  case Bar[S, T](y: T) extends Foo[y.type](y)
}

Compiler error:

2 |  case Bar[S, T](y: T) extends Foo[2](y)
  |                                      ^
  |                                      found:    T(y)
  |                                      required: Int(2)

@liufengyun liufengyun assigned odersky and unassigned liufengyun Jan 27, 2018
@odersky
Copy link
Contributor Author

odersky commented Jan 28, 2018

That's devious (or clever, depending how you look at it)! It seems to fix this properly we need #3920 to be solved, and it is uncertain whether we will ever get there.

The only quick fix I see is to disallow parameter references in extension clauses of enum cases.

@odersky
Copy link
Contributor Author

odersky commented Jan 28, 2018

Or we leave it as is, and just accept the fact that the enum expansion is not type correct in some cases.

@odersky
Copy link
Contributor Author

odersky commented Jan 28, 2018

I filed a separate issue #3935 to keep track of this.

@odersky odersky merged commit 3649fa8 into scala:master Jan 28, 2018
@liufengyun liufengyun deleted the fix-#2663 branch March 1, 2019 15:27
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