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

#2420. Add extension types exhaustiveness tests. Enums #2422

Merged
merged 1 commit into from
Dec 11, 2023

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!

I think we should have a set of similar tests where the underlying exhaustiveness of a non-extension type is trivial, because it will be very easy to see with those tests that they are testing exactly the support for a matched value type which is an extension type.

The current tests are also including a tricky case involving enumerated values which are instances of a generic enum, so they are much easier to mess up (say, if they need to be updated somehow in the future).

However, that could just as well be in a different PR, so I'll land this one.

switch (e) {
case E.a:
case E.c:
return "ok";
Copy link
Member

Choose a reason for hiding this comment

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

This seems to work, but it would be somewhat esoteric (and this has nothing to do with extension types, the situation exists after erasure).

The reason why this is a very special exception is that we (presumably) rely on the fact that this is a switch over enumerated values. This means that we can iterate over all the elements, and it is guaranteed that we won't encounter any other objects than the ones that are declared in the enum declaration itself, and they have a statically known run-time type.

Lots and lots of stuff to know!

OK, I was worried that this would be on the very edge of the capabilities of the exhaustiveness analysis, but it seems to work fine.

In any case, we should probably also have a simpler test where the enumerated type isn't generic, such that it is easy to see that we have an extension type on top of a very straightforward setup (which will be a test that is only about being an extension type, so it's very safe to assume that it will test just that).

@eernstg eernstg merged commit 3734cda into dart-lang:master Dec 11, 2023
2 checks passed
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Dec 19, 2023
2023-12-19 sgrekhov22@gmail.com Fixes dart-lang/co19#2441. Fix roll failures (dart-lang/co19#2443)
2023-12-18 sgrekhov22@gmail.com Fixes dart-lang/co19#2435. Fix roll failures (dart-lang/co19#2437)
2023-12-18 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Variables (dart-lang/co19#2431)
2023-12-15 sgrekhov22@gmail.com Fixes dart-lang/co19#2430. Update expected errors positions for CFE (dart-lang/co19#2433)
2023-12-15 sgrekhov22@gmail.com Fixes dart-lang/co19#2432. Remove excessive expected error for CFE (dart-lang/co19#2434)
2023-12-14 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Maps (dart-lang/co19#2426)
2023-12-14 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Lists (dart-lang/co19#2424)
2023-12-14 sgrekhov22@gmail.com dart-lang/co19#2139. Fix wrong failure of Language/Functions/element_type_A02_t06 (dart-lang/co19#2429)
2023-12-13 sgrekhov22@gmail.com dart-lang/co19#2350. Add more factory constructors tests (dart-lang/co19#2427)
2023-12-13 sgrekhov22@gmail.com Fixes dart-lang/co19#2415. Update `StreamController.broadcast()` test according to the changed documentation (dart-lang/co19#2425)
2023-12-13 sgrekhov22@gmail.com dart-lang/co19#2350. Add/update factory constructor tests. Part 4 (dart-lang/co19#2367)
2023-12-12 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Enums, trivial cases (dart-lang/co19#2423)
2023-12-11 sgrekhov22@gmail.com dart-lang/co19#2420. Add extension types exhaustiveness tests. Enums (dart-lang/co19#2422)
2023-12-11 sgrekhov22@gmail.com dart-lang/co19#2386. Rename well-bounded tests. Update descriptions (dart-lang/co19#2405)
2023-12-08 sgrekhov22@gmail.com dart-lang/co19#2415. Change expectations for Stream.asyncMap() according to the current behavior (dart-lang/co19#2421)

Change-Id: I777eba4f1615c8477a5d2044f295696dfc210b1d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342582
Commit-Queue: Erik Ernst <eernst@google.com>
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.

None yet

2 participants