dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Fix joining variables in switch statement.

Browse Source 
Instead of joining pattern variables in shared case scope after every
top-level pattern, we now accumulate them, and join at the end of the
shared scope.

Change-Id: Icc1d426e53ccfe48a72e3194e1d7e96e6e90d781
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279264
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Paul Berry <paulberry@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This commit is contained in:
Konstantin Shcheglov
2023-01-18 19:32:42 +00:00
committed by Commit Queue
parent b41c70463e
commit 68c32274c7
4 changed files with 270 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/_fe_analyzer_shared/lib/src/type_inference/variable_bindings.dart
+64 -53
View File
@@ -67,45 +67,9 @@ abstract class VariableBinder<Node extends Object, Variable extends Object> {
Map<String, Variable> variables = _variables.removeLast();
if (sharedCaseScopeKey != null) {
Map<String, Variable> right = variables;
_SharedCaseScope<Variable> sharedScope = _sharedCaseScopes.last;
assert(sharedScope.key == sharedCaseScopeKey);
Map<String, Variable>? left = sharedScope.variables;
if (left == null) {
sharedScope.variables = right;
} else {
Map<String, Variable> result = {};
for (MapEntry<String, Variable> leftEntry in left.entries) {
String name = leftEntry.key;
Variable leftVariable = leftEntry.value;
Variable? rightVariable = right[name];
if (rightVariable != null) {
result[name] = joinPatternVariables(
key: sharedCaseScopeKey,
components: [leftVariable, rightVariable],
isConsistent: true,
);
} else {
result[name] = joinPatternVariables(
key: sharedCaseScopeKey,
components: [leftVariable],
isConsistent: false,
);
}
}
for (MapEntry<String, Variable> rightEntry in right.entries) {
String name = rightEntry.key;
Variable rightVariable = rightEntry.value;
if (!left.containsKey(name)) {
result[name] = joinPatternVariables(
key: sharedCaseScopeKey,
components: [rightVariable],
isConsistent: false,
);
}
}
sharedScope.variables = result;
}
sharedScope.addAll(variables);
}
return variables;
@@ -201,20 +165,7 @@ abstract class VariableBinder<Node extends Object, Variable extends Object> {
void switchStatementSharedCaseScopeEmpty(Object key) {
_SharedCaseScope<Variable> sharedScope = _sharedCaseScopes.last;
assert(sharedScope.key == key);
Map<String, Variable>? left = sharedScope.variables;
if (left != null) {
Map<String, Variable> result = {};
for (MapEntry<String, Variable> leftEntry in left.entries) {
String name = leftEntry.key;
Variable leftVariable = leftEntry.value;
result[name] = joinPatternVariables(
key: key,
components: [leftVariable],
isConsistent: false,
);
}
sharedScope.variables = result;
}
sharedScope.addAll({});
}
/// Notifies that computing of the shared case scope was finished, returns
@@ -226,7 +177,23 @@ abstract class VariableBinder<Node extends Object, Variable extends Object> {
assert(_variables.isEmpty);
_SharedCaseScope<Variable> sharedScope = _sharedCaseScopes.removeLast();
assert(sharedScope.key == key);
return sharedScope.variables ?? {};
Map<String, Variable> result = {};
for (MapEntry<String, _SharedCaseScopeVariable<Variable>> entry
in sharedScope.variables.entries) {
_SharedCaseScopeVariable<Variable> sharedVariable = entry.value;
List<Variable> variables = sharedVariable.variables;
if (sharedVariable.isConsistent && variables.length == 1) {
result[entry.key] = variables[0];
} else {
result[entry.key] = joinPatternVariables(
key: key,
components: variables,
isConsistent: sharedVariable.isConsistent,
);
}
}
return result;
}
/// Notifies that computing new shared case scope should be started.
@@ -262,7 +229,51 @@ abstract class VariableBinderErrors<Node extends Object,
class _SharedCaseScope<Variable extends Object> {
final Object key;
Map<String, Variable>? variables;
bool isEmpty = true;
Map<String, _SharedCaseScopeVariable<Variable>> variables = {};
_SharedCaseScope(this.key);
/// Adds [newVariables] to [variables], marking absent variables as not
/// consistent. If [isEmpty], just sets given variables as the starting set.
void addAll(Map<String, Variable> newVariables) {
if (isEmpty) {
isEmpty = false;
for (MapEntry<String, Variable> entry in newVariables.entries) {
String name = entry.key;
Variable variable = entry.value;
_getVariable(name).variables.add(variable);
}
} else {
for (MapEntry<String, _SharedCaseScopeVariable<Variable>> entry
in variables.entries) {
String name = entry.key;
_SharedCaseScopeVariable<Variable> variable = entry.value;
Variable? newVariable = newVariables[name];
if (newVariable != null) {
variable.variables.add(newVariable);
} else {
variable.isConsistent = false;
}
}
for (MapEntry<String, Variable> newEntry in newVariables.entries) {
String name = newEntry.key;
Variable newVariable = newEntry.value;
if (!variables.containsKey(name)) {
_getVariable(name)
..isConsistent = false
..variables.add(newVariable);
}
}
}
}
_SharedCaseScopeVariable _getVariable(String name) {
return variables[name] ??= new _SharedCaseScopeVariable();
}
}
class _SharedCaseScopeVariable<Variable extends Object> {
bool isConsistent = true;
final List<Variable> variables = [];
}
pkg/_fe_analyzer_shared/test/type_inference/variable_bindings_test.dart
+28 -11
View File
@@ -113,15 +113,27 @@ main() {
expectedVariables: {'x: [1, 3]', 'y: notConsistent [2]'},
);
});
test('Has default', () {
h.runSwitchStatementSharedBody(
sharedCaseScopeKey: 0,
casePatterns: [
_VarPattern('x', 1),
],
hasDefault: true,
expectedVariables: {'x: notConsistent [1]'},
);
group('Has default', () {
test('First', () {
h.runSwitchStatementSharedBody(
sharedCaseScopeKey: 0,
casePatterns: [
_VarPattern('x', 1),
],
hasDefaultFirst: true, // does not happen normally
expectedVariables: {'x: notConsistent [1]'},
);
});
test('Last', () {
h.runSwitchStatementSharedBody(
sharedCaseScopeKey: 0,
casePatterns: [
_VarPattern('x', 1),
],
hasDefaultLast: true,
expectedVariables: {'x: notConsistent [1]'},
);
});
});
group('With logical-or', () {
test('Both have', () {
@@ -254,11 +266,16 @@ class _Harness {
void runSwitchStatementSharedBody({
required Object sharedCaseScopeKey,
required List<_Node> casePatterns,
bool hasDefault = false,
bool hasDefaultFirst = false,
bool hasDefaultLast = false,
List<String> expectErrors = const [],
required Set<String> expectedVariables,
}) {
assert(!(hasDefaultFirst && hasDefaultLast));
_binder.switchStatementSharedCaseScopeStart(sharedCaseScopeKey);
if (hasDefaultFirst) {
_binder.switchStatementSharedCaseScopeEmpty(sharedCaseScopeKey);
}
for (var casePattern in casePatterns) {
_binder.casePatternStart();
casePattern._visit(this);
@@ -266,7 +283,7 @@ class _Harness {
sharedCaseScopeKey: sharedCaseScopeKey,
);
}
if (hasDefault) {
if (hasDefaultLast) {
_binder.switchStatementSharedCaseScopeEmpty(sharedCaseScopeKey);
}
var variables =
pkg/analyzer/lib/src/dart/resolver/resolution_visitor.dart
+13 -11
View File
@@ -1230,27 +1230,27 @@ class ResolutionVisitor extends RecursiveAstVisitor<void> {
node.expression.accept(this);
for (var group in node.memberGroups) {
_patternVariables.switchStatementSharedCaseScopeStart(node);
_patternVariables.switchStatementSharedCaseScopeStart(group);
for (var member in group.members) {
_buildLabelElements(member.labels, true);
if (member is SwitchCaseImpl) {
member.expression.accept(this);
} else if (member is SwitchDefaultImpl) {
_patternVariables.switchStatementSharedCaseScopeEmpty(node);
_patternVariables.switchStatementSharedCaseScopeEmpty(group);
} else if (member is SwitchPatternCaseImpl) {
_resolveGuardedPattern(
member.guardedPattern,
sharedCaseScopeKey: node,
sharedCaseScopeKey: group,
);
} else {
throw UnimplementedError('(${member.runtimeType}) $member');
}
}
if (group.hasLabels) {
_patternVariables.switchStatementSharedCaseScopeEmpty(node);
_patternVariables.switchStatementSharedCaseScopeEmpty(group);
}
group.variables =
_patternVariables.switchStatementSharedCaseScopeFinish(node);
_patternVariables.switchStatementSharedCaseScopeFinish(group);
_withNameScope(() {
var statements = group.statements;
_buildLocalElements(statements);
@@ -1688,18 +1688,20 @@ class _VariableBinder
var first = components.first;
List<PatternVariableElementImpl> expandedVariables;
if (key is LogicalOrPatternImpl) {
expandedVariables = components.expand((component) {
component as PatternVariableElementImpl;
if (component is JoinPatternVariableElementImpl) {
return component.variables;
expandedVariables = components.expand((variable) {
variable as PatternVariableElementImpl;
if (variable is JoinPatternVariableElementImpl) {
return variable.variables;
} else {
return [component];
return [variable];
}
}).toList(growable: false);
} else {
} else if (key is SwitchStatementCaseGroup) {
expandedVariables = components
.map((e) => e as PatternVariableElementImpl)
.toList(growable: false);
} else {
throw UnimplementedError('(${key.runtimeType}) $key');
}
return JoinPatternVariableElementImpl(
first.name,
pkg/analyzer/test/src/dart/resolution/switch_statement_test.dart
+165
View File
@@ -1064,6 +1064,67 @@ SwitchStatement
''');
}
test_variables_joinedCase_hasDefault2() async {
await assertErrorsInCode(r'''
void f(Object? x) {
switch (x) {
case var a:
case var a:
default:
a;
}
}
''', [
error(
CompileTimeErrorCode.INCONSISTENT_PATTERN_VARIABLE_SHARED_CASE_SCOPE,
86,
1),
]);
final node = findNode.switchStatement('switch');
assertResolvedNodeText(node, r'''
SwitchStatement
switchKeyword: switch
leftParenthesis: (
expression: SimpleIdentifier
token: x
staticElement: self::@function::f::@parameter::x
staticType: Object?
rightParenthesis: )
leftBracket: {
members
SwitchPatternCase
keyword: case
guardedPattern: GuardedPattern
pattern: DeclaredVariablePattern
keyword: var
name: a
declaredElement: hasImplicitType a@48
type: Object?
colon: :
SwitchPatternCase
keyword: case
guardedPattern: GuardedPattern
pattern: DeclaredVariablePattern
keyword: var
name: a
declaredElement: hasImplicitType a@64
type: Object?
colon: :
SwitchDefault
keyword: default
colon: :
statements
ExpressionStatement
expression: SimpleIdentifier
token: a
staticElement: notConsistent a[a@48, a@64]
staticType: Object?
semicolon: ;
rightBracket: }
''');
}
test_variables_joinedCase_hasLabel() async {
await assertErrorsInCode(r'''
void f(Object? x) {
@@ -1140,6 +1201,110 @@ SwitchStatement
''');
}
test_variables_joinedCase_notConsistent3() async {
await assertErrorsInCode(r'''
void f(Object? x) {
switch (x) {
case int a:
case double b:
case String c:
a;
b;
c;
}
}
''', [
error(
CompileTimeErrorCode.INCONSISTENT_PATTERN_VARIABLE_SHARED_CASE_SCOPE,
95,
1),
error(
CompileTimeErrorCode.INCONSISTENT_PATTERN_VARIABLE_SHARED_CASE_SCOPE,
104,
1),
error(
CompileTimeErrorCode.INCONSISTENT_PATTERN_VARIABLE_SHARED_CASE_SCOPE,
113,
1),
]);
final node = findNode.switchStatement('switch');
assertResolvedNodeText(node, r'''
SwitchStatement
switchKeyword: switch
leftParenthesis: (
expression: SimpleIdentifier
token: x
staticElement: self::@function::f::@parameter::x
staticType: Object?
rightParenthesis: )
leftBracket: {
members
SwitchPatternCase
keyword: case
guardedPattern: GuardedPattern
pattern: DeclaredVariablePattern
type: NamedType
name: SimpleIdentifier
token: int
staticElement: dart:core::@class::int
staticType: null
type: int
name: a
declaredElement: a@48
type: int
colon: :
SwitchPatternCase
keyword: case
guardedPattern: GuardedPattern
pattern: DeclaredVariablePattern
type: NamedType
name: SimpleIdentifier
token: double
staticElement: dart:core::@class::double
staticType: null
type: double
name: b
declaredElement: b@67
type: double
colon: :
SwitchPatternCase
keyword: case
guardedPattern: GuardedPattern
pattern: DeclaredVariablePattern
type: NamedType
name: SimpleIdentifier
token: String
staticElement: dart:core::@class::String
staticType: null
type: String
name: c
declaredElement: c@86
type: String
colon: :
statements
ExpressionStatement
expression: SimpleIdentifier
token: a
staticElement: notConsistent a[a@48]
staticType: int
semicolon: ;
ExpressionStatement
expression: SimpleIdentifier
token: b
staticElement: notConsistent b[b@67]
staticType: double
semicolon: ;
ExpressionStatement
expression: SimpleIdentifier
token: c
staticElement: notConsistent c[c@86]
staticType: String
semicolon: ;
rightBracket: }
''');
}
test_variables_logicalOr() async {
await assertNoErrorsInCode(r'''
void f(Object? x) {