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

[breaking change] Disambiguate extension type to declare an extension type #53883

Closed
eernstg opened this issue Oct 27, 2023 · 8 comments
Closed
Assignees
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). breaking-change-approved breaking-change-request This tracks requests for feedback on breaking changes implementation Track the implementation of a specific feature (use on area-meta issue, not issues for each tool)

Comments

@eernstg
Copy link
Member

eernstg commented Oct 27, 2023

Change Intent

The Dart grammar currently allows extension type on (int i,) {} as a declaration of an extension whose name is type and whose on-type is a record type.

With the upcoming introduction of extension type declarations, the language team wishes to ensure that it cannot be an extension, such that a reader of Dart code can safely assume that a declaration starting with extension type will be an extension type declaration.

The change proposed here is an adjustment of the grammar such that this goal is achieved. Details here: dart-lang/language#3431.

Justification

The introduction of extension type declarations creates an ambiguity, and it must be resolved. The language team prefers to ensure that a declaration that starts with extension type actually declares an extension type and not a plain extension, because that is considered to be the least surprising choice. Also, the value of being able to have an extension whose name is type is considered low.

Impact

Probably very low: It only affects extension declarations whose name is type, and that choice of name violates the general rule that type-like entities should have a capitalized name.

Mitigation

Rename the extension named type to some other name. This will affect explicitly resolved extension member invocations (where type must be replaced by the new name). It is expected to be a very rare phenomenon: Extensions named type are presumably very rare, and only few extension member invocations are resolved explicitly: Low probability times low probability is even lower probability.

Change Timeline

As soon as possible. This ambiguity will have to be handled at some point anyway.

Associated CLs

None, yet.

@eernstg eernstg added the breaking-change-request This tracks requests for feedback on breaking changes label Oct 27, 2023
@lrhn lrhn added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Oct 27, 2023
@leafpetersen
Copy link
Member

cc @itsjustkevin @Hixie @vsmenon

I'd like to expedite review on this since it's on the critical path for release. This is a small technical breaking change which I don't expect to break anything in practice.

@itsjustkevin
Copy link
Contributor

@grouma could you weigh in on this breaking change as well?

@vsmenon
Copy link
Member

vsmenon commented Nov 6, 2023

lgtm

@grouma
Copy link
Member

grouma commented Nov 6, 2023

SGTM.

eernstg added a commit to dart-lang/language that referenced this issue Nov 9, 2023
Move the intersection type rules up in the definition of `UP`, such that no results are of the form `(X & B)?`. The point is that types of the form `(X & B)?` are not intended to occur at all, and this change will ensure that `UP` will not create any such type.

For further information, see:
- Implementation: https://dart-review.googlesource.com/c/sdk/+/333922
- Breaking change request: dart-lang/sdk#53883
@Hixie
Copy link
Contributor

Hixie commented Nov 9, 2023

i strongly support this

@eernstg eernstg added the implementation Track the implementation of a specific feature (use on area-meta issue, not issues for each tool) label Nov 10, 2023
@eernstg
Copy link
Member Author

eernstg commented Nov 10, 2023

Very good, thanks!

The behavior has been implemented already, so there will not be a separate 'implementation' issue, I'll put the 'implementation' link here such that it is documented that the implementation has been handled.

@eernstg eernstg closed this as completed Nov 10, 2023
@github-project-automation github-project-automation bot moved this from In review to Complete in Breaking Changes Nov 10, 2023
@eernstg
Copy link
Member Author

eernstg commented Nov 10, 2023

@sgrekhov, I believe we also have co19 tests for this topic, right?

@sgrekhov
Copy link
Contributor

For extension we have
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Extension-methods/syntax_t05.dart
https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Extension-methods/syntax_t06.dart

I'll add this corner case with record type as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). breaking-change-approved breaking-change-request This tracks requests for feedback on breaking changes implementation Track the implementation of a specific feature (use on area-meta issue, not issues for each tool)
Projects
Status: Complete
Development

No branches or pull requests

9 participants