Fix memory leak in incremental resolver

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

Index: pkg/analyzer/lib/src/dart/element/element.dart
diff --git a/pkg/analyzer/lib/src/dart/element/element.dart b/pkg/analyzer/lib/src/dart/element/element.dart
index 1ae2e9d9a18c6a469d42a3f0e508a28bb0c55fb0..4139cd188265456c289ba8676c338e6d8d7634f7 100644
--- a/pkg/analyzer/lib/src/dart/element/element.dart
+++ b/pkg/analyzer/lib/src/dart/element/element.dart
@@ -1706,6 +1706,13 @@ abstract class ElementImpl implements Element {
   int _docRangeLength;
 
   /**
+   * If true, the element is being used as a map key.
+   * Mutations that change operator== (and hashCode)
+   * will throw an exception.
+   */
+  bool frozen = false;

[Brian Wilkerson, 2016/02/01 14:58:16]

This would be better implemented as a Modifier because then it wouldn't increase
the memory usage.

[skybrian, 2016/02/02 02:12:19]

Hmm. That apparently makes it part of the public API?

[Brian Wilkerson, 2016/02/02 16:32:06]

I'm not sure why. The class Modifier isn't part of the public API. Anyway, we
need to resolve the bigger issues before worrying about this one.

+
+  /**
    * Initialize a newly created element to have the given [name] at the given
    * [_nameOffset].
    */
@@ -1755,6 +1762,7 @@ abstract class ElementImpl implements Element {
    * Set the enclosing element of this element to the given [element].
    */
   void set enclosingElement(Element element) {
+    _checkNotFrozen();
     _enclosingElement = element as ElementImpl;
     _cachedLocation = null;
     _cachedHashCode = null;
@@ -1836,6 +1844,7 @@ abstract class ElementImpl implements Element {
   String get name => _name;
 
   void set name(String name) {
+    _checkNotFrozen();

[scheglov, 2016/02/01 16:01:25]

The location of an Element consists of identifiers (names) of all enclosing
elements.
So, in order to guarantee that the location does not change we should freeze not
only this, but also all the enclosing elements.

[Brian Wilkerson, 2016/02/01 16:11:58]

Good point!

If we went with the method on CachePartition we would need to remove and restore
all children of the element being modified.

If we stick with the approach of freezing elements, then we'll probably have to
go with a reference counting scheme so that if multiple children of an element
are both placed in the cache we won't unfreeze the parent until all of the
children are removed. (Which makes me even more concerned about freezing
elements.)

[Brian Wilkerson, 2016/02/02 16:32:06]

It seems to me that the consequences are that we're postponing the problem until
something forces the cached data to be flushed. It appears that the only time
we're flushing the cached hash code is when something changes that would have
demonstrated the problem anyway, so it might not be an issue.
I'm not totally convinced that it's easy :-). Also, I'm starting to get confused
by all of the possibilities, so I'm not sure what changes we're actually looking
at making at this point. Could you update the CL to reflect your current
understanding of what we should do to the code? Then we can move forward from
there.

     this._name = name;
     _cachedLocation = null;
     _cachedHashCode = null;
@@ -1852,6 +1861,7 @@ abstract class ElementImpl implements Element {
    * declaration of this element.
    */
   void set nameOffset(int offset) {
+    _checkNotFrozen();
     _nameOffset = offset;
     _cachedHashCode = null;
     _cachedLocation = null;
@@ -2015,6 +2025,13 @@ abstract class ElementImpl implements Element {
   void visitChildren(ElementVisitor visitor) {
     // There are no children to visit
   }
+
+  void _checkNotFrozen() {
+    if (frozen) {
+      //print("$this at $location is frozen and can't be mutated");

[Brian Wilkerson, 2016/02/01 14:58:16]

Remove debugging code.

[skybrian, 2016/02/02 01:16:54]

I've seen other commented out debugging code checked in.
Should I remove it when I see it?

[skybrian, 2016/02/02 02:12:19]

Done.

[Brian Wilkerson, 2016/02/02 16:32:06]

It's a judgement call. Commented out code tends to get stale quickly, so if it
gets left in it should be really useful code (and even then we should consider
whether it can be re-written so that it can be left uncommented. In this case,
we're already throwing an exception with the same message, so the print
statement would be easy to re-create.

+      throw new StateError("$this at $location is frozen and can't be mutated");
+    }
+  }
 }
 
 /**
@@ -4107,8 +4124,8 @@ class ParameterElementImpl extends VariableElementImpl
   /**
    * Creates a synthetic parameter with [name], [type] and [kind].
    */
-  factory ParameterElementImpl.synthetic(String name, DartType type,
-      ParameterKind kind) {
+  factory ParameterElementImpl.synthetic(
+      String name, DartType type, ParameterKind kind) {
     ParameterElementImpl element = new ParameterElementImpl(name, -1);
     element.type = type;
     element.synthetic = true;