Remove duplicate resolveMember, fix type substitution for subtype of generic class

frog/type.dart

Index: frog/type.dart
diff --git a/frog/type.dart b/frog/type.dart
index bef2a2dce91997e8abb9d8b24a50ba3f2438c4ad..9186dfc5937e6a8515d563d18d55099a250b80ab 100644
--- a/frog/type.dart
+++ b/frog/type.dart
@@ -19,7 +19,10 @@ class Type implements Named, Hashable {
   /** Stubs used to call into this method dynamically. Lazy initialized. */
   Map<String, VarMember> varStubs;
 
-  Type(this.name): isTested = false;
+  /** Cache of [MemberSet]s that have been resolved. */
+  Map<String, MemberSet> _resolvedMembers;
+
+  Type(this.name): isTested = false, _resolvedMembers = {};
 
   void markUsed() {}
   abstract void genMethod(Member method);
@@ -37,7 +40,6 @@ class Type implements Named, Hashable {
 
   abstract Type resolveTypeParams(ConcreteType inType);
 
-  abstract MemberSet resolveMember(String name);
   Member getMember(String name) => null;
   abstract MethodMember getConstructor(String name);
   abstract MethodMember getFactory(Type type, String name);
@@ -46,6 +48,7 @@ class Type implements Named, Hashable {
   abstract addDirectSubtype(Type type);
   abstract bool get isClass();
   abstract Library get library();
+  Set<Type> get subtypes() => null;
 
   // TODO(jmesserly): rename to isDynamic?
   bool get isVar() => false;
@@ -169,6 +172,41 @@ class Type implements Named, Hashable {
     }
   }
 
+  MemberSet resolveMember(String memberName) {
+    MemberSet ret = _resolvedMembers[memberName];
+    if (ret != null) return ret;
+
+    Member member = getMember(memberName);
+    if (member == null) {
+      // TODO(jimhug): Check for members on subtypes given dart's dynamism.
+      return null;
+    }
+
+    // TODO(jimhug): Move this adding subtypes logic to MemberSet?
+    ret = new MemberSet(member);
+    _resolvedMembers[memberName] = ret;
+    if (member.isStatic) {
+      return ret;
+    } else {
+      for (var t in subtypes) {
+        if (!isClass && t.isClass) {
+          // If this is an interface, the actual implementation may
+          // come from a class that does not implement this interface.
+          // TODO(vsm): Use a more efficient lookup strategy.
+          // TODO(jimhug): This is made uglier by need to avoid dups.
+          final m = t.getMember(memberName);
+          if (m != null && ret.members.indexOf(m) == -1) {
+            ret.add(m);
+          }
+        } else {
+          final m = t.members[memberName];
+          if (m != null) ret.add(m);
+        }
+      }
+      return ret;
+    }
+  }
+
   void ensureSubtypeOf(Type other, SourceSpan span, [bool typeErrors=false]) {
     if (!isSubtypeOf(other)) {
       var msg = 'type $name is not a subtype of ${other.name}';
@@ -487,6 +525,7 @@ class ConcreteType extends Type {
   Map<String, Type> typeArguments;
   List<Type> _interfaces;
   Type _parent;
+  Set<Type> _subtypes;
   List<Type> typeArgsInOrder;
 
   bool get isList() => genericType.isList;
@@ -506,7 +545,6 @@ class ConcreteType extends Type {
    * generate appropriate concrete checks on.
    */
   Map<String, Member> members;
-  Map<String, MemberSet> _resolvedMembers;
   Map<String, MethodMember> constructors;
   FactoryMap factories;
 
@@ -514,8 +552,7 @@ class ConcreteType extends Type {
                this.genericType,
                this.typeArguments,
                this.typeArgsInOrder):
-    super(name), constructors = {}, members = {}, _resolvedMembers = {},
-      factories = new FactoryMap();
+    super(name), constructors = {}, members = {}, factories = new FactoryMap();
 
   Type resolveTypeParams(ConcreteType inType) {
     var newTypeArgs = [];
@@ -550,6 +587,16 @@ class ConcreteType extends Type {
     return _interfaces;
   }
 
+  Set<Type> get subtypes() {
+    if (_subtypes == null) {
+      _subtypes = new Set<Type>();
+      for (var s in genericType.subtypes) {
+        _subtypes.add(s.resolveTypeParams(this));

[jimhug, 2011/11/23 19:43:31]

Does this always work?  It's clearly better than what I had (which was to just
use the raw types).  However, it seems that this would get confused when the
names of type params get changed around i.e.
class C<T> {}
class D<X, Y> extends C<Y> {}

Should this work, or do we need a TODO to figure this one out?

[Jennifer Messerly, 2011/11/23 23:39:36]

Doh. Great catch. That's going to be nasty. I'll see if I can come up with a
clean fix.

+      }
+    }
+    return _subtypes;
+  }
+
   // TODO(jmesserly): fill in type args?
   // We can't look in our own members, because we'll get a ConcreteMember
   // which is not fully compatible with MethodMember.
@@ -631,50 +678,12 @@ class ConcreteType extends Type {
     return _getMemberInParents(memberName);
   }
 
-  MemberSet resolveMember(String memberName) {
-    // TODO(jimhug): Cut-and-paste and tweak from Type <frown>.
-    MemberSet ret = _resolvedMembers[memberName];
-    if (ret != null) return ret;
-
-    Member member = getMember(memberName);
-    if (member == null) {
-      // TODO(jimhug): Check for members on subtypes given dart's dynamism.
-      return null;
-    }
-
-    // TODO(jimhug): Move this adding subtypes logic to MemberSet?
-    ret = new MemberSet(member);
-    _resolvedMembers[memberName] = ret;
-    if (member.isStatic) {
-      return ret;
-    } else {
-      for (var t in genericType.subtypes) {
-        // TODO(jimhug): Make these non-generic!
-        if (!isClass && t.isClass) {
-          // If this is an interface, the actual implementation may
-          // come from a class that does not implement this interface.
-          // TODO(vsm): Use a more efficient lookup strategy.
-          // TODO(jimhug): This is made uglier by need to avoid dups.
-          final m = t.getMember(memberName);
-          if (m != null && ret.members.indexOf(m) == -1) {
-            ret.add(m);
-          }
-        } else {
-          final m = t.members[memberName];
-          if (m != null) ret.add(m);
-        }
-      }
-      return ret;
-    }
-  }
-
   Type resolveType(TypeReference node, bool isRequired) {
     var ret = genericType.resolveType(node, isRequired);
     // add type info
     return ret;
   }
 
-
   addDirectSubtype(Type type) {
     // TODO(jimhug): Does this go on the generic type or the concrete one?
     genericType.addDirectSubtype(type);
@@ -708,8 +717,6 @@ class DefinedType extends Type {
   Map<String, Member> members;
   FactoryMap factories;
 
-  Map<String, MemberSet> _resolvedMembers;
-
   Map<String, ConcreteType> _concreteTypes;
 
   /** Methods to be generated once we know for sure that the type is used. */
@@ -720,7 +727,7 @@ class DefinedType extends Type {
 
   DefinedType(String name, this.library, Definition definition, this.isClass)
       : super(name), directSubtypes = new Set<Type>(), constructors = {},
-        members = {}, factories = new FactoryMap(), _resolvedMembers = {} {
+        members = {}, factories = new FactoryMap() {
     setDefinition(definition);
   }
 
@@ -1159,41 +1166,6 @@ class DefinedType extends Type {
     return _getMemberInParents(memberName);
   }
 
-  MemberSet resolveMember(String memberName) {
-    MemberSet ret = _resolvedMembers[memberName];
-    if (ret != null) return ret;
-
-    Member member = getMember(memberName);
-    if (member == null) {
-      // TODO(jimhug): Check for members on subtypes given dart's dynamism.
-      return null;
-    }
-
-    // TODO(jimhug): Move this adding subtypes logic to MemberSet?
-    ret = new MemberSet(member);
-    _resolvedMembers[memberName] = ret;
-    if (member.isStatic) {
-      return ret;
-    } else {
-      for (var t in subtypes) {
-        if (!isClass && t.isClass) {
-          // If this is an interface, the actual implementation may
-          // come from a class that does not implement this interface.
-          // TODO(vsm): Use a more efficient lookup strategy.
-          // TODO(jimhug): This is made uglier by need to avoid dups.
-          final m = t.getMember(memberName);
-          if (m != null && ret.members.indexOf(m) == -1) {
-            ret.add(m);
-          }
-        } else {
-          final m = t.members[memberName];
-          if (m != null) ret.add(m);
-        }
-      }
-      return ret;
-    }
-  }
-
   static String _getDottedName(NameTypeReference type) {
     if (type.names != null) {
       var names = map(type.names, (n) => n.name);