Skip to content

#3057. Add promotion via type test tests. Check top and bottom types and Null #3165

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

Merged
merged 5 commits into from
May 16, 2025

Conversation

sgrekhov
Copy link
Contributor

@sgrekhov sgrekhov commented May 1, 2025

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.

Looks good! I'm suggesting that a couple of tests should be deleted, because they unfold some cases where the test is basically not possible to perform ("check how this promotion works, except that this promotion never occurs").

Also, a couple of typos.

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.

Updated, PTAL. Some tests deleted, some changed they expected result.

@sgrekhov sgrekhov requested a review from eernstg May 12, 2025 14:55
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.

Just one location in TypeSystem/flow-analysis/promotion_via_type_test_A03_t04.dart has a comment.

@sgrekhov sgrekhov requested a review from eernstg May 13, 2025 11:13
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.

Test t13 remains tricky because factor works in ways that may seem really confusing in special cases like top types.

@sgrekhov sgrekhov requested a review from eernstg May 16, 2025 06:53
@eernstg
Copy link
Member

eernstg commented May 16, 2025

Perhaps the update was not pushed?

@sgrekhov
Copy link
Contributor Author

Really. Sorry. Pushed now.

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, thanks!

@eernstg
Copy link
Member

eernstg commented May 16, 2025

No. Problem. ;-D

@eernstg eernstg merged commit eaa797d into dart-lang:master May 16, 2025
2 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request May 23, 2025
2025-05-23 sgrekhov22@gmail.com dart-lang/co19#3182. Update augmenting_declarations_A04_*.dart (dart-lang/co19#3197)
2025-05-22 sgrekhov22@gmail.com dart-lang/co19#3182. Add application order tests (dart-lang/co19#3196)
2025-05-22 sgrekhov22@gmail.com dart-lang/co19#3182. Update augmenting_declarations_A01/02_*.dart tests (dart-lang/co19#3195)
2025-05-22 sgrekhov22@gmail.com dart-lang/co19#3182. Update type_inheritance_A04/5_t*.dart (dart-lang/co19#3194)
2025-05-22 sgrekhov22@gmail.com dart-lang/co19#3182. Update type_inheritance_A02/3_t*.dart (dart-lang/co19#3193)
2025-05-22 sgrekhov22@gmail.com dart-lang/co19#3057. Add "diamond" inheritance case to promotion via assignment tests (dart-lang/co19#3179)
2025-05-20 sgrekhov22@gmail.com dart-lang/co19#3182. Update type_inheritance_A01_* tests (dart-lang/co19#3191)
2025-05-20 sgrekhov22@gmail.com dart-lang/co19#3182. Update scoping tests (dart-lang/co19#3190)
2025-05-20 sgrekhov22@gmail.com dart-lang/co19#3182. Add enums and extensions tests (dart-lang/co19#3189)
2025-05-19 sgrekhov22@gmail.com dart-lang/co19#3182. Remove `augmented` expression tests (dart-lang/co19#3188)
2025-05-16 sgrekhov22@gmail.com dart-lang/co19#3057. Add promotion via type test tests. Check top and bottom types and Null (dart-lang/co19#3165)

Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try
Change-Id: I0720274d910fb4e1385efeb038080995b35656f0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/430660
Reviewed-by: Alexander Thomas <athom@google.com>
Reviewed-by: Erik Ernst <eernst@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