Make addGetterOrSetter (now: defineAccessor) not add the abstract field as a member.

lib/compiler/implementation/elements/elements.dart

 // Copyright (c) 2012, 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.
 
 #library('elements');
 
 #import('../tree/tree.dart');
 #import('../scanner/scannerlib.dart');
 #import('../leg.dart');  // TODO(karlklose): we only need type.
 #import('../util/util.dart');
@@ skipping 229 matching lines @@
   }
 
   bool _isNative = false;
   void setNative() { _isNative = true; }
   bool isNative() => _isNative;
 
   FunctionElement asFunctionElement() => null;
 }
 
 class ContainerElement extends Element {
-  ContainerElement(name, kind, enclosingElement) :
-    super(name, kind, enclosingElement);
+  ContainerElement(name, kind, enclosingElement)
+      : super(name, kind, enclosingElement);
 
   abstract void addMember(Element element, DiagnosticListener listener);
 
-  void addGetterOrSetter(Element element,
-                         Element existing,
-                         DiagnosticListener listener) {
+  /**
+   * Adds a definition for an accessor (getter or setter) to a container.

[ahe, 2012/08/10 11:21:46]

Please use the [] notation to refer to the arguments. For example:

"Adds a definition for an [accessor] ..."

And rename the parameter accordingly.

[Lasse Reichstein Nielsen, 2012/08/13 13:37:51]

Done.

+   * The definition binds to an abstract field that can hold both a getter
+   * and a setter.
+   *
+   * The abstract field is added once, for the first getter or setter, and
+   * reused if the other one is also added.
+   * The abstract field should not be treated as a proper member of the
+   * container, it's simply a way to return two results for one lookup.
+   * I.e., the getter or setter does not have the abstract field as enclosing

[ahe, 2012/08/10 11:21:46]

Use English, not Latin. That is, use "that is", not "i.e."

[Lasse Reichstein Nielsen, 2012/08/13 13:37:51]

Done.

+   * element, they are enclosed by the class, as is the abstract field.
+   */
+  void defineAccessor(Element element,
+                      Element existing,
+                      DiagnosticListener listener) {
     void reportError(Element other) {
       listener.cancel('duplicate definition of ${element.name.slowToString()}',
                       element: element);
       listener.cancel('existing definition', element: other);
     }
 
     if (existing != null) {
       if (existing.kind !== ElementKind.ABSTRACT_FIELD) {
         reportError(existing);
       } else {
         AbstractFieldElement field = existing;
         if (element.kind == ElementKind.GETTER) {
           if (field.getter != null && field.getter != element) {
             reportError(field.getter);
           }
           field.getter = element;
         } else {
+          assert(element.kind == ElementKind.SETTER);

[ahe, 2012/08/10 11:21:46]

This assertion does not hold if I write:

class Foo {
  get x() => null;
  get x() => null;
}

[Lasse Reichstein Nielsen, 2012/08/13 13:37:51]

I don't see how. Either element is a getter or it's a setter, or you shouldn't
have called this function. For the second getter in the example above, this will
report an error, and then overwrite the first getter on the abstract field with
the second getter.

           if (field.setter != null && field.setter != element) {
             reportError(field.setter);
           }
           field.setter = element;
         }
       }
     } else {
       AbstractFieldElement field = new AbstractFieldElement(element.name, this);
       if (element.kind == ElementKind.GETTER) {
         field.getter = element;
       } else {
         field.setter = element;
       }
-      addMember(field, listener);
+      define(field, listener);
     }
   }
+
+  abstract void define(Element element, DiagnosticListener listener);

[Anders Johnsen, 2012/08/07 13:02:30]

Could you add a short description to this member?

[Lasse Reichstein Nielsen, 2012/08/13 13:37:51]

Done.

 }
 
 class CompilationUnitElement extends ContainerElement {
   final Script script;
   Link<Element> topLevelElements = const EmptyLink<Element>();
 
   CompilationUnitElement(Script script, Element enclosing)
     : this.script = script,
       super(new SourceString(script.name),
             ElementKind.COMPILATION_UNIT,
@@ skipping 44 matching lines @@
   void addTag(ScriptTag tag, DiagnosticListener listener) {
     tags = tags.prepend(tag);
   }
 
   void addMember(Element element, DiagnosticListener listener) {
     topLevelElements = topLevelElements.prepend(element);
     define(element, listener);
   }
 
   void define(Element element, DiagnosticListener listener) {
-    if (element.kind == ElementKind.GETTER
-        || element.kind == ElementKind.SETTER) {
-      addGetterOrSetter(element, elements[element.name], listener);
-    } else {
-      Element existing = elements.putIfAbsent(element.name, () => element);
-      if (existing !== element) {
-        listener.cancel('duplicate definition', token: element.position());
-        listener.cancel('existing definition', token: existing.position());
-      }
+    if (element.isAccessor()) {
+      defineAccessor(element, find(element.name), listener);
+      return;
+    }
+    Element existing = elements.putIfAbsent(element.name, () => element);
+    if (existing !== element) {
+      listener.cancel('duplicate definition', token: element.position());
+      listener.cancel('existing definition', token: existing.position());
     }
   }
 
   /** Look up a top-level element in this library. The element could
     * potentially have been imported from another library. Returns
     * null if no such element exist. */
   Element find(SourceString elementName) {
     return elements[elementName];
   }
 
@@ skipping 516 matching lines @@
   Link<Type> allSupertypes;
   ClassElement patch = null;
 
   ClassElement(SourceString name, CompilationUnitElement enclosing, this.id)
     : localMembers = new Map<SourceString, Element>(),
       constructors = new Map<SourceString, Element>(),
       super(name, ElementKind.CLASS, enclosing);
 
   void addMember(Element element, DiagnosticListener listener) {
     members = members.prepend(element);
+    define(element, listener);
+  }
+
+  void define(Element element, DiagnosticListener listener) {
+    if (element.isAccessor()) {
+      defineAccessor(element, localMembers[element.name], listener);
+      return;
+    }
     if (element.kind == ElementKind.GENERATIVE_CONSTRUCTOR ||
         element.modifiers.isFactory()) {
       constructors[element.name] = element;
-    } else if (element.kind == ElementKind.GETTER
-               || element.kind == ElementKind.SETTER) {
-      addGetterOrSetter(element, localMembers[element.name], listener);
     } else {
       localMembers[element.name] = element;
     }
   }
 
   InterfaceType computeType(compiler) {
     if (type == null) {
       ClassNode node = parseNode(compiler);
       Link<Type> parameters =
           TypeDeclarationElement.createTypeVariables(this, node.typeParameters);
@@ skipping 332 matching lines @@
   TypeVariableElement(name, Element enclosing, this.cachedNode,
                       [this.type, this.bound])
     : super(name, ElementKind.TYPE_VARIABLE, enclosing);
 
   TypeVariableType computeType(compiler) => type;
 
   Node parseNode(compiler) => cachedNode;
 
   String toString() => "${enclosingElement.toString()}.${name.slowToString()}";
 }