Skip to content

Commit c383330

Browse files
committed
[beta] Flow analysis: Use extension type erasure for implicit is reachability.
Whenever a pattern performs an implicit `is` test, flow analysis attempts to determine whether the `is` test is guaranteed to succeed; if it is, then flow analysis considers the code path in which the `is` test fails to be unreachable. This allows flow analysis to recognize switch statements that are trivially exhaustive (because one of the cases is guaranteed to match), avoiding spurious errors such as "variable must be assigned before use" or "missing return statement". This change upgrades the logic for computing when an `is` test is guaranteed to succeed, so that it accounts for type erasure of extension types. This brings flow analysis's treatment of switch statements into closer alignment with the exhaustiveness checker, which should reduce the risk of confusing error messages. For more information see dart-lang/language#3534 (comment). Fixes dart-lang/language#3534. Bug: dart-lang/language#3534 Change-Id: Ic3f840bbc5793aecf4f6aa4e569526e8482181fc Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/345822 Cherry-pick-request: #54720 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/348200 Reviewed-by: Konstantin Shcheglov <[email protected]> Reviewed-by: Chloe Stefantsova <[email protected]>
1 parent 437ab21 commit c383330

File tree

9 files changed

+150
-3
lines changed

9 files changed

+150
-3
lines changed

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5159,7 +5159,9 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
51595159
_PatternContext<Type> context = _stack.last as _PatternContext<Type>;
51605160
_Reference<Type> matchedValueReference =
51615161
context.createReference(matchedType, _current);
5162-
bool coversMatchedType = operations.isSubtypeOf(matchedType, knownType);
5162+
bool coversMatchedType = operations.isSubtypeOf(
5163+
operations.extensionTypeErasure(matchedType),
5164+
operations.extensionTypeErasure(knownType));
51635165
// Promote the synthetic cache variable the pattern is being matched
51645166
// against.
51655167
ExpressionInfo<Type> promotionInfo =

pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis_operations.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ abstract interface class FlowAnalysisTypeOperations<Type extends Object> {
5555
/// the [TypeClassification] enum.
5656
TypeClassification classifyType(Type type);
5757

58+
/// If [type] is an extension type, returns the ultimate representation type.
59+
/// Otherwise returns [type] as is.
60+
Type extensionTypeErasure(Type type);
61+
5862
/// Returns the "remainder" of [from] when [what] has been removed from
5963
/// consideration by an instance check.
6064
Type factor(Type from, Type what);

pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,11 +358,12 @@ mixin TypeAnalyzer<
358358
required Type requiredType,
359359
}) {
360360
Type matchedValueType = flow.getMatchedValueType();
361-
bool matchedTypeIsSubtypeOfRequired = flow.promoteForPattern(
361+
flow.promoteForPattern(
362362
matchedType: matchedValueType,
363363
knownType: requiredType,
364364
matchFailsIfWrongType: false);
365-
if (matchedTypeIsSubtypeOfRequired) {
365+
if (operations.isSubtypeOf(matchedValueType, requiredType) &&
366+
!operations.isError(requiredType)) {
366367
errors.matchedTypeIsSubtypeOfRequired(
367368
pattern: pattern,
368369
matchedType: matchedValueType,

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10459,6 +10459,32 @@ main() {
1045910459
]),
1046010460
]);
1046110461
});
10462+
10463+
test('matched type is extension type', () {
10464+
h.addSuperInterfaces('E', (_) => [Type('Object?')]);
10465+
h.addExtensionTypeErasure('E', 'int');
10466+
var x = Var('x');
10467+
h.run([
10468+
ifCase(expr('E'), x.pattern(type: 'int'), [
10469+
checkReachable(true),
10470+
], [
10471+
checkReachable(false),
10472+
]),
10473+
]);
10474+
});
10475+
10476+
test('known type is extension type', () {
10477+
h.addSuperInterfaces('E', (_) => [Type('Object?')]);
10478+
h.addExtensionTypeErasure('E', 'int');
10479+
var x = Var('x');
10480+
h.run([
10481+
ifCase(expr('int'), x.pattern(type: 'E'), [
10482+
checkReachable(true),
10483+
], [
10484+
checkReachable(false),
10485+
]),
10486+
]);
10487+
});
1046210488
});
1046310489

1046410490
group("doesn't cover matched type:", () {

pkg/_fe_analyzer_shared/test/mini_ast.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2800,6 +2800,7 @@ class MiniAstOperations implements TypeAnalyzerOperations<Var, Type> {
28002800
fail('Unknown downward inference query: $query');
28012801
}
28022802

2803+
@override
28032804
Type extensionTypeErasure(Type type) {
28042805
var query = '$type';
28052806
return _extensionTypeErasure[query] ?? type;

pkg/_fe_analyzer_shared/test/type_inference/type_inference_test.dart

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,6 +1937,38 @@ main() {
19371937
]);
19381938
});
19391939
});
1940+
1941+
group('Fully covered due to extension type erasure:', () {
1942+
test('Cast to representation type', () {
1943+
// If an `as` pattern fully covers the matched value type due to
1944+
// extension type erasure, the "matchedTypeIsSubtypeOfRequired"
1945+
// warning should not be issued.
1946+
h.addSuperInterfaces('E', (_) => [Type('Object?')]);
1947+
h.addExtensionTypeErasure('E', 'int');
1948+
h.run([
1949+
ifCase(expr('E'), wildcard().as_('int'), [
1950+
checkReachable(true),
1951+
], [
1952+
checkReachable(false),
1953+
]),
1954+
]);
1955+
});
1956+
1957+
test('Cast to extension type', () {
1958+
// If an `as` pattern fully covers the matched value type due to
1959+
// extension type erasure, the "matchedTypeIsSubtypeOfRequired"
1960+
// warning should not be issued.
1961+
h.addSuperInterfaces('E', (_) => [Type('Object?')]);
1962+
h.addExtensionTypeErasure('E', 'int');
1963+
h.run([
1964+
ifCase(expr('int'), wildcard().as_('E'), [
1965+
checkReachable(true),
1966+
], [
1967+
checkReachable(false),
1968+
]),
1969+
]);
1970+
});
1971+
});
19401972
});
19411973

19421974
group('Const or literal:', () {

pkg/analyzer/lib/src/dart/resolver/flow_analysis_visitor.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,11 @@ class TypeSystemOperations
435435
}
436436
}
437437

438+
@override
439+
DartType extensionTypeErasure(DartType type) {
440+
return type.extensionTypeErasure;
441+
}
442+
438443
@override
439444
DartType factor(DartType from, DartType what) {
440445
return typeSystem.factor(from, what);

pkg/front_end/lib/src/fasta/type_inference/type_inference_engine.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,11 @@ class OperationsCfe
853853
return new InterfaceType(typeEnvironment.coreTypes.streamClass,
854854
Nullability.nonNullable, <DartType>[elementType]);
855855
}
856+
857+
@override
858+
DartType extensionTypeErasure(DartType type) {
859+
return type.extensionTypeErasure;
860+
}
856861
}
857862

858863
/// Type inference results used for testing.
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// SharedOptions=--enable-experiment=inline-class
6+
7+
// This test verifies that extension type erasure is used when computing whether
8+
// a pattern fully covers the matched value type. This is necessary to avoid a
9+
// "catch-22" situation in which flow analysis and the exhaustiveness checker
10+
// disagree about the exhaustiveness of a switch statement. For details see
11+
// https://github.com/dart-lang/language/issues/3534#issuecomment-1885839268.
12+
13+
import '../static_type_helper.dart';
14+
15+
extension type E(int i) {}
16+
17+
// Helper method used to verify that a local variable has been definitely
18+
// assigned.
19+
void checkAssigned(Object? o) {}
20+
21+
testIfCase(E e, int i) {
22+
int? j = 0; // promotes `j` to `int`
23+
j.expectStaticType<Exactly<int>>();
24+
25+
if (e case int _) {
26+
// reachable
27+
} else {
28+
// unreachable
29+
j = null; // demotes `j`
30+
j.expectStaticType<Exactly<int?>>();
31+
}
32+
// `j` is still promoted because the demotion is in an unreachable code path.
33+
j.expectStaticType<Exactly<int>>();
34+
35+
if (i case E _) {
36+
// reachable
37+
} else {
38+
// unreachable
39+
j = null; // demotes `j`
40+
j.expectStaticType<Exactly<int?>>();
41+
}
42+
// `j` is still promoted because the demotion is in an unreachable code path.
43+
j.expectStaticType<Exactly<int>>();
44+
}
45+
46+
testSwitchStatement(E e, int i) {
47+
{
48+
int j;
49+
switch (e) {
50+
case int _:
51+
j = 0;
52+
}
53+
// `int` fully covers `E`, so `j` has been definitely assigned now.
54+
checkAssigned(j);
55+
}
56+
57+
{
58+
int j;
59+
switch (i) {
60+
case E _:
61+
j = 0;
62+
}
63+
// `int` fully covers `E`, so `j` has been definitely assigned now.
64+
checkAssigned(j);
65+
}
66+
}
67+
68+
main() {
69+
testIfCase(E(0), 0);
70+
testSwitchStatement(E(0), 0);
71+
}

0 commit comments

Comments
 (0)