Skip to content

#2559. Add tests for reserved word augmented in constructors. Part 1. #3014

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 12, 2024
Merged

#2559. Add tests for reserved word augmented in constructors. Part 1. #3014

merged 2 commits into from
Dec 12, 2024

Conversation

@sgrekhov
Copy link
Contributor

@sgrekhov sgrekhov commented Dec 12, 2024

No description provided.

eernstg
eernstg reviewed Dec 12, 2024
View reviewed changes
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.

Looks good! I think there are a number of situations where one more case could just as well have been included, but isn't there. It's mainly about extension types whose primary constructor is augmented.

LanguageFeatures/Augmentation-libraries/augmented_expression_A11_t19.dart
// ^^^^^^^^^
// [analyzer] unspecified
// [cfe] unspecified
const E.bar({({int augmented}) r = (augmented: 0)});
Copy link
Member

@eernstg eernstg Dec 12, 2024

Choose a reason for hiding this comment

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

Ditto.

Copy link
Contributor Author

@sgrekhov sgrekhov Dec 12, 2024

Choose a reason for hiding this comment

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

The same as the above

sgrekhov
sgrekhov commented Dec 12, 2024
View reviewed changes
Copy link
Contributor Author

@sgrekhov sgrekhov left a comment

Choose a reason for hiding this comment

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

Fixed. In several cases augmenting constructors of extension type are missed by purpose. PTAL.

LanguageFeatures/Augmentation-libraries/augmented_expression_A11_t19.dart
// ^^^^^^^^^
// [analyzer] unspecified
// [cfe] unspecified
const E.bar({({int augmented}) r = (augmented: 0)});
Copy link
Contributor Author

@sgrekhov sgrekhov Dec 12, 2024

Choose a reason for hiding this comment

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

The same as the above

@sgrekhov sgrekhov requested a review from eernstg December 12, 2024 16:27
eernstg
eernstg approved these changes Dec 12, 2024
View reviewed changes
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 8aa6775 into dart-lang:master Dec 12, 2024
2 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Dec 13, 2024
@sgrekhov
[co19] Roll co19 to 8aa677577f4a2d4edd99f1bd4e3ec554c3712edb
362be52 
2024-12-12 sgrekhov22@gmail.com dart-lang/co19#2559. Add tests for reserved word `augmented` in constructors. Part 1. (dart-lang/co19#3014)
2024-12-12 sgrekhov22@gmail.com dart-lang/co19#2559. Update assertions for redirecting generative constructors tests (dart-lang/co19#3013)
2024-12-11 sgrekhov22@gmail.com dart-lang/co19#2559. Update non-redirecting generative constructors tests (dart-lang/co19#3012)
2024-12-11 sgrekhov22@gmail.com dart-lang/co19#2559. Add factory constructors to augmented_expression_A04_t*.dart tests (dart-lang/co19#3011)
2024-12-09 sgrekhov22@gmail.com dart-lang/co19#2559. Update assertion text for for some of augmented expressions tests (dart-lang/co19#3010)

Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try
Change-Id: I1d95b2092a91651bebb63c82c11cf154ff5da863
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/400580
Reviewed-by: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Erik Ernst <eernst@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eernstg eernstg eernstg approved these changes

@chloestefantsova chloestefantsova Awaiting requested review from chloestefantsova

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sgrekhov @eernstg