mirror of
https://github.com/dart-lang/sdk
synced 2024-09-15 21:40:07 +00:00
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 https://github.com/dart-lang/language/issues/3534#issuecomment-1885839268. Fixes https://github.com/dart-lang/language/issues/3534. Bug: https://github.com/dart-lang/language/issues/3534 Change-Id: Ib73d191e04e7fa8c0e6888c2733dae73d8f389da Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/345822 Reviewed-by: Chloe Stefantsova <cstefantsova@google.com> Commit-Queue: Paul Berry <paulberry@google.com>
This commit is contained in:
parent
89060a64e9
commit
97edad1568
|
@ -5159,7 +5159,9 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
|
|||
_PatternContext<Type> context = _stack.last as _PatternContext<Type>;
|
||||
_Reference<Type> matchedValueReference =
|
||||
context.createReference(matchedType, _current);
|
||||
bool coversMatchedType = operations.isSubtypeOf(matchedType, knownType);
|
||||
bool coversMatchedType = operations.isSubtypeOf(
|
||||
operations.extensionTypeErasure(matchedType),
|
||||
operations.extensionTypeErasure(knownType));
|
||||
// Promote the synthetic cache variable the pattern is being matched
|
||||
// against.
|
||||
ExpressionInfo<Type> promotionInfo =
|
||||
|
|
|
@ -55,6 +55,10 @@ abstract interface class FlowAnalysisTypeOperations<Type extends Object> {
|
|||
/// the [TypeClassification] enum.
|
||||
TypeClassification classifyType(Type type);
|
||||
|
||||
/// If [type] is an extension type, returns the ultimate representation type.
|
||||
/// Otherwise returns [type] as is.
|
||||
Type extensionTypeErasure(Type type);
|
||||
|
||||
/// Returns the "remainder" of [from] when [what] has been removed from
|
||||
/// consideration by an instance check.
|
||||
Type factor(Type from, Type what);
|
||||
|
|
|
@ -360,11 +360,12 @@ mixin TypeAnalyzer<
|
|||
required Type requiredType,
|
||||
}) {
|
||||
Type matchedValueType = flow.getMatchedValueType();
|
||||
bool matchedTypeIsSubtypeOfRequired = flow.promoteForPattern(
|
||||
flow.promoteForPattern(
|
||||
matchedType: matchedValueType,
|
||||
knownType: requiredType,
|
||||
matchFailsIfWrongType: false);
|
||||
if (matchedTypeIsSubtypeOfRequired) {
|
||||
if (operations.isSubtypeOf(matchedValueType, requiredType) &&
|
||||
!operations.isError(requiredType)) {
|
||||
errors.matchedTypeIsSubtypeOfRequired(
|
||||
pattern: pattern,
|
||||
matchedType: matchedValueType,
|
||||
|
|
|
@ -64,10 +64,6 @@ abstract interface class TypeAnalyzerOperations<Variable extends Object,
|
|||
/// If [type] is a record type, returns it.
|
||||
RecordType<Type>? asRecordType(Type type);
|
||||
|
||||
/// If [type] is an extension type, returns the ultimate representation type.
|
||||
/// Otherwise returns [type] as is.
|
||||
Type extensionTypeErasure(Type type);
|
||||
|
||||
/// Computes the greatest lower bound of [type1] and [type2].
|
||||
Type glb(Type type1, Type type2);
|
||||
|
||||
|
|
|
@ -10459,6 +10459,32 @@ main() {
|
|||
]),
|
||||
]);
|
||||
});
|
||||
|
||||
test('matched type is extension type', () {
|
||||
h.addSuperInterfaces('E', (_) => [Type('Object?')]);
|
||||
h.addExtensionTypeErasure('E', 'int');
|
||||
var x = Var('x');
|
||||
h.run([
|
||||
ifCase(expr('E'), x.pattern(type: 'int'), [
|
||||
checkReachable(true),
|
||||
], [
|
||||
checkReachable(false),
|
||||
]),
|
||||
]);
|
||||
});
|
||||
|
||||
test('known type is extension type', () {
|
||||
h.addSuperInterfaces('E', (_) => [Type('Object?')]);
|
||||
h.addExtensionTypeErasure('E', 'int');
|
||||
var x = Var('x');
|
||||
h.run([
|
||||
ifCase(expr('int'), x.pattern(type: 'E'), [
|
||||
checkReachable(true),
|
||||
], [
|
||||
checkReachable(false),
|
||||
]),
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
group("doesn't cover matched type:", () {
|
||||
|
|
|
@ -1937,6 +1937,38 @@ main() {
|
|||
]);
|
||||
});
|
||||
});
|
||||
|
||||
group('Fully covered due to extension type erasure:', () {
|
||||
test('Cast to representation type', () {
|
||||
// If an `as` pattern fully covers the matched value type due to
|
||||
// extension type erasure, the "matchedTypeIsSubtypeOfRequired"
|
||||
// warning should not be issued.
|
||||
h.addSuperInterfaces('E', (_) => [Type('Object?')]);
|
||||
h.addExtensionTypeErasure('E', 'int');
|
||||
h.run([
|
||||
ifCase(expr('E'), wildcard().as_('int'), [
|
||||
checkReachable(true),
|
||||
], [
|
||||
checkReachable(false),
|
||||
]),
|
||||
]);
|
||||
});
|
||||
|
||||
test('Cast to extension type', () {
|
||||
// If an `as` pattern fully covers the matched value type due to
|
||||
// extension type erasure, the "matchedTypeIsSubtypeOfRequired"
|
||||
// warning should not be issued.
|
||||
h.addSuperInterfaces('E', (_) => [Type('Object?')]);
|
||||
h.addExtensionTypeErasure('E', 'int');
|
||||
h.run([
|
||||
ifCase(expr('int'), wildcard().as_('E'), [
|
||||
checkReachable(true),
|
||||
], [
|
||||
checkReachable(false),
|
||||
]),
|
||||
]);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
group('Const or literal:', () {
|
||||
|
|
71
tests/language/extension_type/fully_covers_test.dart
Normal file
71
tests/language/extension_type/fully_covers_test.dart
Normal file
|
@ -0,0 +1,71 @@
|
|||
// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
|
||||
// for details. All rights reserved. Use of this source code is governed by a
|
||||
// BSD-style license that can be found in the LICENSE file.
|
||||
|
||||
// SharedOptions=--enable-experiment=inline-class
|
||||
|
||||
// This test verifies that extension type erasure is used when computing whether
|
||||
// a pattern fully covers the matched value type. This is necessary to avoid a
|
||||
// "catch-22" situation in which flow analysis and the exhaustiveness checker
|
||||
// disagree about the exhaustiveness of a switch statement. For details see
|
||||
// https://github.com/dart-lang/language/issues/3534#issuecomment-1885839268.
|
||||
|
||||
import '../static_type_helper.dart';
|
||||
|
||||
extension type E(int i) {}
|
||||
|
||||
// Helper method used to verify that a local variable has been definitely
|
||||
// assigned.
|
||||
void checkAssigned(Object? o) {}
|
||||
|
||||
testIfCase(E e, int i) {
|
||||
int? j = 0; // promotes `j` to `int`
|
||||
j.expectStaticType<Exactly<int>>();
|
||||
|
||||
if (e case int _) {
|
||||
// reachable
|
||||
} else {
|
||||
// unreachable
|
||||
j = null; // demotes `j`
|
||||
j.expectStaticType<Exactly<int?>>();
|
||||
}
|
||||
// `j` is still promoted because the demotion is in an unreachable code path.
|
||||
j.expectStaticType<Exactly<int>>();
|
||||
|
||||
if (i case E _) {
|
||||
// reachable
|
||||
} else {
|
||||
// unreachable
|
||||
j = null; // demotes `j`
|
||||
j.expectStaticType<Exactly<int?>>();
|
||||
}
|
||||
// `j` is still promoted because the demotion is in an unreachable code path.
|
||||
j.expectStaticType<Exactly<int>>();
|
||||
}
|
||||
|
||||
testSwitchStatement(E e, int i) {
|
||||
{
|
||||
int j;
|
||||
switch (e) {
|
||||
case int _:
|
||||
j = 0;
|
||||
}
|
||||
// `int` fully covers `E`, so `j` has been definitely assigned now.
|
||||
checkAssigned(j);
|
||||
}
|
||||
|
||||
{
|
||||
int j;
|
||||
switch (i) {
|
||||
case E _:
|
||||
j = 0;
|
||||
}
|
||||
// `int` fully covers `E`, so `j` has been definitely assigned now.
|
||||
checkAssigned(j);
|
||||
}
|
||||
}
|
||||
|
||||
main() {
|
||||
testIfCase(E(0), 0);
|
||||
testSwitchStatement(E(0), 0);
|
||||
}
|
Loading…
Reference in a new issue