Refactor Library/CompilationUnit and how we define local Scope.

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('dart:uri');
+
 #import('../tree/tree.dart');
 #import('../scanner/scannerlib.dart');
 #import('../leg.dart');  // TODO(karlklose): we only need type.
 #import('../util/util.dart');
 
 class ElementCategory {
   /**
    * Represents things that we don't expect to find when looking in a
    * scope.
    */
@@ skipping 98 matching lines @@
 
   void addMetadata(Node node) {
     metadata = metadata.prepend(node);
   }
 
   bool isFunction() => kind === ElementKind.FUNCTION;
   bool isClosure() => false;
   bool isMember() =>
       enclosingElement !== null && enclosingElement.kind === ElementKind.CLASS;
   bool isInstanceMember() => false;
+  bool isConstructor() => isFactoryConstructor() || isGenerativeConstructor();
   bool isFactoryConstructor() => modifiers !== null && modifiers.isFactory();
   bool isGenerativeConstructor() => kind === ElementKind.GENERATIVE_CONSTRUCTOR;
   bool isGenerativeConstructorBody() =>
       kind === ElementKind.GENERATIVE_CONSTRUCTOR_BODY;
   bool isCompilationUnit() {
     return kind === ElementKind.COMPILATION_UNIT ||
            kind === ElementKind.LIBRARY;
   }
   bool isClass() => kind === ElementKind.CLASS;
   bool isPrefix() => kind === ElementKind.PREFIX;
@@ skipping 105 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);
+  Link<Element> localMembers = const EmptyLink<Element>();
 
-  abstract void addMember(Element element, DiagnosticListener listener);
+  ContainerElement(name, kind, enclosingElement)
+    : super(name, kind, enclosingElement);
+
+  void addMember(Element element, DiagnosticListener listener) {
+    localMembers = localMembers.prepend(element);
+  }
+}
+
+class ScopeContainerElement extends ContainerElement {
+  final Map<SourceString, Element> localScope;
+
+  ScopeContainerElement(name, kind, enclosingElement)
+    : super(name, kind, enclosingElement),
+      localScope = new Map<SourceString, Element>();
+
+  void addMember(Element element, DiagnosticListener listener) {
+    super.addMember(element, listener);
+    addToScope(element, listener);
+  }
+
+  void addToScope(Element element, DiagnosticListener listener) {
+    if (element.isAccessor()) {
+      addGetterOrSetter(element, localScope[element.name], listener);
+    } else {
+      Element existing = localScope.putIfAbsent(element.name, () => element);
+      if (existing !== element) {
+        listener.cancel('duplicate definition', token: element.position());
+        listener.cancel('existing definition', token: existing.position());
+      }
+    }
+  }
+
+  Element localLookup(SourceString elementName) {
+    return localScope[elementName];
+  }
 
   void addGetterOrSetter(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);
     }
 
@@ skipping 19 matching lines @@
       if (element.kind == ElementKind.GETTER) {
         field.getter = element;
       } else {
         field.setter = element;
       }
       addMember(field, listener);
     }
   }
 }
 
+

[ahe, 2012/08/08 17:39:05]

extra lines

+
 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,
             enclosing);
 
-  CompilationUnitElement.library(Script script)
-    : this.script = script,
-      super(new SourceString(script.name), ElementKind.LIBRARY, null);
-
   void addMember(Element element, DiagnosticListener listener) {
-    LibraryElement library = enclosingElement;
-    library.addMember(element, listener);
-    topLevelElements = topLevelElements.prepend(element);
-  }
-
-  void define(Element element, DiagnosticListener listener) {
-    LibraryElement library = enclosingElement;
-    library.define(element, listener);
-  }
-
-  void addTag(ScriptTag tag, DiagnosticListener listener) {
-    listener.cancel("script tags not allowed here", node: tag);
+    // Keep a list of top level members.
+    super.addMember(element, listener);
+    // Provide the member to the library to build scope.
+    getLibrary().addMember(element, listener);
   }
 }
 
-class LibraryElement extends CompilationUnitElement {
-  // TODO(ahe): Library element should not be a subclass of
-  // CompilationUnitElement.
+class LibraryElement extends ScopeContainerElement {
+  CompilationUnitElement entryCompilationUnit;
+  Link<CompilationUnitElement> compilationUnits =
+      const EmptyLink<CompilationUnitElement>();
 
-  Link<CompilationUnitElement> compilationUnits =
-    const EmptyLink<CompilationUnitElement>();
   Link<ScriptTag> tags = const EmptyLink<ScriptTag>();
   ScriptTag libraryTag;
-  Map<SourceString, Element> elements;
   bool canUseNative = false;
   LibraryElement patch = null;
 
   LibraryElement(Script script)
-      : elements = new Map<SourceString, Element>(),
-        super.library(script);
+    : super(new SourceString(script.name), ElementKind.LIBRARY, null) {
+    entryCompilationUnit = new CompilationUnitElement(script, this);

[Lasse Reichstein Nielsen, 2012/08/08 14:22:12]

In the longer run, maybe we should add entryCompilationUnit to compilationUnits.
For now, it's not what uses of compilationUnits expects.

[Anders Johnsen, 2012/08/08 17:30:37]

I partly agree. It would be nice memory-wise, but those compilation units are
very different in meaning.

+  }
+
+  Uri get uri() => entryCompilationUnit.script.uri;
 
   bool get isPatched() => patch !== null;
 
   void addCompilationUnit(CompilationUnitElement element) {
     compilationUnits = compilationUnits.prepend(element);
   }
 
   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());
-      }
-    }
-  }
-
   /** 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];
+    return localScope[elementName];
   }
 
   /** Look up a top-level element in this library, but only look for
     * non-imported elements. Returns null if no such element exist. */
   Element findLocal(SourceString elementName) {
-    Element result = elements[elementName];
+    Element result = localScope[elementName];
     if (result === null || result.getLibrary() != this) return null;
     return result;
   }
 
   void forEachExport(f(Element element)) {
-    elements.forEach((SourceString _, Element e) {
+    localMembers.forEach((Element e) {
       if (this === e.getLibrary()

[Lasse Reichstein Nielsen, 2012/08/08 14:22:12]

The library test should be unnecessary when you only look at localMembers.

[Anders Johnsen, 2012/08/08 17:30:37]

Done.

           && e.kind !== ElementKind.PREFIX
           && e.kind !== ElementKind.FOREIGN) {
         if (!e.name.isPrivate()) f(e);

[Lasse Reichstein Nielsen, 2012/08/08 14:22:12]

Put the !e.name.isPrivate() into the first if too.

[Anders Johnsen, 2012/08/08 17:30:37]

Done.

       }
     });
   }
 
   bool hasLibraryName() => libraryTag !== null;
 
   /**
    * Returns the library name (as defined by the #library tag) or for script
    * (which have no #library tag) the script file name. The latter case is used
    * to private 'library name' for scripts to use for instance in dartdoc.
    */
   String getLibraryOrScriptName() {
     if (libraryTag !== null) {
       return libraryTag.argument.dartString.slowToString();
     } else {
       // Use the file name as script name.
-      var path = script.uri.path;
+      var path = uri.path;

[Lasse Reichstein Nielsen, 2012/08/08 14:22:12]

var -> String.

[Anders Johnsen, 2012/08/08 17:30:37]

While I believe we are moving away from this style, I'll do it :)

       return path.substring(path.lastIndexOf('/') + 1);
     }
   }
 
+  CompilationUnitElement getCompilationUnit() => entryCompilationUnit;
+
   Scope buildEnclosingScope() => new TopScope(this);
 }
 
 class PrefixElement extends Element {
   Map<SourceString, Element> imported;
   Token firstPosition;
-  final CompilationUnitElement patchSource;
+  final LibraryElement patchLibrary;

[Lasse Reichstein Nielsen, 2012/08/08 14:22:12]

Why not store the CompilationUnitElement of patchLibrary.entryCompilationUnit
directly, instead of storing the library?

[Anders Johnsen, 2012/08/08 17:30:37]

Done.

 
   PrefixElement(SourceString prefix, Element enclosing, this.firstPosition,
-                [this.patchSource])
+                [this.patchLibrary])
     : imported = new Map<SourceString, Element>(),
       super(prefix, ElementKind.PREFIX, enclosing);
 
   CompilationUnitElement getCompilationUnit() {
-    if (patchSource !== null) return patchSource;
+    if (patchLibrary !== null) return patchLibrary.entryCompilationUnit;
     return super.getCompilationUnit();
   }
 
   lookupLocalMember(SourceString memberName) => imported[memberName];
 
   Type computeType(Compiler compiler) => compiler.types.dynamicType;
 
   Token position() => firstPosition;
 }
 
@@ skipping 430 matching lines @@
       TypeVariableElement variableElement =
           new TypeVariableElement(variableName, element, node);
       TypeVariableType variableType = new TypeVariableType(variableElement);
       variableElement.type = variableType;
       arguments.addLast(variableType);
     }
     return arguments.toLink();
   }
 }
 
-class ClassElement extends ContainerElement
+class ClassElement extends ScopeContainerElement
     implements TypeDeclarationElement {
   final int id;
   InterfaceType type;
   Type supertype;
   Type defaultClass;
-  Link<Element> members = const EmptyLink<Element>();
-  Map<SourceString, Element> localMembers;
-  Map<SourceString, Element> constructors;
   Link<Type> interfaces = const EmptyLink<Type>();
   bool isResolved = false;
   bool isBeingResolved = false;
   // backendMembers are members that have been added by the backend to simplify
   // compilation. They don't have any user-side counter-part.
   Link<Element> backendMembers = const EmptyLink<Element>();
 
   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);
-    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;
-    }
-  }
+    : super(name, ElementKind.CLASS, enclosing);
 
   InterfaceType computeType(compiler) {
     if (type == null) {
       ClassNode node = parseNode(compiler);
       Link<Type> parameters =
           TypeDeclarationElement.createTypeVariables(this, node.typeParameters);
       type = new InterfaceType(this, parameters);
     }
     return type;
   }
 
   Link<Type> get typeVariables() => type.arguments;
 
   ClassElement ensureResolved(Compiler compiler) {
     compiler.resolveClass(this);
     return this;
   }
 
+  /**
+   * Lookup local members in the class. This will ignore constructors.
+   */
   Element lookupLocalMember(SourceString memberName) {
-    return localMembers[memberName];
+    var result = localLookup(memberName);
+    if (result !== null && result.isConstructor()) return null;
+    return result;
   }
 
+  /**
+  * Lookup super members for the class. This will ignore constructors.
+   */
   Element lookupSuperMember(SourceString memberName) {
     bool isPrivate = memberName.isPrivate();
     for (ClassElement s = superclass; s != null; s = s.superclass) {
       // Private members from a different library are not visible.
       if (isPrivate && getLibrary() !== s.getLibrary()) continue;
       Element e = s.lookupLocalMember(memberName);
       if (e === null) continue;
       // Static members are not inherited.
       if (e.modifiers.isStatic()) continue;
       return e;
     }
     return null;
   }
 
   /**
    * Find the first member in the class chain with the given
    * [memberName]. This method is NOT to be used for resolving
    * unqualified sends because it does not implement the scoping
    * rules, where library scope comes before superclass scope.
    */
   Element lookupMember(SourceString memberName) {
-    Element localMember = localMembers[memberName];
+    Element localMember = lookupLocalMember(memberName);
     return localMember === null ? lookupSuperMember(memberName) : localMember;
   }
 
   /**
    * Returns true if the [fieldMember] is shadowed by another field. The given
    * [fieldMember] must be a member of this class.
    *
    * This method also works if the [fieldMember] is private.
    */
   bool isShadowedByField(Element fieldMember) {
@@ skipping 24 matching lines @@
                             Element noMatch(Element)]) {
     // TODO(karlklose): have a map from class names to a map of constructors
     //                  instead of creating the name here?
     SourceString normalizedName;
     if (constructorName !== const SourceString('')) {
       normalizedName = Elements.constructConstructorName(className,
                                                          constructorName);
     } else {
       normalizedName = className;
     }
-    Element result = constructors[normalizedName];
-    if (result === null && noMatch !== null) {
-      result = noMatch(lookupLocalMember(constructorName));
+    return localLookup(normalizedName);
+  }
+
+  bool get hasConstructor() {
+    // Search in scope to be sure we search patched constructors.

[Lasse Reichstein Nielsen, 2012/08/08 14:22:12]

Huh? Patched constructors are also members, as are injected constructors.

[Anders Johnsen, 2012/08/08 17:30:37]

Yes, but they are going to be hidden under a PatchedCompilationUnit, hiding the
properties of isConstructor(). We do, however, know that localScope is the
direct members that we expose from this class. Also, things as default
constructor implementations does now enter localMembers but only localScope.

+    for (var element in localScope.getValues()) {
+      if (element.isConstructor()) return true;
     }
-    return result;
+    return false;
+  }
+
+  Collection<Element> get constructors() {
+    // TODO(ajohnsen): See if we can avoid this method at some point.
+    var result = <Element>[];
+    // Search in scope to be sure we search patched constructors.
+    return localScope.getValues().filter((element) => element.isConstructor());
   }
 
   /**
    * Returns the super class, if any.
    *
    * The returned element may not be resolved yet.
    */
   ClassElement get superclass() {
     assert(isResolved);
     return supertype === null ? null : supertype.element;
   }
 
   /**
    * Runs through all members of this class.
    *
    * The enclosing class is passed to the callback. This is useful when
    * [includeSuperMembers] is [:true:].
    */
   void forEachMember([void f(ClassElement enclosingClass, Element member),
                       includeBackendMembers = false,
                       includeSuperMembers = false]) {
     Set<ClassElement> seen = new Set<ClassElement>();
     ClassElement classElement = this;
     do {
       if (seen.contains(classElement)) return;
       seen.add(classElement);
-      for (Element element in classElement.members) {
+      for (Element element in classElement.localMembers) {
         f(classElement, element);
       }
       if (includeBackendMembers) {
         for (Element element in classElement.backendMembers) {
           f(classElement, element);
         }
       }
       classElement = includeSuperMembers ? classElement.superclass : null;
     } while(classElement !== null);
   }
@@ skipping 237 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()}";
 }