Skip to content

Commit 2414b29

Browse files
johnniwinthercommit-bot@chromium.org
johnniwinther
authored and
[email protected]
committed
[cfe] Throw error on exhaustive switch and expression of type Never
This requires a change to flow analysis that does not promote expressions to Never through is-tests and equality tests. Change-Id: Iec2ba5b78e61d205ad21dad6f07dbdb25fc746d3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/163380 Reviewed-by: Dmitry Stefantsov <[email protected]>
1 parent 55496a1 commit 2414b29

48 files changed

Lines changed: 2637 additions & 568 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,26 @@ abstract class FlowAnalysis<Node, Statement extends Node, Expression, Variable,
449449
/// collection element. See [forEach_bodyBegin] for details.
450450
void forEach_end();
451451

452+
/// Call this method to forward information on [oldExpression] to
453+
/// [newExpression].
454+
///
455+
/// This can be used to preserve promotions through a replacement from
456+
/// [oldExpression] to [newExpression]. For instance when rewriting
457+
///
458+
/// method(int i) {
459+
/// if (i is int) { ... } else { ... }
460+
/// }
461+
///
462+
/// to
463+
///
464+
/// method(int i) {
465+
/// if (i is int || throw ...) { ... } else { ... }
466+
/// }
467+
///
468+
/// the promotion `i is int` can be forwarded to `i is int || throw ...` and
469+
/// there preserved in the surrounding if statement.
470+
void forwardExpression(Expression newExpression, Expression oldExpression);
471+
452472
/// Call this method just before visiting the body of a function expression or
453473
/// local function.
454474
///
@@ -915,6 +935,12 @@ class FlowAnalysisDebug<Node, Statement extends Node, Expression, Variable,
915935
return _wrap('forEach_end()', () => _wrapped.forEach_end());
916936
}
917937

938+
@override
939+
void forwardExpression(Expression newExpression, Expression oldExpression) {
940+
return _wrap('forwardExpression($newExpression, $oldExpression)',
941+
() => _wrapped.forwardExpression(newExpression, oldExpression));
942+
}
943+
918944
@override
919945
void functionExpression_begin(Node node) {
920946
_wrap('functionExpression_begin($node)',
@@ -2655,6 +2681,13 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
26552681
_current = _join(_current, context._previous);
26562682
}
26572683

2684+
@override
2685+
void forwardExpression(Expression newExpression, Expression oldExpression) {
2686+
if (identical(_expressionWithInfo, oldExpression)) {
2687+
_expressionWithInfo = newExpression;
2688+
}
2689+
}
2690+
26582691
@override
26592692
void functionExpression_begin(Node node) {
26602693
AssignedVariablesNodeInfo<Variable> info =
@@ -2885,9 +2918,7 @@ class _FlowAnalysisImpl<Node, Statement extends Node, Expression, Variable,
28852918
@override
28862919
void parenthesizedExpression(
28872920
Expression outerExpression, Expression innerExpression) {
2888-
if (identical(_expressionWithInfo, innerExpression)) {
2889-
_expressionWithInfo = outerExpression;
2890-
}
2921+
forwardExpression(outerExpression, innerExpression);
28912922
}
28922923

28932924
@override

‎pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6367,6 +6367,45 @@ const MessageCode messageNativeClauseShouldBeAnnotation = const MessageCode(
63676367
tip:
63686368
r"""Try removing this native clause and adding @native() or @native('native-name') before the declaration.""");
63696369

6370+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
6371+
const Code<Null> codeNeverReachableSwitchDefaultError =
6372+
messageNeverReachableSwitchDefaultError;
6373+
6374+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
6375+
const MessageCode messageNeverReachableSwitchDefaultError = const MessageCode(
6376+
"NeverReachableSwitchDefaultError",
6377+
message:
6378+
r"""`null` encountered as case in a switch expression with a non-nullable enum type.""");
6379+
6380+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
6381+
const Code<Null> codeNeverReachableSwitchDefaultWarning =
6382+
messageNeverReachableSwitchDefaultWarning;
6383+
6384+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
6385+
const MessageCode messageNeverReachableSwitchDefaultWarning = const MessageCode(
6386+
"NeverReachableSwitchDefaultWarning",
6387+
severity: Severity.warning,
6388+
message:
6389+
r"""The default case is not reachable with sound null safety because the switch expression is non-nullable.""");
6390+
6391+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
6392+
const Code<Null> codeNeverValueError = messageNeverValueError;
6393+
6394+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
6395+
const MessageCode messageNeverValueError = const MessageCode("NeverValueError",
6396+
message:
6397+
r"""`null` encountered as the result from expression with type `Never`.""");
6398+
6399+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
6400+
const Code<Null> codeNeverValueWarning = messageNeverValueWarning;
6401+
6402+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
6403+
const MessageCode messageNeverValueWarning = const MessageCode(
6404+
"NeverValueWarning",
6405+
severity: Severity.warning,
6406+
message:
6407+
r"""The expression can not result in a value with sound null safety because the expression type is `Never`.""");
6408+
63706409
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
63716410
const Template<
63726411
Message Function(Token token)> templateNoFormals = const Template<

‎pkg/front_end/lib/src/api_prototype/compiler_options.dart‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,10 @@ class CompilerOptions {
233233
/// 'non-nullable' is enabled.
234234
NnbdMode nnbdMode = NnbdMode.Weak;
235235

236+
/// Whether to emit a warning when a ReachabilityError is thrown to ensure
237+
/// soundness in mixed mode.
238+
bool warnOnReachabilityCheck = false;
239+
236240
/// The current sdk version string, e.g. "2.6.0-edge.sha1hash".
237241
/// For instance used for language versioning (specifying the maximum
238242
/// version).

‎pkg/front_end/lib/src/base/command_line_options.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,4 +40,5 @@ class Flags {
4040
static const String singleRootScheme = "--single-root-scheme";
4141
static const String verbose = "--verbose";
4242
static const String verify = "--verify";
43+
static const String warnOnReachabilityCheck = "--warn-on-reachability-check";
4344
}

‎pkg/front_end/lib/src/base/processed_options.dart‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,8 @@ class ProcessedOptions {
181181

182182
NnbdMode get nnbdMode => _raw.nnbdMode;
183183

184+
bool get warnOnReachabilityCheck => _raw.warnOnReachabilityCheck;
185+
184186
/// The entry-points provided to the compiler.
185187
final List<Uri> inputs;
186188

‎pkg/front_end/lib/src/fasta/kernel/inference_visitor.dart‎

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ class InferenceVisitor
355355
for (Expression expression in node.expressions) {
356356
expressionResults.add(inferrer.inferExpression(
357357
expression, const UnknownType(), !inferrer.isTopLevel,
358-
isVoidAllowed: true));
358+
isVoidAllowed: true, forEffect: true));
359359
}
360360
List<Statement> body = [];
361361
for (int index = 0; index < expressionResults.length; index++) {
@@ -711,7 +711,7 @@ class InferenceVisitor
711711
StatementInferenceResult visitExpressionStatement(ExpressionStatement node) {
712712
ExpressionInferenceResult result = inferrer.inferExpression(
713713
node.expression, const UnknownType(), !inferrer.isTopLevel,
714-
isVoidAllowed: true);
714+
isVoidAllowed: true, forEffect: true);
715715
node.expression = result.expression..parent = node;
716716
return const StatementInferenceResult();
717717
}
@@ -5083,7 +5083,11 @@ class InferenceVisitor
50835083
@override
50845084
ExpressionInferenceResult visitRethrow(Rethrow node, DartType typeContext) {
50855085
inferrer.flowAnalysis.handleExit();
5086-
return new ExpressionInferenceResult(const BottomType(), node);
5086+
return new ExpressionInferenceResult(
5087+
inferrer.isNonNullableByDefault
5088+
? const NeverType(Nullability.nonNullable)
5089+
: const BottomType(),
5090+
node);
50875091
}
50885092

50895093
@override
@@ -5375,6 +5379,7 @@ class InferenceVisitor
53755379
inferrer.flowAnalysis.switchStatement_expressionEnd(node);
53765380

53775381
bool hasDefault = false;
5382+
bool lastCaseTerminates = true;
53785383
for (int caseIndex = 0; caseIndex < node.cases.length; ++caseIndex) {
53795384
SwitchCaseImpl switchCase = node.cases[caseIndex];
53805385
hasDefault = hasDefault || switchCase.isDefault;
@@ -5441,19 +5446,56 @@ class InferenceVisitor
54415446
switchCase.body = bodyResult.statement..parent = switchCase;
54425447
}
54435448

5444-
if (!inferrer.isTopLevel && inferrer.isNonNullableByDefault) {
5445-
// The last case block is allowed to complete normally.
5446-
if (caseIndex < node.cases.length - 1 &&
5447-
inferrer.flowAnalysis.isReachable) {
5448-
inferrer.library.addProblem(messageSwitchCaseFallThrough,
5449-
switchCase.fileOffset, noLength, inferrer.helper.uri);
5449+
if (inferrer.isNonNullableByDefault) {
5450+
lastCaseTerminates = !inferrer.flowAnalysis.isReachable;
5451+
if (!inferrer.isTopLevel) {
5452+
// The last case block is allowed to complete normally.
5453+
if (caseIndex < node.cases.length - 1 &&
5454+
inferrer.flowAnalysis.isReachable) {
5455+
inferrer.library.addProblem(messageSwitchCaseFallThrough,
5456+
switchCase.fileOffset, noLength, inferrer.helper.uri);
5457+
}
54505458
}
54515459
}
54525460
}
54535461
bool isExhaustive =
54545462
hasDefault || (enumFields != null && enumFields.isEmpty);
54555463
inferrer.flowAnalysis.switchStatement_end(isExhaustive);
5456-
return const StatementInferenceResult();
5464+
Statement replacement;
5465+
if (isExhaustive &&
5466+
!hasDefault &&
5467+
inferrer.shouldThrowUnsoundnessException) {
5468+
if (!lastCaseTerminates) {
5469+
LabeledStatement breakTarget;
5470+
if (node.parent is LabeledStatement) {
5471+
breakTarget = node.parent;
5472+
} else {
5473+
replacement = breakTarget = new LabeledStatement(node);
5474+
}
5475+
5476+
SwitchCase lastCase = node.cases.last;
5477+
Statement body = lastCase.body;
5478+
if (body is Block) {
5479+
body.statements.add(new BreakStatementImpl(isContinue: false)
5480+
..target = breakTarget
5481+
..targetStatement = node
5482+
..fileOffset = node.fileOffset);
5483+
}
5484+
}
5485+
node.cases.add(new SwitchCase(
5486+
[],
5487+
[],
5488+
_createExpressionStatement(inferrer.createReachabilityError(
5489+
node.fileOffset,
5490+
messageNeverReachableSwitchDefaultError,
5491+
messageNeverReachableSwitchDefaultWarning)),
5492+
isDefault: true)
5493+
..fileOffset = node.fileOffset
5494+
..parent = node);
5495+
}
5496+
return replacement != null
5497+
? new StatementInferenceResult.single(replacement)
5498+
: const StatementInferenceResult();
54575499
}
54585500

54595501
@override

‎pkg/front_end/lib/src/fasta/source/source_loader.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1486,6 +1486,9 @@ class Symbol {
14861486
}
14871487
14881488
T unsafeCast<T>(Object v) {}
1489+
class ReachabilityError {
1490+
ReachabilityError([message]);
1491+
}
14891492
""";
14901493

14911494
/// A minimal implementation of dart:typed_data that is sufficient to create an

‎pkg/front_end/lib/src/fasta/type_inference/type_inferrer.dart‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,33 @@ class TypeInferrerImpl implements TypeInferrer {
275275
return type.withDeclaredNullability(library.nonNullable);
276276
}
277277

278+
Expression createReachabilityError(
279+
int fileOffset, Message errorMessage, Message warningMessage) {
280+
if (library.loader.target.context.options.warnOnReachabilityCheck &&
281+
warningMessage != null) {
282+
helper?.addProblem(warningMessage, fileOffset, noLength);
283+
}
284+
Arguments arguments;
285+
if (errorMessage != null) {
286+
arguments = new Arguments(
287+
[new StringLiteral(errorMessage.message)..fileOffset = fileOffset])
288+
..fileOffset = fileOffset;
289+
} else {
290+
arguments = new Arguments([])..fileOffset = fileOffset;
291+
}
292+
assert(coreTypes.reachabilityErrorConstructor != null);
293+
return new Throw(
294+
new ConstructorInvocation(
295+
coreTypes.reachabilityErrorConstructor, arguments)
296+
..fileOffset = fileOffset)
297+
..fileOffset = fileOffset;
298+
}
299+
300+
/// Returns `true` if exceptions should be thrown in paths reachable only due
301+
/// to unsoundness in flow analysis in mixed mode.
302+
bool get shouldThrowUnsoundnessException =>
303+
isNonNullableByDefault && nnbdMode != NnbdMode.Strong;
304+
278305
void registerIfUnreachableForTesting(TreeNode node, {bool isReachable}) {
279306
if (dataForTesting == null) return;
280307
isReachable ??= flowAnalysis.isReachable;
@@ -1561,7 +1588,7 @@ class TypeInferrerImpl implements TypeInferrer {
15611588
/// the expression type and calls the appropriate specialized "infer" method.
15621589
ExpressionInferenceResult inferExpression(
15631590
Expression expression, DartType typeContext, bool typeNeeded,
1564-
{bool isVoidAllowed: false}) {
1591+
{bool isVoidAllowed: false, bool forEffect: false}) {
15651592
registerIfUnreachableForTesting(expression);
15661593

15671594
// `null` should never be used as the type context. An instance of
@@ -1597,6 +1624,19 @@ class TypeInferrerImpl implements TypeInferrer {
15971624
}
15981625
if (coreTypes.isBottom(result.inferredType)) {
15991626
flowAnalysis.handleExit();
1627+
if (shouldThrowUnsoundnessException &&
1628+
// Don't throw on expressions that inherently return the bottom type.
1629+
!(result.nullAwareAction is Throw ||
1630+
result.nullAwareAction is Rethrow ||
1631+
result.nullAwareAction is InvalidExpression)) {
1632+
Expression replacement = createLet(
1633+
createVariable(result.expression, result.inferredType),
1634+
createReachabilityError(expression.fileOffset,
1635+
messageNeverValueError, messageNeverValueWarning));
1636+
flowAnalysis.forwardExpression(replacement, result.expression);
1637+
result =
1638+
new ExpressionInferenceResult(result.inferredType, replacement);
1639+
}
16001640
}
16011641
return result;
16021642
}

‎pkg/front_end/messages.status‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,14 @@ NamedFunctionExpression/example: Fail
497497
NamedMixinOverride/analyzerCode: Fail
498498
NamedMixinOverride/example: Fail
499499
NativeClauseShouldBeAnnotation/example: Fail
500+
NeverReachableSwitchDefaultError/analyzerCode: Fail
501+
NeverReachableSwitchDefaultError/example: Fail
502+
NeverReachableSwitchDefaultWarning/analyzerCode: Fail
503+
NeverReachableSwitchDefaultWarning/example: Fail
504+
NeverValueError/analyzerCode: Fail
505+
NeverValueError/example: Fail
506+
NeverValueWarning/analyzerCode: Fail
507+
NeverValueWarning/example: Fail
500508
NoFormals/example: Fail
501509
NonAgnosticConstant/analyzerCode: Fail
502510
NonAgnosticConstant/example: Fail

‎pkg/front_end/messages.yaml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4307,3 +4307,17 @@ NonVoidReturnSetter:
43074307
script:
43084308
- int set setter(_) {}
43094309
- dynamic set setter(_) {}
4310+
4311+
NeverReachableSwitchDefaultError:
4312+
template: "`null` encountered as case in a switch expression with a non-nullable enum type."
4313+
4314+
NeverReachableSwitchDefaultWarning:
4315+
template: "The default case is not reachable with sound null safety because the switch expression is non-nullable."
4316+
severity: WARNING
4317+
4318+
NeverValueError:
4319+
template: "`null` encountered as the result from expression with type `Never`."
4320+
4321+
NeverValueWarning:
4322+
template: "The expression can not result in a value with sound null safety because the expression type is `Never`."
4323+
severity: WARNING

0 commit comments

Comments
 (0)