Skip to content

Commit 265f6fc

Browse files
stereotype441commit-bot@chromium.org
authored andcommitted
Flow analysis: Fix "why not promoted" functionality for implicit this
Bug: #44898 Change-Id: Ief6014c34aef329e5f408916f5d8ffe14a62effd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/187940 Reviewed-by: Konstantin Shcheglov <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent eb58f3d commit 265f6fc

File tree

5 files changed

+124
-34
lines changed

5 files changed

+124
-34
lines changed

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,8 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
935935
/// promotion, to retrieve information about why [target] was not promoted.
936936
/// This call must be made right after visiting [target].
937937
///
938+
/// If [target] is `null` it is assumed to be an implicit reference to `this`.
939+
///
938940
/// The returned value is a map whose keys are types that the user might have
939941
/// been expecting the target to be promoted to, and whose values are reasons
940942
/// why the corresponding promotion did not occur. The caller is expected to
@@ -943,7 +945,7 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
943945
/// occurs due to the target having a nullable type, the caller should report
944946
/// a non-promotion reason associated with non-promotion to a non-nullable
945947
/// type).
946-
Map<Type, NonPromotionReason> whyNotPromoted(Expression target);
948+
Map<Type, NonPromotionReason> whyNotPromoted(Expression? target);
947949

948950
/// Register write of the given [variable] in the current state.
949951
/// [writtenType] should be the type of the value that was written.
@@ -1460,7 +1462,7 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
14601462
}
14611463

14621464
@override
1463-
Map<Type, NonPromotionReason> whyNotPromoted(Expression target) {
1465+
Map<Type, NonPromotionReason> whyNotPromoted(Expression? target) {
14641466
return _wrap(
14651467
'whyNotPromoted($target)', () => _wrapped.whyNotPromoted(target),
14661468
isQuery: true);
@@ -4257,13 +4259,16 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
42574259
}
42584260

42594261
@override
4260-
Map<Type, NonPromotionReason> whyNotPromoted(Expression target) {
4261-
if (identical(target, _expressionWithReference)) {
4262-
Reference<Variable, Type>? reference = _expressionReference;
4263-
if (reference != null) {
4264-
return reference.getNonPromotionReasons(
4265-
_current.variableInfo, typeOperations);
4266-
}
4262+
Map<Type, NonPromotionReason> whyNotPromoted(Expression? target) {
4263+
Reference<Variable, Type>? reference;
4264+
if (target == null) {
4265+
reference = new _ThisReference<Variable, Type>();
4266+
} else if (identical(target, _expressionWithReference)) {
4267+
reference = _expressionReference;
4268+
}
4269+
if (reference != null) {
4270+
return reference.getNonPromotionReasons(
4271+
_current.variableInfo, typeOperations);
42674272
}
42684273
return {};
42694274
}
@@ -4850,7 +4855,7 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
48504855
void whileStatement_end() {}
48514856

48524857
@override
4853-
Map<Type, NonPromotionReason> whyNotPromoted(Expression target) {
4858+
Map<Type, NonPromotionReason> whyNotPromoted(Expression? target) {
48544859
return {};
48554860
}
48564861

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_mini_ast.dart

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,10 @@ Statement if_(Expression condition, List<Statement> ifTrue,
144144
[List<Statement>? ifFalse]) =>
145145
new _If(condition, ifTrue, ifFalse);
146146

147+
Statement implicitThis_whyNotPromoted(
148+
void Function(Map<Type, NonPromotionReason>) callback) =>
149+
new _WhyNotPromoted_ImplicitThis(callback);
150+
147151
Statement labeled(Statement body) => new _LabeledStatement(body);
148152

149153
Statement localFunction(List<Statement> body) => _LocalFunction(body);
@@ -1760,6 +1764,30 @@ class _WhyNotPromoted extends Expression {
17601764
}
17611765
}
17621766

1767+
class _WhyNotPromoted_ImplicitThis extends Statement {
1768+
final void Function(Map<Type, NonPromotionReason>) callback;
1769+
1770+
_WhyNotPromoted_ImplicitThis(this.callback) : super._();
1771+
1772+
@override
1773+
String toString() => 'implicit this (whyNotPromoted)';
1774+
1775+
@override
1776+
void _preVisit(AssignedVariables<Node, Var> assignedVariables) {}
1777+
1778+
@override
1779+
void _visit(
1780+
Harness h, FlowAnalysis<Node, Statement, Expression, Var, Type> flow) {
1781+
assert(!Type._allowComparisons);
1782+
Type._allowComparisons = true;
1783+
try {
1784+
callback(flow.whyNotPromoted(null));
1785+
} finally {
1786+
Type._allowComparisons = false;
1787+
}
1788+
}
1789+
}
1790+
17631791
class _WrappedExpression extends Expression {
17641792
final Statement? before;
17651793
final Expression expr;

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5683,6 +5683,40 @@ main() {
56835683
]);
56845684
});
56855685
});
5686+
5687+
group('because this', () {
5688+
test('explicit', () {
5689+
var h = Harness()
5690+
..addSubtype('D', 'Object?', true)
5691+
..addFactor('Object?', 'D', 'Object?');
5692+
h.run([
5693+
if_(this_('C').isNot('D'), [
5694+
return_(),
5695+
]),
5696+
this_('C').whyNotPromoted((reasons) {
5697+
expect(reasons.keys, unorderedEquals([Type('D')]));
5698+
var nonPromotionReason = reasons.values.single;
5699+
expect(nonPromotionReason, TypeMatcher<ThisNotPromoted>());
5700+
}).stmt,
5701+
]);
5702+
});
5703+
5704+
test('implicit', () {
5705+
var h = Harness()
5706+
..addSubtype('D', 'Object?', true)
5707+
..addFactor('Object?', 'D', 'Object?');
5708+
h.run([
5709+
if_(this_('C').isNot('D'), [
5710+
return_(),
5711+
]),
5712+
implicitThis_whyNotPromoted((reasons) {
5713+
expect(reasons.keys, unorderedEquals([Type('D')]));
5714+
var nonPromotionReason = reasons.values.single;
5715+
expect(nonPromotionReason, TypeMatcher<ThisNotPromoted>());
5716+
}),
5717+
]);
5718+
});
5719+
});
56865720
});
56875721
}
56885722

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright (c) 2020, 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+
// This test validates integration of "why not promoted" when the user tries to
6+
// promote `this`.
7+
8+
// TODO(paulberry): once we support adding "why not promoted" information to
9+
// errors that aren't related to null safety, test references to `this` in
10+
// classes and mixins.
11+
12+
extension on int? {
13+
extension_explicit_this() {
14+
if (this == null) return;
15+
this. /*notPromoted(thisNotPromoted)*/ isEven;
16+
}
17+
18+
extension_implicit_this() {
19+
if (this == null) return;
20+
/*notPromoted(thisNotPromoted)*/ isEven;
21+
}
22+
}

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

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -516,29 +516,27 @@ class ResolverVisitor extends ScopedVisitor {
516516
List<DiagnosticMessage> computeWhyNotPromotedMessages(
517517
Expression? receiver, SyntacticEntity errorEntity) {
518518
List<DiagnosticMessage> messages = [];
519-
if (receiver != null) {
520-
var whyNotPromoted = flowAnalysis?.flow?.whyNotPromoted(receiver);
521-
if (whyNotPromoted != null) {
522-
for (var entry in whyNotPromoted.entries) {
523-
var whyNotPromotedVisitor = _WhyNotPromotedVisitor(
524-
source, receiver, flowAnalysis!.dataForTesting);
525-
if (typeSystem.isPotentiallyNullable(entry.key)) continue;
526-
var message = entry.value.accept(whyNotPromotedVisitor);
527-
if (message != null) {
528-
if (flowAnalysis!.dataForTesting != null) {
529-
var nonPromotionReasonText = entry.value.shortName;
530-
if (whyNotPromotedVisitor.propertyReference != null) {
531-
var id =
532-
computeMemberId(whyNotPromotedVisitor.propertyReference!);
533-
nonPromotionReasonText += '($id)';
534-
}
535-
flowAnalysis!.dataForTesting!.nonPromotionReasons[errorEntity] =
536-
nonPromotionReasonText;
519+
var whyNotPromoted = flowAnalysis?.flow?.whyNotPromoted(receiver);
520+
if (whyNotPromoted != null) {
521+
for (var entry in whyNotPromoted.entries) {
522+
var whyNotPromotedVisitor = _WhyNotPromotedVisitor(
523+
source, receiver, errorEntity, flowAnalysis!.dataForTesting);
524+
if (typeSystem.isPotentiallyNullable(entry.key)) continue;
525+
var message = entry.value.accept(whyNotPromotedVisitor);
526+
if (message != null) {
527+
if (flowAnalysis!.dataForTesting != null) {
528+
var nonPromotionReasonText = entry.value.shortName;
529+
if (whyNotPromotedVisitor.propertyReference != null) {
530+
var id =
531+
computeMemberId(whyNotPromotedVisitor.propertyReference!);
532+
nonPromotionReasonText += '($id)';
537533
}
538-
messages = [message];
534+
flowAnalysis!.dataForTesting!.nonPromotionReasons[errorEntity] =
535+
nonPromotionReasonText;
539536
}
540-
break;
537+
messages = [message];
541538
}
539+
break;
542540
}
543541
}
544542
return messages;
@@ -3343,13 +3341,16 @@ class _WhyNotPromotedVisitor
33433341
PromotableElement> {
33443342
final Source source;
33453343

3346-
final Expression _receiver;
3344+
final Expression? _receiver;
3345+
3346+
final SyntacticEntity _errorEntity;
33473347

33483348
final FlowAnalysisDataForTesting? _dataForTesting;
33493349

33503350
PropertyAccessorElement? propertyReference;
33513351

3352-
_WhyNotPromotedVisitor(this.source, this._receiver, this._dataForTesting);
3352+
_WhyNotPromotedVisitor(
3353+
this.source, this._receiver, this._errorEntity, this._dataForTesting);
33533354

33543355
@override
33553356
DiagnosticMessage? visitDemoteViaExplicitWrite(
@@ -3420,8 +3421,8 @@ class _WhyNotPromotedVisitor
34203421
return DiagnosticMessageImpl(
34213422
filePath: source.fullName,
34223423
message: "'this' can't be promoted.",
3423-
offset: _receiver.offset,
3424-
length: _receiver.length);
3424+
offset: _errorEntity.offset,
3425+
length: _errorEntity.length);
34253426
}
34263427

34273428
DiagnosticMessageImpl _contextMessageForProperty(

0 commit comments

Comments
 (0)