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

Specify that enums extend _Enum from dart:core. #3671

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stereotype441
Copy link
Member

See
#3665 (comment) for details.


  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
Contribution guidelines:

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

@stereotype441 stereotype441 requested a review from lrhn March 21, 2024 17:31
*or* retain the current `_Enum` class and make that the actual superclass of `enum` classes. Either works, I’ll use `Enum` as the superclass directly in the following.

Then desugar an `enum` declaration to an actual `class` declaration and rewrite every generative constructor of the `enum` declaration to take two extra leading positional arguments.
An `enum` declaration is desugared to an actual `class` declaration, which extends a private `_Enum` class in `dart:core`. _This class holds the `final int index;` declaration and a `final String _name;` declaration (used by the the `EnumName.name` getter), and both fields are initialized by a constructor._ Every generative constructor of the `enum` declaration is rewritten to take two extra leading positional arguments.
Copy link
Member

Choose a reason for hiding this comment

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

I really wish this wasn't necessary. We're specifying an implementation detail.

Could we at least avoid specifying the name of the class?
Just say that there exists an anonymous class which implements Enum and which enum classes extend,
so that the depth of an enum class is at least depth(Enum) + 2.

That still implies that there is a class at depth(Enum) + 1 that other classes can conflict with. Can we introduce a third class at depth depth(Enum) + 1 that enum classes also implement, so UP can never yield _Enum? Like for class HideEfficientLengthIterable does for EfficientLengthIterable.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking more about this, I'd prefer to just want to make it unspecified behavior.
Or stop extending _Enum. It is an implementation detail.
Heck, make _Enum not implement Enum, and it would give it a lower depth. (And make inferring Enum impossible, because LUB is moronic.) Or make _Enum a superclass of Enum.

I really, really do not want to enshrine a completely arbitrary private implementation class in the specification. I'd rather make every tool lie about it. That is a more future proof design.

(I do consider if we can make enum classes be allowed to have superclasses, so they only implement Enum. Maybe make _Enum a mixin then - probably needs more mixin features.)

*or* retain the current `_Enum` class and make that the actual superclass of `enum` classes. Either works, I’ll use `Enum` as the superclass directly in the following.

Then desugar an `enum` declaration to an actual `class` declaration and rewrite every generative constructor of the `enum` declaration to take two extra leading positional arguments.
An `enum` declaration is desugared to an actual `class` declaration, which extends a private `_Enum` class in `dart:core`. _This class holds the `final int index;` declaration and a `final String _name;` declaration (used by the the `EnumName.name` getter), and both fields are initialized by a constructor._ Every generative constructor of the `enum` declaration is rewritten to take two extra leading positional arguments.
Copy link
Member

Choose a reason for hiding this comment

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

typo: "used by the the"

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.

3 participants