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

Fixes #2574. Add more augmenting types tests #2581

Merged
merged 3 commits into from
Apr 2, 2024

Conversation

sgrekhov
Copy link
Contributor

No description provided.

Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

One fix needed, after all.

@eernstg
Copy link
Member

eernstg commented Apr 2, 2024

Concluded at the meeting: augment enum EAlias needs a value in order to be a syntactically correct enum declaration.

@sgrekhov
Copy link
Contributor Author

sgrekhov commented Apr 2, 2024

Thank you. Fixed

@sgrekhov sgrekhov requested a review from eernstg April 2, 2024 08:16
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

I can see in the feature specification that we can do augment e1 in an augment enum declaration in the case where the corresponding enum declaration already has a value named e1.

I do think we should get an error from enum E { e1 } and augment enum E { e1 } (which would include the EAlias case as well).

}

augment enum EAlias {
e1;
Copy link
Member

Choose a reason for hiding this comment

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

This is going to be interesting! ;-)

I would expect that this is an error because there is an e1 in E already (so it would fail when we try to add another declaration named e1).

However, the feature specification does use a construct which would be augment e1 in this case, which would then be implied to be OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to augment e1;

@sgrekhov sgrekhov requested a review from eernstg April 2, 2024 09:24
Copy link
Member

@eernstg eernstg left a comment

Choose a reason for hiding this comment

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

LGTM

@eernstg eernstg merged commit 8d2cada into dart-lang:master Apr 2, 2024
1 of 2 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Apr 5, 2024
2024-04-04 sgrekhov22@gmail.com Fixes dart-lang/co19#2589. Fix roll failure (dart-lang/co19#2590)
2024-04-03 sgrekhov22@gmail.com dart-lang/co19#2559. Add more augmenting types tests. Augment extends (dart-lang/co19#2588)
2024-04-03 sgrekhov22@gmail.com dart-lang/co19#2559. Rename and regroup augmentation types tests (dart-lang/co19#2587)
2024-04-02 sgrekhov22@gmail.com dart-lang/co19#2559. Add augmenting types tests. Part 5 (dart-lang/co19#2582)
2024-04-02 sgrekhov22@gmail.com dart-lang/co19#2559. Update augmenting libraries tests according dart-lang/co19#2583 (dart-lang/co19#2586)
2024-04-02 sgrekhov22@gmail.com Fixes dart-lang/co19#2574. Add more augmenting types tests (dart-lang/co19#2581)
2024-04-02 sgrekhov22@gmail.com dart-lang/co19#2559. Add augmenting types tests. Part 6 (dart-lang/co19#2583)
2024-04-01 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 4.1.1 to 4.1.2 (dart-lang/co19#2585)
2024-04-01 49699333+dependabot[bot]@users.noreply.github.com Bump actions/setup-java from 4.1.0 to 4.2.1 (dart-lang/co19#2584)
2024-03-27 sgrekhov22@gmail.com Fixes dart-lang/co19#2577. Add more chained patterns assignment tests (dart-lang/co19#2580)
2024-03-26 sgrekhov22@gmail.com Fixes dart-lang/co19#2575. Remove null-aware warnings for CFE (dart-lang/co19#2578)
2024-03-26 sgrekhov22@gmail.com Fixes dart-lang/co19#2576. Replace `library augment` by `augment library` (dart-lang/co19#2579)
2024-03-25 sgrekhov22@gmail.com dart-lang/co19#2559. Add augmenting types tests. Part 2 (dart-lang/co19#2569)

R=brianwilkerson@google.com, vegorov@google.com

Change-Id: Ibf64d7fdcae0044b7317718ecf7e0852e0983f7d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/361161
Reviewed-by: Erik Ernst <eernst@google.com>
Auto-Submit: Sergey Grekhov <sgrekhov22@gmail.com>
Reviewed-by: Alexander Thomas <athom@google.com>
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