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

enums page: if you use extends, you have to supply type parameters … #6095

Closed
wants to merge 1 commit into from

Conversation

SethTisue
Copy link
Member

…if you want them

@dragos noted that the compiler actually accepts the original form though, is that a bug? if so, that may need a separate bug report

but then the whole thing seems iffy, since T certainly looks like it's in scope, but then it isn't...?

so, not sure if this should be merged, opening it anyway so the Dotty team is reminded to decide

…if you want them

everyone at the SIP retreat agreed this is what was intended

@dragos noted that the compiler actually accepts the original form though, is that a bug? if so, that may need a separate bug report

 but then the whole thing seems iffy, since `T` certainly _looks_ like it's in scope, but then it isn't...?

so, not sure if this should be merged, opening it anyway so the Dotty team is reminded to decide
Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

  1. Separate subject from body with a blank line
  2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
  3. Limit the subject line to 72 characters
  4. Capitalize the subject line
  5. Do not end the subject line with a period
  6. Use the imperative mood in the subject line ("Add" instead of "Added")
  7. Wrap the body at 80 characters
  8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

@OlivierBlanvillain
Copy link
Contributor

If I recall correctly this is working as intended, the T from Option is "in scope" for case Some. I don't disagree than it's weird, but it might by better to move the discussion to an issue.

@sjrd
Copy link
Member

sjrd commented Mar 16, 2019

The remark on scoping is more or less independent from the contents of this PR.

the SIP retreat we (virtually?) all agreed on the change in this PR. The change is required for a user to be able define a class case that has no type parameter, even when the enum itself has some.

Also note that the spec of enums, as currently written, agrees with the requirement for [T] in this example.

@odersky
Copy link
Contributor

odersky commented Mar 17, 2019

I think we need to give this some more thought. I am not sure we reached a consensus at the SIP meeting. We have the following possibilities:

  1. type parameters are added only of there is no extends clause,
  2. (all) type parameters are added if one of them is referenced in the case,
  3. type parameters are always added if the case has value parameters (i.e. is not translated to an object).

(1) is what the current rules say. (2) is what Adriaan proposed. (3) is what is currently implemented.

In addition we have to clarify what should happen if a type parameter that is not added is nevertheless referred to in the case. Since enum expansion is done during desugaring, this could silently rebind to some other type. We should come up with a solution that avoids this, if possible.

@odersky
Copy link
Contributor

odersky commented Mar 18, 2019

My tendency is to do a variation of (2):

  • If the case has value parameters, enclosing enum type parameters are added

    • if an extends clause is missing, or
    • if one of the type parameters is referenced in the parameter types or in the type arguments of the extends clause.

odersky added a commit to odersky/dotty that referenced this pull request Mar 24, 2019
Change the condition when type parameters of an enum are added to a class case.

Quoting my comment on scala#6095:

We have the following possibilities:

 1. type parameters are added only of there is no extends clause,
 2. (all) type parameters are added if one of them is referenced in the case,
 3. type parameters are always added if the case has value parameters (i.e. is not translated to an object).

(1) is what the current rules say. (2) is what Adriaan proposed. (3) is what is currently implemented.

In addition we have to clarify what should happen if a type parameter that is not added is nevertheless referred to in the case. Since enum expansion is done during desugaring, this could silently rebind to some other type. We should come up with a solution that avoids this, if possible.

This PR changes the rules to implement (2).
@odersky odersky mentioned this pull request Mar 24, 2019
@odersky
Copy link
Contributor

odersky commented Mar 30, 2019

Superseded by #6154

@odersky odersky closed this Mar 30, 2019
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.

None yet

5 participants