Rewrite, document, and test qualifiedNamesIn.

dart/pkg/dart2js_incremental/lib/library_updater.dart

Index: dart/pkg/dart2js_incremental/lib/library_updater.dart
diff --git a/dart/pkg/dart2js_incremental/lib/library_updater.dart b/dart/pkg/dart2js_incremental/lib/library_updater.dart
index 362123509c1737f8afc08b37b001e27271da4db0..c4ca10b9574649f9add2f1b5af2f4058bcaf13e0 100644
--- a/dart/pkg/dart2js_incremental/lib/library_updater.dart
+++ b/dart/pkg/dart2js_incremental/lib/library_updater.dart
@@ -235,12 +235,12 @@ class LibraryUpdater extends JsFeatures {
   bool canReuseRemovedFunction(PartialFunctionElement element) {
     logVerbose("Removed method $element.");
 
-    PartialClassElement cls = element.enclosingClass;
-    for (ScopeContainerElement scope in scopesAffectedBy(element, cls)) {
+    ScopeContainerElement container = element.enclosingElement;
+    for (ScopeContainerElement scope in scopesAffectedBy(element, container)) {
       scanSites(scope, (Element member, DeclarationSite site) {
         // TODO(ahe): Cache qualifiedNamesIn to avoid quadratic behavior.
         Map<String, List<String>> names = qualifiedNamesIn(site);
-        if (canNamesResolveTo(names, element, cls)) {
+        if (canNamesResolveTo(names, element, container)) {
           _elementsToInvalidate.add(member);
         }
       });
@@ -253,6 +253,8 @@ class LibraryUpdater extends JsFeatures {
     return true;
   }
 
+  /// Invoke [f] on each [DeclarationSite] in [element]. If [element] is a
+  /// [ScopeContainerElement], invoke f on all local members as well.
   void scanSites(
       Element element,
       void f(ElementX element, DeclarationSite site)) {
@@ -265,15 +267,19 @@ class LibraryUpdater extends JsFeatures {
     }
   }
 
+  /// Assume [element] is either removed from or added to [container], and
+  /// return all [ScopeContainerElement] that can see this change.
   List<ScopeContainerElement> scopesAffectedBy(
       Element element,
-      ClassElement cls) {
+      ScopeContainerElement container) {
     // TODO(ahe): Use library export graph to compute this.
     // TODO(ahe): Should return all user-defined libraries and packages.
-    LibraryElement library = element.library;
+    LibraryElement library = container.library;
     List<ScopeContainerElement> result = <ScopeContainerElement>[library];
 
-    if (cls == null) return result;
+    if (!container.isClass) return result;
+
+    ClassElement cls = container;
 
     var externalSubtypes =
         compiler.world.subtypesOf(cls).where((e) => e.library != library);
@@ -674,7 +680,11 @@ class RemovedFunctionUpdate extends Update with JsFeatures, ReuseFunction {
   }
 }
 
-Map<String, List<String>> qualifiedNamesIn(PartialElement element) {
+/// Returns all qualified names in [element] with less than four identifiers. A
+/// qualified name is an identifier followed by a sequence of dots and
+/// identifiers, for example, "x", and "x.y.z". But not "x.y.z.w" ("w" is the
+/// fourth identifier).

[Johnni Winther, 2014/11/14 10:33:35]

I guess the reason for the limit is prefix.Class.staticProperty is the longest
qualified name for a statically resolved entity. 

Please add the (this) reasoning to documentation.

[ahe, 2014/11/17 10:39:47]

Done.

+Set<String> qualifiedNamesIn(PartialElement element) {
   Token beginToken = element.beginToken;
   Token endToken = element.endToken;
   Token token = beginToken;
@@ -687,15 +697,33 @@ Map<String, List<String>> qualifiedNamesIn(PartialElement element) {
       }
     }
   }
-  Map<String, List<String>> names = new Map<String, List<String>>();
-  List<List<Token>> qualifieds = <List<Token>>[];
+  Set<String> names = new Set<String>();
   do {
     if (token.isIdentifier()) {
-      List<String> name = names.putIfAbsent(token.value, () => <String>[]);
-      while (identical('.', token.next.stringValue) &&
-             token.next.next.isIdentifier()) {
+      String name = token.value;
+      // [name] is a single "identifier".
+      names.add(name);
+      if (identical('.', token.next.stringValue) &&
+          token.next.next.isIdentifier()) {
         token = token.next.next;
-        name.add(token.value);
+        name += '.${token.value}';
+        // [name] is "idenfifier.idenfifier".
+        names.add(name);
+
+        if (identical('.', token.next.stringValue) &&
+            token.next.next.isIdentifier()) {
+          token = token.next.next;
+          name += '.${token.value}';
+          // [name] is "idenfifier.idenfifier.idenfifier".
+          names.add(name);
+
+          while (identical('.', token.next.stringValue) &&
+                 token.next.next.isIdentifier()) {
+            // Skip remaining identifiers, they cannot statically resolve to
+            // anything, and must be dynamic sends.
+            token = token.next.next;
+          }
+        }
       }
     }
     token = token.next;
@@ -703,21 +731,19 @@ Map<String, List<String>> qualifiedNamesIn(PartialElement element) {
   return names;
 }
 
+/// Returns true if one of the qualified names in names (as computed by
+/// [qualifiedNamesIn]) could be a static reference to [element].
 bool canNamesResolveTo(

[Johnni Winther, 2014/11/14 10:33:35]

Maybe rename to 'canNamesResolveStaticallyTo'

[ahe, 2014/11/17 10:39:47]

Done.

-    Map<String, List<String>> names,
+    Set<String> names,
     Element element,
-    ClassElement cls) {
-  if (names.containsKey(element.name)) {
-    return true;
-  }
-  if (cls != null) {
-    List<String> rest = names[cls.name];
-    if (rest != null && rest.contains(element.name)) {
-      // [names] contains C.m, where C is the name of [cls], and m is the name
-      // of [element].
-      return true;
-    }
-  }
+    ScopeContainerElement container) {
+  if (names.contains(element.name)) return true;
+  if (container != null && container.isClass) {
+    // [names] contains C.m, where C is the name of [container], and m is the
+    // name of [element].
+    if (names.contains("${container.name}.${element.name}")) return true;
+  }
+  // TODO(ahe): Check for prefixes as well.
   return false;
 }