Optimize DDC private library files.

pkg/dev_compiler/lib/src/compiler/nullable_type_inference.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:collection';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/ast/standard_resolution_map.dart';
 import 'package:analyzer/dart/ast/token.dart' show TokenType;
 import 'package:analyzer/dart/ast/visitor.dart' show RecursiveAstVisitor;
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
-import 'element_helpers.dart' show getStaticType, isInlineJS;
+import 'element_helpers.dart' show getStaticType, isInlineJS, findAnnotation;
+import 'js_interop.dart' show isNotNullAnnotation, isNullCheckAnnotation;
 import 'property_model.dart';
 
 /// An inference engine for nullable types.
 ///
 /// This can answer questions about whether expressions are nullable
 /// (see [isNullable]). Given a set of compilation units in a library, it will
 /// determine if locals can be null using flow-insensitive analysis.
 ///
 /// The analysis for null expressions is conservative and incomplete, but it can
 /// optimize some patterns.
 // TODO(vsm): Revisit whether we really need this when we get
 // better non-nullability in the type system.
 abstract class NullableTypeInference {
   LibraryElement get dartCoreLibrary;
   VirtualFieldModel get virtualFields;
 
+  InterfaceType getImplementationType(DartType type);
   bool isPrimitiveType(DartType type);
   bool isObjectMember(String name);
 
   /// Known non-null local variables.
   HashSet<LocalVariableElement> _notNullLocals;
 
   void inferNullableTypes(AstNode node) {
     var visitor = new _NullableLocalInference(this);
     node.accept(visitor);
     _notNullLocals = visitor.computeNotNullLocals();
   }
 
   /// Adds a new variable, typically a compiler generated temporary, and record
   /// whether its type is nullable.
   void addTemporaryVariable(LocalVariableElement local, {bool nullable: true}) {
     if (!nullable) _notNullLocals.add(local);
   }
 
   /// Returns true if [expr] can be null.
   bool isNullable(Expression expr) => _isNullable(expr);
 
+  bool _isNonNullMethodInvocation(MethodInvocation expr) {
+    // TODO(vsm): This logic overlaps with the resolver.
+    // Where is the best place to put this?
+    var e = resolutionMap.staticElementForIdentifier(expr.methodName);
+    if (e == null) return false;
+    if (isInlineJS(e)) {
+      // Fix types for JS builtin calls.
+      //
+      // This code was taken from analyzer. It's not super sophisticated:
+      // only looks for the type name in dart:core, so we just copy it here.
+      //
+      // TODO(jmesserly): we'll likely need something that can handle a wider
+      // variety of types, especially when we get to JS interop.
+      var args = expr.argumentList.arguments;
+      var first = args.isNotEmpty ? args.first : null;
+      if (first is SimpleStringLiteral) {
+        var types = first.stringValue;
+        if (types != '' &&
+            types != 'var' &&
+            !types.split('|').contains('Null')) {
+          return true;
+        }
+      }
+    }
+
+    if (e.name == 'identical' && identical(e.library, dartCoreLibrary)) {
+      return true;
+    }
+    // If this is a method call, check to see whether it is to a final
+    // type for which we have a known implementation type (i.e. int, bool,
+    // double, and String), and if so use the element for the implementation
+    // type instead.
+    if (e is MethodElement) {

[Jennifer Messerly, 2017/08/23 19:49:37]

conceptually, are we asking if the method is final/sealed? It would be pretty
easy to extend the analysis to know about even more things. For example we often
know when private fields are sealed (similar with private classes, or "public"
classes in the SDK that are in private libraries). Knowing about private methods
would help us eliminate some covariance checks, so it would be useful besides
null.

Maybe this could be factored into an "isSealed" function that we extend over
time?

+      Element container = e.enclosingElement;
+      if (container is ClassElement) {
+        DartType targetType = container.type;
+        InterfaceType implType = getImplementationType(targetType);
+        if (implType != null) {
+          MethodElement method = implType.lookUpMethod(e.name, dartCoreLibrary);
+          if (method != null) e = method;
+        }
+      }
+    }
+    // If the method or function is annotated as returning a non-null value
+    // then the result of the call is non-null.
+    return (e is MethodElement || e is FunctionElement) && _assertedNotNull(e);

[Jennifer Messerly, 2017/08/23 19:49:37]

this seems potentially unsafe -- does it work if I override a member from the
core libraries? Or are we being careful to not annotate return types on public
interfaces from them?

maybe worth a comment

+  }
+
+  bool _isNonNullProperty(Element element, String name) {
+    if (element is! PropertyInducingElement &&
+        element is! PropertyAccessorElement) {
+      return false;
+    }
+    // If this is a reference to an element of a type for which
+    // we have a known implementation type (i.e. int, double,
+    // bool, String), then use the element for the implementation
+    // type.
+    Element container = element.enclosingElement;
+    if (container is ClassElement) {
+      DartType targetType = container.type;
+      InterfaceType implType = getImplementationType(targetType);

[Jennifer Messerly, 2017/08/23 19:49:38]

var?

+      if (implType != null) {
+        PropertyAccessorElement getter =

[Jennifer Messerly, 2017/08/23 19:49:37]

var?

+            implType.lookUpGetter(name, dartCoreLibrary);
+        if (getter != null) element = getter;
+      }
+    }
+    // If the getter is a synthetic element, then any annotations will
+    // be on the variable, so use those instead.
+    if (element is PropertyAccessorElement &&
+        element.isSynthetic &&
+        element.variable != null) {

[Jennifer Messerly, 2017/08/23 19:49:37]

I'm pretty sure this is guaranteed to be non-null, once the other conditions are
satisfied

+      element = (element as PropertyAccessorElement).variable;

[Jennifer Messerly, 2017/08/23 19:49:37]

this could just be `return _assertedNotNull(element.variable);` ... that way you
don't reassign so you don't lose the type promotion

+    }
+    // Return true if the element is annotated as returning a non-null value.
+    return _assertedNotNull(element);

[Jennifer Messerly, 2017/08/23 19:51:11]

this one also seems potentially unsafe -- I'm guessing we only put these
annotations on non-extensible members, and never on members of interface types?

+  }
+
   /// Returns true if [expr] can be null, optionally using [localIsNullable]
   /// for locals.
   ///
   /// If [localIsNullable] is not supplied, this will use the known list of
   /// [_notNullLocals].
   bool _isNullable(Expression expr,
       [bool localIsNullable(LocalVariableElement e)]) {
     // TODO(jmesserly): we do recursive calls in a few places. This could
     // leads to O(depth) cost for calling this function. We could store the
     // resulting value if that becomes an issue, so we maintain the invariant
     // that each node is visited once.
     Element element = null;
+    String name = null;
     if (expr is PropertyAccess &&
         expr.operator?.type != TokenType.QUESTION_PERIOD) {
       element = expr.propertyName.staticElement;
+      name = expr.propertyName.name;
+    } else if (expr is PrefixedIdentifier) {
+      element = expr.staticElement;
+      name = expr.identifier.name;
     } else if (expr is Identifier) {
       element = expr.staticElement;
+      name = expr.name;
     }
     if (element != null) {
+      if (_isNonNullProperty(element, name)) return false;
+
       // Type literals are not null.
       if (element is ClassElement || element is FunctionTypeAliasElement) {
         return false;
       }
 
       if (element is LocalVariableElement) {
         if (localIsNullable != null) {
           return localIsNullable(element);
         }
         return !_notNullLocals.contains(element);
       }
 
+      if (element is ParameterElement && _assertedNotNull(element)) {
+        return false;
+      }
+
       if (element is FunctionElement || element is MethodElement) {
         // A function or method. This can't be null.
         return false;
       }
 
       if (element is PropertyAccessorElement && element.isGetter) {
         PropertyInducingElement variable = element.variable;
         var isVirtual =
             variable is FieldElement && virtualFields.isVirtual(variable);
         return isVirtual || (variable.computeConstantValue()?.isNull ?? true);
@@ skipping 71 matching lines @@
       type = getStaticType(expr.leftOperand);
     } else if (expr is PrefixExpression) {
       if (expr.operator.type == TokenType.BANG) return false;
       type = getStaticType(expr.operand);
     } else if (expr is PostfixExpression) {
       type = getStaticType(expr.operand);
     }
     if (type != null && isPrimitiveType(type)) {
       return false;
     }
-    if (expr is MethodInvocation) {
-      // TODO(vsm): This logic overlaps with the resolver.
-      // Where is the best place to put this?
-      var e = resolutionMap.staticElementForIdentifier(expr.methodName);
-      if (isInlineJS(e)) {
-        // Fix types for JS builtin calls.
-        //
-        // This code was taken from analyzer. It's not super sophisticated:
-        // only looks for the type name in dart:core, so we just copy it here.
-        //
-        // TODO(jmesserly): we'll likely need something that can handle a wider
-        // variety of types, especially when we get to JS interop.
-        var args = expr.argumentList.arguments;
-        var first = args.isNotEmpty ? args.first : null;
-        if (first is SimpleStringLiteral) {
-          var types = first.stringValue;
-          if (types != '' &&
-              types != 'var' &&
-              !types.split('|').contains('Null')) {
-            return false;
-          }
-        }
-      }
-
-      if (e?.name == 'identical' && identical(e.library, dartCoreLibrary)) {
-        return false;
-      }
+    if (expr is MethodInvocation && _isNonNullMethodInvocation(expr)) {
+      return false;
     }
 
     // TODO(ochafik,jmesserly): handle other cases such as: refs to top-level
     // finals that have been assigned non-nullable values.
 
     // Failed to recognize a non-nullable case: assume it can be null.
     return true;
   }
 }
 
@@ skipping 43 matching lines @@
   }
 
   @override
   visitVariableDeclaration(VariableDeclaration node) {
     var element = node.element;
     var initializer = node.initializer;
     if (element is LocalVariableElement) {
       _locals.add(element);
       if (initializer != null) {
         _visitAssignment(node.name, initializer);
-      } else {
+      } else if (!_assertedNotNull(element)) {
         _nullableLocals.add(element);
       }
     }
     super.visitVariableDeclaration(node);
   }
 
   @override
+  visitForEachStatement(ForEachStatement node) {
+    if (node.identifier == null) {
+      var declaration = node.loopVariable;
+      var element = declaration.element;
+      _locals.add(element);
+      if (!_assertedNotNull(element)) {
+        _nullableLocals.add(element);
+      }
+    } else {
+      var element = node.identifier.staticElement;
+      if (element is LocalVariableElement && !_assertedNotNull(element)) {
+        _nullableLocals.add(element);
+      }
+    }
+    super.visitForEachStatement(node);
+  }
+
+  @override
   visitCatchClause(CatchClause node) {
     var e = node.exceptionParameter?.staticElement;
     if (e != null) {
       _locals.add(e);
       // TODO(jmesserly): we allow throwing of `null`, for better or worse.
       _nullableLocals.add(e);
     }
 
     e = node.stackTraceParameter?.staticElement;
     if (e != null) _locals.add(e);
@@ skipping 30 matching lines @@
     var op = node.operator.type;
     if (op.isIncrementOperator) {
       _visitAssignment(node.operand, node);
     }
     super.visitPrefixExpression(node);
   }
 
   void _visitAssignment(Expression left, Expression right) {
     if (left is SimpleIdentifier) {
       var element = left.staticElement;
-      if (element is LocalVariableElement) {
+      if (element is LocalVariableElement && !_assertedNotNull(element)) {
         bool visitLocal(LocalVariableElement otherLocal) {
           // Record the assignment.
           _assignments
               .putIfAbsent(otherLocal, () => new HashSet.identity())
               .add(element);
           // Optimistically assume this local is not null.
           // We will validate this assumption later.
           return false;
         }
 
         if (_nullInference._isNullable(right, visitLocal)) {
           _nullableLocals.add(element);
         }
       }
     }
   }
 }
+
+bool _assertedNotNull(Element e) =>
+    findAnnotation(e, isNotNullAnnotation) != null ||
+    findAnnotation(e, isNullCheckAnnotation) != null;