Remove const instance creation expressions from the dependency graph.

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

 // Copyright (c) 2014, 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.
 
 // This code was auto-generated, is not intended to be edited, and is subject to
 // significant change. Please see the README file for more information.
 
 library engine.constant;
 
 import 'dart:collection';
@@ skipping 137 matching lines @@
  * constants to be evaluated.
  */
 class ConstantAstCloner extends AstCloner {
   ConstantAstCloner() : super(true);
 
   @override
   InstanceCreationExpression visitInstanceCreationExpression(
       InstanceCreationExpression node) {
     InstanceCreationExpression expression =
         super.visitInstanceCreationExpression(node);
-    expression.constantHandle = node.constantHandle;
+    expression.staticElement = node.staticElement;
     return expression;
   }
 
   @override
   RedirectingConstructorInvocation visitRedirectingConstructorInvocation(
       RedirectingConstructorInvocation node) {
     RedirectingConstructorInvocation invocation =
         super.visitRedirectingConstructorInvocation(node);
     invocation.staticElement = node.staticElement;
     return invocation;
@@ skipping 502 matching lines @@
   bool runtimeTypeMatch(DartObjectImpl obj, DartType type) {
     if (obj.isNull) {
       return true;
     }
     if (type.isUndefined) {
       return false;
     }
     return obj.type.isSubtypeOf(type);
   }
 
-  ConstructorElementImpl _getConstructorBase(ConstructorElement constructor) {
-    while (constructor is ConstructorMember) {
-      constructor = (constructor as ConstructorMember).baseElement;
-    }
-    return constructor;
-  }
-
   /**
    * Determine whether the given string is a valid name for a public symbol
    * (i.e. whether it is allowed for a call to the Symbol constructor).
    */
   static bool isValidPublicSymbol(String name) => name.isEmpty ||
       name == "void" ||
       new JavaPatternMatcher(_PUBLIC_SYMBOL_PATTERN, name).matches();
+
+  static ConstructorElementImpl _getConstructorBase(
+      ConstructorElement constructor) {
+    while (constructor is ConstructorMember) {
+      constructor = (constructor as ConstructorMember).baseElement;
+    }
+    return constructor;
+  }
 }
 
 /**
  * Interface used by unit tests to verify correct dependency analysis during
  * constant evaluation.
  */
 abstract class ConstantEvaluationValidator {
   /**
    * This method is called just before computing the constant value associated
    * with [constNode]. Unit tests will override this method to introduce
@@ skipping 153 matching lines @@
       new HashMap<PotentiallyConstVariableElement, VariableDeclaration>();
 
   /**
    * A table mapping constant constructors to the declarations of those
    * constructors.
    */
   final HashMap<ConstructorElement, ConstructorDeclaration> constructorMap =
       new HashMap<ConstructorElement, ConstructorDeclaration>();
 
   /**
-   * A collection of constant constructor invocations.
-   */
-  final List<InstanceCreationExpression> constructorInvocations =
-      new List<InstanceCreationExpression>();
-
-  /**
    * A collection of annotations.
    */
   final List<Annotation> annotations = <Annotation>[];
 
   /**
    * True if instance variables marked as "final" should be treated as "const".
    */
   bool treatFinalInstanceVarAsConst = false;
 
   @override
@@ skipping 25 matching lines @@
     if (node.constKeyword != null) {
       ConstructorElement element = node.element;
       if (element != null) {
         constructorMap[element] = node;
       }
     }
     return null;
   }
 
   @override
-  Object visitInstanceCreationExpression(InstanceCreationExpression node) {
-    super.visitInstanceCreationExpression(node);
-    if (node.isConst) {
-      constructorInvocations.add(node);
-    }
-    return null;
-  }
-
-  @override
   Object visitVariableDeclaration(VariableDeclaration node) {
     super.visitVariableDeclaration(node);
     Expression initializer = node.initializer;
     VariableElement element = node.element;
     if (initializer != null &&
         (node.isConst ||
             treatFinalInstanceVarAsConst &&
                 element is FieldElement &&
                 node.isFinal &&
                 !element.isStatic)) {
@@ skipping 45 matching lines @@
    */
   HashMap<PotentiallyConstVariableElement, VariableDeclaration> _variableDeclarationMap;
 
   /**
    * A table mapping constant constructors to the declarations of those
    * constructors.
    */
   HashMap<ConstructorElement, ConstructorDeclaration> constructorDeclarationMap;
 
   /**
-   * A collection of constant constructor invocations.
-   */
-  List<InstanceCreationExpression> _constructorInvocations;
-
-  /**
    * A collection of annotations.
    */
   List<Annotation> _annotations;
 
   /**
    * The evaluation engine that does the work of evaluating instance creation
    * expressions.
    */
   final ConstantEvaluationEngine evaluationEngine;
 
@@ skipping 16 matching lines @@
     unit.accept(_constantFinder);
   }
 
   /**
    * Compute values for all of the constants in the compilation units that were
    * added.
    */
   void computeValues() {
     _variableDeclarationMap = _constantFinder.variableMap;
     constructorDeclarationMap = _constantFinder.constructorMap;
-    _constructorInvocations = _constantFinder.constructorInvocations;
     _annotations = _constantFinder.annotations;
     _variableDeclarationMap.values.forEach((VariableDeclaration declaration) {
       ReferenceFinder referenceFinder = new ReferenceFinder(declaration,
           referenceGraph, _variableDeclarationMap, constructorDeclarationMap);
       referenceGraph.addNode(declaration);
       declaration.initializer.accept(referenceFinder);
     });
     constructorDeclarationMap.forEach((ConstructorElementImpl element,
         ConstructorDeclaration declaration) {
       element.isCycleFree = false;
       ConstructorElement redirectedConstructor =
           evaluationEngine.getConstRedirectedConstructor(element);
       if (redirectedConstructor != null) {
         ConstructorElement redirectedConstructorBase =
-            evaluationEngine._getConstructorBase(redirectedConstructor);
+            ConstantEvaluationEngine._getConstructorBase(redirectedConstructor);
         ConstructorDeclaration redirectedConstructorDeclaration =
             findConstructorDeclaration(redirectedConstructorBase);
         referenceGraph.addEdge(declaration, redirectedConstructorDeclaration);
         return;
       }
       ReferenceFinder referenceFinder = new ReferenceFinder(declaration,
           referenceGraph, _variableDeclarationMap, constructorDeclarationMap);
       referenceGraph.addNode(declaration);
       bool superInvocationFound = false;
       NodeList<ConstructorInitializer> initializers = declaration.initializers;
@@ skipping 35 matching lines @@
           Expression defaultValue = parameter.defaultValue;
           if (defaultValue != null) {
             ReferenceFinder parameterReferenceFinder = new ReferenceFinder(
                 parameter, referenceGraph, _variableDeclarationMap,
                 constructorDeclarationMap);
             defaultValue.accept(parameterReferenceFinder);
           }
         }
       }
     });
-    for (InstanceCreationExpression expression in _constructorInvocations) {
-      referenceGraph.addNode(expression);
-      ConstructorElement constructor = expression.staticElement;
-      if (constructor == null) {
-        continue;
-      }
-      ConstructorDeclaration declaration =
-          findConstructorDeclaration(constructor);
-      // An instance creation expression depends both on the constructor and
-      // the arguments passed to it.
-      ReferenceFinder referenceFinder = new ReferenceFinder(expression,
-          referenceGraph, _variableDeclarationMap, constructorDeclarationMap);
-      if (declaration != null) {
-        referenceGraph.addEdge(expression, declaration);
-      }
-      expression.argumentList.accept(referenceFinder);
-    }
     List<List<AstNode>> topologicalSort =
         referenceGraph.computeTopologicalSort();
     for (List<AstNode> constantsInCycle in topologicalSort) {
       if (constantsInCycle.length == 1) {
         _computeValueFor(constantsInCycle[0]);
       } else {
         for (AstNode constant in constantsInCycle) {
           _generateCycleError(constantsInCycle, constant);
         }
       }
     }
     // Since no constant can depend on an annotation, we don't waste time
     // including them in the topological sort.  We just process all the
     // annotations after all other constants are finished.
     for (Annotation annotation in _annotations) {
       _computeValueFor(annotation);
     }
   }
 
   ConstructorDeclaration findConstructorDeclaration(
           ConstructorElement constructor) => constructorDeclarationMap[
-      evaluationEngine._getConstructorBase(constructor)];
+      ConstantEvaluationEngine._getConstructorBase(constructor)];
 
   VariableDeclaration findVariableDeclaration(
           PotentiallyConstVariableElement variable) =>
       _variableDeclarationMap[variable];
 
   /**
    * Compute a value for the given [constNode].
    */
   void _computeValueFor(AstNode constNode) {
     evaluationEngine.validator.beforeComputeValue(constNode);
     if (constNode is VariableDeclaration) {
       VariableElement element = constNode.element;
       RecordingErrorListener errorListener = new RecordingErrorListener();
       ErrorReporter errorReporter =
           new ErrorReporter(errorListener, element.source);
       DartObjectImpl dartObject =
           (element as PotentiallyConstVariableElement).constantInitializer
               .accept(new ConstantVisitor(evaluationEngine, errorReporter));
-      if (dartObject != null) {
+      // Only check the type for truly const declarations (don't check final
+      // fields with initializers, since their types may be generic.  The type
+      // of the final field will be checked later, when the constructor is
+      // invoked).
+      if (dartObject != null && constNode.isConst) {
         if (!evaluationEngine.runtimeTypeMatch(dartObject, element.type)) {
           errorReporter.reportErrorForElement(
               CheckedModeCompileTimeErrorCode.VARIABLE_TYPE_MISMATCH, element, [
             dartObject.type,
             element.type
           ]);
         }
       }
       (element as VariableElementImpl).evaluationResult =
           new EvaluationResultImpl.con2(dartObject, errorListener.errors);
-    } else if (constNode is InstanceCreationExpression) {
-      InstanceCreationExpression expression = constNode;
-      ConstructorElement constructor = expression.staticElement;
-      if (constructor == null) {
-        // Couldn't resolve the constructor so we can't compute a value.
-        // No problem - the error has already been reported.
-        // But we still need to store an evaluation result.
-        expression.constantHandle.evaluationResult =
-            new EvaluationResultImpl.con1(null);
-        return;
-      }
-      RecordingErrorListener errorListener = new RecordingErrorListener();
-      CompilationUnit sourceCompilationUnit =
-          expression.getAncestor((node) => node is CompilationUnit);
-      ErrorReporter errorReporter = new ErrorReporter(
-          errorListener, sourceCompilationUnit.element.source);
-      ConstantVisitor constantVisitor =
-          new ConstantVisitor(evaluationEngine, errorReporter);
-      DartObjectImpl result = evaluationEngine.evaluateConstructorCall(
-          constNode, expression.argumentList.arguments, constructor,
-          constantVisitor, errorReporter);
-      expression.constantHandle.evaluationResult =
-          new EvaluationResultImpl.con2(result, errorListener.errors);
     } else if (constNode is ConstructorDeclaration) {
       // No evaluation needs to be done; constructor declarations are only in
       // the dependency graph to ensure that any constants referred to in
       // initializer lists and parameter defaults are evaluated before
       // invocations of the constructor.  However we do need to annotate the
       // element as being free of constant evaluation cycles so that later code
       // will know that it is safe to evaluate.
       ConstructorElementImpl constructor = constNode.element;
       constructor.isCycleFree = true;
     } else if (constNode is FormalParameter) {
@@ skipping 48 matching lines @@
       }
     } else {
       // Should not happen.
       AnalysisEngine.instance.logger.logError(
           "Constant value computer trying to compute the value of a node which is not a VariableDeclaration, InstanceCreationExpression, FormalParameter, or ConstructorDeclaration");
       return;
     }
   }
 
   /**
-   * Generate an error indicating that the given [constant] is not a valid
+   * Generate an error indicating that the given [constNode] is not a valid
    * compile-time constant because it references at least one of the constants
    * in the given [cycle], each of which directly or indirectly references the
    * constant.
    */
-  void _generateCycleError(List<AstNode> cycle, AstNode constant) {
-    // TODO(brianwilkerson) Implement this.
+  void _generateCycleError(List<AstNode> cycle, AstNode constNode) {
+    if (constNode is VariableDeclaration) {
+      VariableElement element = constNode.element;
+      RecordingErrorListener errorListener = new RecordingErrorListener();
+      ErrorReporter errorReporter =
+          new ErrorReporter(errorListener, element.source);
+      errorReporter.reportErrorForElement(
+          CompileTimeErrorCode.RECURSIVE_COMPILE_TIME_CONSTANT, element, []);

[Brian Wilkerson, 2015/05/05 21:25:24]

It would be really nice if we could extract enough information from the 'cycle'
argument to provide the user with a description of the cycle. Perhaps a TODO
comment for now so that we don't forget it.

[Paul Berry, 2015/05/05 21:46:31]

Done.

+      (element as VariableElementImpl).evaluationResult =
+          new EvaluationResultImpl.con2(null, errorListener.errors);
+    } else if (constNode is ConstructorDeclaration) {
+      // We don't report cycle errors on constructor declarations since there
+      // is nowhere to put the error information.
+    } else if (constNode is FormalParameter) {
+      // Formal parameter defaults should never appear as part of a cycle
+      // because they can't be referred to.
+      assert(false);
+    } else if (constNode is Annotation) {
+      // Annotations should never appear as part of a cycle because they can't
+      // be referred to.
+      assert(false);
+    } else {
+      // Should not happen.
+      AnalysisEngine.instance.logger.logError(
+          "Constant value computer trying to report a cycle error for an unexpected node");

[Brian Wilkerson, 2015/05/05 21:25:24]

It would be useful for the error message to include the type of the node,
something like:
  ... for a node of type ${constNode.runtimeType}");

[Paul Berry, 2015/05/05 21:46:31]

Done.

+      return;

[Brian Wilkerson, 2015/05/05 21:25:24]

Not sure why we have an explicit return here.

Not sure why we have "assert(false);" in the two previous 'else if' clauses but
not here.

Not sure why we don't log an error message in the previous two 'else if'
clauses.

Perhaps drop the other two clauses and move the 'assert' to the 'else'.

[Paul Berry, 2015/05/05 21:46:31]

Done.

+    }
   }
 }
 
 /**
  * A visitor used to evaluate constant expressions to produce their compile-time
  * value. According to the Dart Language Specification: <blockquote> A constant
  * expression is one of the following:
  *
  * * A literal number.
  * * A literal boolean.
@@ skipping 205 matching lines @@
       new DartObjectImpl(_typeProvider.doubleType, new DoubleState(node.value));
 
   @override
   DartObjectImpl visitInstanceCreationExpression(
       InstanceCreationExpression node) {
     if (!node.isConst) {
       // TODO(brianwilkerson) Figure out which error to report.
       _error(node, null);
       return null;
     }
-    evaluationEngine.validator.beforeGetEvaluationResult(node);
-    EvaluationResultImpl result = node.evaluationResult;
-    if (result != null) {
-      return result.value;
+    ConstructorElement constructor = node.staticElement;
+    if (constructor == null) {
+      // Couldn't resolve the constructor so we can't compute a value.  No
+      // problem - the error has already been reported.
+      return null;
     }
-    // TODO(brianwilkerson) Figure out which error to report.
-    _error(node, null);
-    return null;
+    return evaluationEngine.evaluateConstructorCall(
+        node, node.argumentList.arguments, constructor, this, _errorReporter);
   }
 
   @override
   DartObjectImpl visitIntegerLiteral(IntegerLiteral node) =>
       new DartObjectImpl(_typeProvider.intType, new IntState(node.value));
 
   @override
   DartObjectImpl visitInterpolationExpression(InterpolationExpression node) {
     DartObjectImpl result = node.expression.accept(this);
     if (result != null && !result.isBoolNumStringOrNull) {
@@ skipping 3424 matching lines @@
    * The [source] is the element representing the variable whose initializer
    * will be visited. The [referenceGraph] is a graph recording which variables
    * (heads) reference which other variables (tails) in their initializers. The
    * [variableDeclarationMap] is a table mapping constant variables to the
    * declarations of those variables. The [constructorDeclarationMap] is a table
    * mapping constant constructors to the declarations of those constructors.
    */
   ReferenceFinder(this._source, this._referenceGraph,
       this._variableDeclarationMap, this._constructorDeclarationMap);
 
+  ConstructorDeclaration findConstructorDeclaration(
+          ConstructorElement constructor) => _constructorDeclarationMap[
+      ConstantEvaluationEngine._getConstructorBase(constructor)];
+
   @override
   Object visitInstanceCreationExpression(InstanceCreationExpression node) {
     if (node.isConst) {
-      _referenceGraph.addEdge(_source, node);
+      ConstructorElement constructor = node.staticElement;
+      if (constructor != null) {
+        ConstructorDeclaration declaration =
+            findConstructorDeclaration(constructor);
+        _referenceGraph.addEdge(_source, declaration);
+      }
     }
-    return null;
+    return super.visitInstanceCreationExpression(node);
   }
 
   @override
   Object visitRedirectingConstructorInvocation(
       RedirectingConstructorInvocation node) {
     super.visitRedirectingConstructorInvocation(node);
     ConstructorElement target = node.staticElement;
     if (target != null && target.isConst) {
       ConstructorDeclaration targetDeclaration =
           _constructorDeclarationMap[target];
@@ skipping 256 matching lines @@
       return BoolState.from(_element == rightElement);
     } else if (rightOperand is DynamicState) {
       return BoolState.UNKNOWN_VALUE;
     }
     return BoolState.FALSE_STATE;
   }
 
   @override
   String toString() => _element == null ? "-unknown-" : _element.name;
 }