Skip to content

Commit 4b4cc13

Browse files
stereotype441commit-bot@chromium.org
authored andcommitted
Flow analysis: clarifying comments for "why not promoted" logic.
These comments address code review comments from https://dart-review.googlesource.com/c/sdk/+/181741. Bug: #44898 Change-Id: I93d35600048318c20e6c41b290b6736d0086a574 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/184980 Reviewed-by: Johnni Winther <[email protected]> Commit-Queue: Paul Berry <[email protected]>
1 parent 02f0f53 commit 4b4cc13

File tree

3 files changed

+37
-0
lines changed

3 files changed

+37
-0
lines changed

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4989,6 +4989,31 @@ class _PropertyGetReference<Variable extends Object, Type extends Object>
49894989

49904990
@override
49914991
Type getDeclaredType(TypeOperations<Variable, Type> typeOperations) {
4992+
// Since we don't actually support property promotion, we just compute what
4993+
// promotions would have occurred if we *did* support it (for the purpose of
4994+
// issuing more useful error messages), we have some leeway in how we define
4995+
// the "declared type" of a property. It's tempting to define it as the
4996+
// return type of the referenced getter, but this is problematic for two
4997+
// reasons: (1) we don't have the necessary hooks to ask the client what
4998+
// this type is, and (2) the referenced getter can become more specific due
4999+
// to the presence of other promotions, breaking the invariant that promoted
5000+
// types are expected to be subtypes of the declared type, e.g.:
5001+
//
5002+
// abstract class C { num? get x; }
5003+
// abstract class D extends C { int? get x; }
5004+
// f(C c) {
5005+
// if (c.x != null) { // "promotes" c.x to `num`
5006+
// if (c is D) { // promotes c to D, so c.x now refers to an `int?`
5007+
// // Invariant broken: `num` is not a subtype of `int?`
5008+
// }
5009+
// }
5010+
// }
5011+
//
5012+
// Rather than break the invariant (which could lead to unexpected behaviors
5013+
// of flow analysis, including possibly crashes), it seems wiser to simply
5014+
// define the "declared type" for properties to be the top type; in practice
5015+
// this still produces useful results, and it doesn't violate soundness
5016+
// because we don't actually promote property references.
49925017
return typeOperations.topType;
49935018
}
49945019

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,6 +328,13 @@ class TypeInferrerImpl implements TypeInferrer {
328328
dataForTesting.flowAnalysisResult.nonPromotionReasons[expression] =
329329
nonPromotionReasonText;
330330
}
331+
// Note: this will always pick the first viable reason (only). I
332+
// (paulberry) believe this is the one that will be the most relevant,
333+
// but I need to do more testing to validate that. I can't do that
334+
// additional testing yet because at the moment we only handle failed
335+
// promotions to non-nullable.
336+
// TODO(paulberry): do more testing and then expand on the comment
337+
// above.
331338
context = [message];
332339
break;
333340
}

pkg/front_end/test/spell_checking_list_code.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ bb
101101
became
102102
beforehand
103103
behave
104+
believe
104105
bellow
105106
belonging
106107
beloning
@@ -631,6 +632,7 @@ layout
631632
lc
632633
ld
633634
leafp
635+
leeway
634636
len
635637
lets
636638
letting
@@ -1175,6 +1177,7 @@ tells
11751177
temp
11761178
temporaries
11771179
temps
1180+
tempting
11781181
term
11791182
termcap
11801183
terminator
@@ -1323,6 +1326,7 @@ vector
13231326
vegorov
13241327
verbosity
13251328
versa
1329+
viable
13261330
vice
13271331
video
13281332
violated
@@ -1355,6 +1359,7 @@ widgets
13551359
wiki
13561360
wikipedia
13571361
wind
1362+
wiser
13581363
with1
13591364
wn
13601365
worthwhile

0 commit comments

Comments
 (0)