Version 3.7.0-55.0.dev

Merge 9a67968 into dev

CHANGELOG.md

@@ -31,6 +31,16 @@ main() {
 }
 ```
 
+#### Other Language Changes
+
+- **Breaking Change** [#56893][]: If a field is promoted to the type `Null`
+  using `is` or `as`, this type promotion is now properly accounted for in
+  reachability analysis. This makes the type system more self-consistent,
+  because it mirrors the behavior of promoted local variables. This change is
+  not expected to make any difference in practice.
+
+[#56893]: https://github.com/dart-lang/sdk/issues/56893
+
 #### Dart to Javascript Compiler (dart2js)
 
 - The dart2js compiler which is invoked when the command

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

@@ -2313,6 +2313,11 @@ class FlowModel<Type extends Object> {
       PromotionModel<Type>? info =
           result.promotionInfo?.get(helper, variableKey);
       if (info == null) continue;
+
+      // We don't need to discard promotions for final variables. They are
+      // guaranteed to be already assigned and won't be assigned again.
+      if (helper.isFinal(variableKey)) continue;
+
       PromotionModel<Type> newInfo =
           info.discardPromotionsAndMarkNotUnassigned();
       if (!identical(info, newInfo)) {
@@ -2823,6 +2828,10 @@ mixin FlowModelHelper<Type extends Object> {
   /// subtyping.
   @visibleForTesting
   FlowAnalysisTypeOperations<Type> get typeOperations;
+
+  /// Whether the variable of [variableKey] was declared with the `final`
+  /// modifier and the `inference-update-4` feature flag is enabled.
+  bool isFinal(int variableKey);
 }
 
 /// Documentation links that might be presented to the user to accompany a "why
@@ -4993,6 +5002,14 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
     }
   }
 
+  @override
+  bool isFinal(int variableKey) {
+    if (!inferenceUpdate4Enabled) return false;
+    Variable? variable = promotionKeyStore.variableForKey(variableKey);
+    if (variable != null && operations.isFinal(variable)) return true;
+    return false;
+  }
+
   @override
   bool isUnassigned(Variable variable) {
     return _current.promotionInfo
@@ -5379,7 +5396,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
         propertyMember: propertyMember,
         promotionKey: propertySsaNode.promotionKey,
         model: _current,
-        type: unpromotedType,
+        type: promotedType ?? unpromotedType,
         ssaNode: propertySsaNode);
     if (wholeExpression != null) {
       _storeExpressionInfo(wholeExpression, propertyReference);
@@ -5401,7 +5418,7 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
         propertyMember: propertyMember,
         promotionKey: propertySsaNode.promotionKey,
         model: _current,
-        type: unpromotedType,
+        type: promotedType ?? unpromotedType,
         ssaNode: propertySsaNode);
     _stack.add(new _PropertyPatternContext<Type>(
         _makeTemporaryReference(

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

@@ -10,6 +10,9 @@ library;
 /// representation of variables and types.
 abstract interface class FlowAnalysisOperations<Variable extends Object,
     Type extends Object> implements FlowAnalysisTypeOperations<Type> {
+  /// Whether the given [variable] was declared with the `final` modifier.
+  bool isFinal(Variable variable);
+
   /// Determines whether the given property can be promoted.
   ///
   /// [property] will correspond to a `propertyMember` value passed to

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_mini_ast.dart

@@ -39,6 +39,13 @@ class FlowAnalysisTestHarness extends Harness
   @override
   FlowAnalysisOperations<Var, SharedTypeView<Type>> get typeOperations =>
       typeAnalyzer.operations;
+
+  @override
+  bool isFinal(int variableKey) {
+    Var? variable = promotionKeyStore.variableForKey(variableKey);
+    if (variable != null && operations.isFinal(variable)) return true;
+    return false;
+  }
 }
 
 /// Helper class allowing tests to examine the values of variables' SSA nodes.

pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart

@@ -1018,6 +1018,99 @@ main() {
       ]);
     });
 
+    test(
+        'functionExpression_begin() cancels promotions of final vars'
+        ' with inference-update-4 disabled', () {
+      // See test for "functionExpression_begin() preserves promotions of final
+      // variables" for enabled behavior.
+      var x = Var('x', isFinal: true);
+      h.disableInferenceUpdate4();
+      h.run([
+        declare(x, type: 'num'),
+        if_(expr('bool'), [
+          x.write(expr('int')),
+        ], [
+          x.write(expr('double')),
+        ]),
+        if_(x.is_('int'), [
+          localFunction([
+            checkNotPromoted(x),
+          ]),
+        ], [
+          localFunction([
+            checkNotPromoted(x),
+          ]),
+        ]),
+      ]);
+    });
+
+    test('functionExpression_begin() cancels promotions of non-final vars', () {
+      // num x;
+      // if (<bool>) {
+      //   x = <int>;
+      // } else {
+      //   x = <double>;
+      // }
+      // if (x is int) {
+      //   () => x is not promoted
+      // } else {
+      //   () => x is not promoted
+      // }
+
+      var x = Var('x');
+      h.run([
+        declare(x, type: 'num'),
+        if_(expr('bool'), [
+          x.write(expr('int')),
+        ], [
+          x.write(expr('double')),
+        ]),
+        if_(x.is_('int'), [
+          localFunction([
+            checkNotPromoted(x),
+          ]),
+        ], [
+          localFunction([
+            checkNotPromoted(x),
+          ]),
+        ]),
+      ]);
+    });
+
+    test('functionExpression_begin() preserves promotions of final variables',
+        () {
+      // final num x;
+      // if (<bool>) {
+      //   x = <int>;
+      // } else {
+      //   x = <double>;
+      // }
+      // if (x is int) {
+      //   () => x is promoted to int
+      // } else {
+      //   () => x is not promoted
+      // }
+
+      var x = Var('x', isFinal: true);
+      h.run([
+        declare(x, type: 'num'),
+        if_(expr('bool'), [
+          x.write(expr('int')),
+        ], [
+          x.write(expr('double')),
+        ]),
+        if_(x.is_('int'), [
+          localFunction([
+            checkPromoted(x, 'int'),
+          ]),
+        ], [
+          localFunction([
+            checkNotPromoted(x),
+          ]),
+        ]),
+      ]);
+    });
+
     test('functionExpression_begin() preserves promotions of initialized vars',
         () {
       var x = Var('x');
@@ -7315,16 +7408,10 @@ main() {
     });
 
     group('and equality:', () {
-      test('promoted type ignored on LHS', () {
-        // Normally flow analysis understands when an `if` test is guaranteed to
-        // succeed (or fail) based on the static types of the LHS and RHS. But
-        // due to https://github.com/dart-lang/language/issues/4127, this
-        // doesn't fully work when the LHS or RHS is a property reference; in
-        // that case, the unpromoted type is used.
-
-        // This test is here to make sure we don't change the existing behavior
-        // by accident; if/when we fix #4127, this test should be changed
-        // accordingly.
+      test('promoted type accounted for on LHS', () {
+        // Flow analysis understands when an `if` test is guaranteed to succeed
+        // (or fail) based on the static types of the LHS and RHS. Make sure
+        // this works when the LHS or RHS is a property reference.
         h.addMember('C', 'f', 'Object?', promotable: true);
         h.thisType = 'C';
         h.run([
@@ -7333,21 +7420,15 @@ main() {
           if_(thisProperty('f').eq(nullLiteral), [
             checkReachable(true),
           ], [
-            checkReachable(true),
+            checkReachable(false),
           ]),
         ]);
       });
 
-      test('promoted type ignored on RHS', () {
-        // Normally flow analysis understands when an `if` test is guaranteed to
-        // succeed (or fail) based on the static types of the LHS and RHS. But
-        // due to https://github.com/dart-lang/language/issues/4127, this
-        // doesn't fully work when the LHS or RHS is a property reference; in
-        // that case, the unpromoted type is used.
-
-        // This test is here to make sure we don't change the existing behavior
-        // by accident; if/when we fix #4127, this test should be changed
-        // accordingly.
+      test('promoted type accounted for on RHS', () {
+        // Flow analysis understands when an `if` test is guaranteed to succeed
+        // (or fail) based on the static types of the LHS and RHS. Make sure
+        // this works when the LHS or RHS is a property reference.
         h.addMember('C', 'f', 'Object?', promotable: true);
         h.thisType = 'C';
         h.run([
@@ -7356,7 +7437,7 @@ main() {
           if_(nullLiteral.eq(thisProperty('f')), [
             checkReachable(true),
           ], [
-            checkReachable(true),
+            checkReachable(false),
           ]),
         ]);
       });

pkg/_fe_analyzer_shared/test/mini_ast.dart

@@ -2963,6 +2963,11 @@ class MiniAstOperations
     return false;
   }
 
+  @override
+  bool isFinal(Var variable) {
+    return variable.isFinal;
+  }
+
   @override
   bool isInterfaceType(SharedTypeView<Type> type) {
     Type unwrappedType = type.unwrapTypeView();

pkg/analyzer/lib/src/dart/element/element.dart

@@ -10710,29 +10710,32 @@ class _Sentinel {
 }
 
 extension on Fragment {
+  /// The content of the documentation comment (including delimiters) for this
+  /// element or fragment.
+  ///
+  /// If the receiver is an element that has fragments, the comment will be a
+  /// concatenation of the comments from all of the fragments.
+  ///
+  /// Returns `null` if the receiver does not have or does not support
+  /// documentation.
   String? get documentationCommentOrNull {
-    // TODO(brianwilkerson): I think that all fragments are annotatable. If
-    //  that's true then this getter isn't necessary and should be removed.
     return switch (this) {
-      LibraryFragment(:var documentationComment) => documentationComment,
-      TypeDefiningFragment(:var documentationComment) => documentationComment,
-      TypeParameterizedFragment(:var documentationComment) =>
-        documentationComment,
-      FormalParameterFragment(:var documentationComment) =>
-        documentationComment,
+      Annotatable(:var documentationComment) => documentationComment,
       _ => null,
     };
   }
 
+  /// The metadata associated with the element or fragment.
+  ///
+  /// If the receiver is an element that has fragments, the list will include
+  /// all of the metadata from all of the fragments.
+  ///
+  /// The list will be empty if the receiver does not have any metadata, does
+  /// not support metadata, or if the library containing this element has not
+  /// yet been fully resolved.
   Metadata get metadataOrEmpty {
-    // TODO(brianwilkerson): I think that all fragments are annotatable. If
-    //  that's true then this getter isn't necessary and should be removed.
     return switch (this) {
-      LibraryFragment(:var metadata2) => metadata2,
-      PropertyInducingFragment(:var metadata2) => metadata2,
-      TypeDefiningFragment(:var metadata2) => metadata2,
-      TypeParameterizedFragment(:var metadata2) => metadata2,
-      FormalParameterFragment(:var metadata2) => metadata2,
+      Annotatable(:var metadata2) => metadata2,
       _ => MetadataImpl(-1, const [], () => null),
     };
   }

pkg/analyzer/lib/src/dart/element/inheritance_manager3.dart

@@ -1314,7 +1314,11 @@ class Name {
   }
 
   Name get forSetter {
-    return Name(libraryUri, '$name=');
+    if (name.endsWith('=')) {
+      return this;
+    } else {
+      return Name(libraryUri, '$name=');
+    }
   }
 
   @override

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

@@ -543,6 +543,11 @@ class TypeSystemOperations
         unwrappedType.element is ExtensionTypeElement;
   }
 
+  @override
+  bool isFinal(PromotableElement variable) {
+    return variable.isFinal;
+  }
+
   @override
   bool isInterfaceType(SharedTypeView<DartType> type) {
     DartType unwrappedType = type.unwrapTypeView();

pkg/analyzer/test/src/dart/element/inheritance_manager3_test.dart

@@ -25,6 +25,7 @@ main() {
     defineReflectiveTests(InheritanceManager3Test);
     defineReflectiveTests(InheritanceManager3Test_elements);
     defineReflectiveTests(InheritanceManager3Test_ExtensionType);
+    defineReflectiveTests(InheritanceManager3NameTest);
     defineReflectiveTests(UpdateNodeTextExpectations);
   });
 }
@@ -1413,6 +1414,47 @@ class B extends A {
   }
 }
 
+@reflectiveTest
+class InheritanceManager3NameTest {
+  test_equals() {
+    expect(Name(null, 'foo'), Name(null, 'foo'));
+    expect(Name(null, 'foo'), Name(null, 'foo=').forGetter);
+    expect(Name(null, 'foo='), Name(null, 'foo='));
+    expect(Name(null, 'foo='), Name(null, 'foo').forSetter);
+    expect(Name(null, 'foo='), Name(null, 'foo').forSetter.forSetter.forSetter);
+  }
+
+  test_forGetter() {
+    var name = Name(null, 'foo');
+    expect(name.forGetter.name, 'foo');
+    expect(name, name.forGetter);
+  }
+
+  test_forGetter_fromSetter() {
+    var name = Name(null, 'foo=');
+    expect(name.forGetter.name, 'foo');
+  }
+
+  test_forSetter() {
+    var name = Name(null, 'foo=');
+    expect(name.forSetter.name, 'foo=');
+    expect(name, name.forSetter);
+  }
+
+  test_forSetter_fromGetter() {
+    var name = Name(null, 'foo');
+    expect(name.forSetter.name, 'foo=');
+  }
+
+  test_name_getter() {
+    expect(Name(null, 'foo').name, 'foo');
+  }
+
+  test_name_setter() {
+    expect(Name(null, 'foo=').name, 'foo=');
+  }
+}
+
 @reflectiveTest
 class InheritanceManager3Test extends _InheritanceManager3Base {
   test_getInherited_closestSuper() async {