dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.23k stars 1.57k forks source link

[analyzer] Make `on` edges from mixins count with respect to exhaustiveness #53391

Open eernstg opened 1 year ago

eernstg commented 1 year ago

This issue is a request for implementation of the breaking change described in https://github.com/dart-lang/sdk/issues/53325. This issue is the analyzer specific part of https://github.com/dart-lang/sdk/issues/53390.

keertip commented 1 year ago

@eernstg , is there a timeline for this? By when is it to be done?

keertip commented 1 year ago

/cc @scheglov , @bwilkerson

eernstg commented 1 year ago

About the timeline: It should not be a big thing, and it is not connected to a language version (it's basically a bug fix). So anywhere it fits would be fine.

sgrekhov commented 1 year ago

The following tests fail in analyzer

https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_sealed_A02_t04.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_sealed_A02_t05.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_sealed_A02_t06.dart https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_sealed_A02_t07.dart

bwilkerson commented 1 year ago

@scheglov

scheglov commented 1 year ago

https://dart-review.googlesource.com/c/sdk/+/328662 should add mixin constraints. But even with these changes, the analyzer fails the same tests as CFE in https://github.com/dart-lang/sdk/issues/53392 I suspect there might be an issue in the shared code.

eernstg commented 6 months ago

This one could be done, or nearly done. It would be great if we could close it!

LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_sealed_A02_t06.dart and LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_sealed_A02_t07.dart are now succeeding in all configurations, including all analyzer configurations.

LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_sealed_A02_t05.dart fails because of the following:

sealed class S {}
base mixin M on S {}
class C extends S {}
base class F extends S with M {}

void main() {
  S s = F();
  int i2 = switch (s) { F() => 1 }; // Error.
  int i4 = switch (s) { C() => 1, F() => 2 }; // No error.
}

An error is expected for both the declaration of i2 and the declaration of i4, but only the error for i2 is reported. However, if the line where i2 is declared is commented out then the error for i4 is reported.

I tend to think this is a bug in the analyzer. It could also be a consequence of flow analysis, but that seems somewhat unlikely because it would then be flow analysis which is applied to an expression that has a compile-time error. Do we do that?

The situation in LanguageFeatures/Patterns/Exhaustiveness/exhaustiveness_sealed_A02_t04.dart looks similar.

eernstg commented 6 months ago

@scheglov, WDYT?

eernstg commented 5 months ago

The missing compile-time errors seem to be caused by the flow analysis of erroneous switch expressions. That is, one error causes another error; we don't usually test that (because error recovery is an implementation specific topic).

More details here: https://github.com/dart-lang/sdk/issues/53392#issuecomment-2113062914.

With a little luck, the tests will succeed after having been modified.