Handle type variables.

pkg/front_end/lib/src/fasta/source/source_library_builder.dart

Index: pkg/front_end/lib/src/fasta/source/source_library_builder.dart
diff --git a/pkg/front_end/lib/src/fasta/source/source_library_builder.dart b/pkg/front_end/lib/src/fasta/source/source_library_builder.dart
index 934b98092526ce2a7b811d3fbff0ea7dd465f94b..48696b7ba1d0b07802f5effaf9a596ced400c6f0 100644
--- a/pkg/front_end/lib/src/fasta/source/source_library_builder.dart
+++ b/pkg/front_end/lib/src/fasta/source/source_library_builder.dart
@@ -47,9 +47,7 @@ abstract class SourceLibraryBuilder<T extends TypeBuilder, R>
     extends LibraryBuilder<T, R> {
   final SourceLoader loader;
 
-  final Map<String, Builder> members = <String, Builder>{};
-
-  final List<T> types = <T>[];
+  final BuilderScope<T> libraryScope = new BuilderScope<T>(<String, Builder>{});
 
   final List<ConstructorReferenceBuilder> constructorReferences =
       <ConstructorReferenceBuilder>[];
@@ -70,10 +68,7 @@ abstract class SourceLibraryBuilder<T extends TypeBuilder, R>
 
   List<MetadataBuilder> metadata;
 
-  Map<String, MemberBuilder> classMembers;
-
-  // TODO(ahe): Rename this. It's not just for classes.
-  List<T> classTypes;
+  BuilderScope<T> innerScope;

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

+dartdoc:

/// Type-scope for the current builder (current class or current method?)

[ahe, 2017/02/08 13:31:41]

Added a bit more information.

 
   SourceLibraryBuilder(this.loader, this.fileUri);
 
@@ -81,13 +76,30 @@ abstract class SourceLibraryBuilder<T extends TypeBuilder, R>
 
   bool get isPart => partOf != null;
 
+  Map<String, Builder> get members => libraryScope.members;
+
+  List<T> get types => libraryScope.types;
+
+  BuilderScope<T> get builderScope => innerScope ?? libraryScope;

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

thinking more about it - here is an alternative idea:

delete builderScope,  rename innerScope => currentScope and make it always
non-null (set it to libraryScope at first).

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

I'm not against this name, another idea could be `currentScope`.

[ahe, 2017/02/08 13:31:40]

I like that idea. Perhaps `currentDeclaration`.

[ahe, 2017/02/08 16:57:21]

Done in CL 2682993002.

+
+  Map<String, MemberBuilder> get classMembers {

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

+dartdoc, when currently within a class, this returns the builder for each
member in the class.

it will be good to highlight also what's the invariant here: is it all members
of the class, or only members that has been parsed?

[ahe, 2017/02/08 13:31:41]

Done.

+    assert(innerScope == builderScope);
+    assert(innerScope.parent == libraryScope);
+    return builderScope.members;

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

since you are anyways assuming this is only called when innerScope ==
builderScope, consider skipping the indirection here?
return innerScope.members;?

[ahe, 2017/02/08 13:31:41]

Yes, but you suggestion above makes this unnecessary.

+  }
+
+  List<T> get declarationTypes {

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

+dartdoc: the type variables declared within the current class or generic
method.

consider adding an example of each:
class:
  [T,R] for class C<T, R> { ... }

top-level method:
  [G,H] for void m<G, H>() { ... }

generic method within class:
  [R] for class C<T> { m<R>() { ... } }
?

[ahe, 2017/02/08 13:31:41]

I already have CL on top of this that removes this getter.

+    assert(innerScope == builderScope);
+    assert(innerScope.parent == libraryScope);
+    return builderScope.types;

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

ditto => innerScope.types;

+  }
+
   T addInterfaceType(String name, List<T> arguments);
 
   T addMixinApplication(T supertype, List<T> mixins);
 
   T addType(T type) {
-    List<T> types = classTypes ?? this.types;
-    types.add(type);
+    builderScope.addType(type);

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

+ assert(innerScope != null) ?

[ahe, 2017/02/08 13:31:41]

I don't think that will work right now. I don't enter a nested scope when
parsing fields.

     return type;
   }
 
@@ -101,14 +113,14 @@ abstract class SourceLibraryBuilder<T extends TypeBuilder, R>
     return ref;
   }
 
-  void beginNestedScope() {
-    classMembers = <String, MemberBuilder>{};
-    classTypes = <T>[];
+  void beginNestedScope({bool hasMembers}) {
+    innerScope = new BuilderScope(<String, MemberBuilder>{}, builderScope);
   }
 
-  void endNestedScope() {
-    classMembers = null;
-    classTypes = null;
+  BuilderScope<T> endNestedScope() {
+    BuilderScope<T> previous = innerScope;
+    innerScope = innerScope.parent;
+    return previous;
   }
 
   Uri resolve(String path) => uri.resolve(path);
@@ -188,7 +200,7 @@ abstract class SourceLibraryBuilder<T extends TypeBuilder, R>
     // TODO(ahe): Set the parent correctly here. Could then change the
     // implementation of MemberBuilder.isTopLevel to test explicitly for a
     // LibraryBuilder.
-    if (classMembers == null) {
+    if (builderScope == libraryScope) {
       if (builder is MemberBuilder) {
         builder.parent = this;
       } else if (builder is TypeDeclarationBuilder) {
@@ -198,8 +210,10 @@ abstract class SourceLibraryBuilder<T extends TypeBuilder, R>
       } else {
         return internalError("Unhandled: ${builder.runtimeType}");
       }
+    } else {
+      assert(builderScope.parent == libraryScope);
     }
-    Map<String, Builder> members = classMembers ?? this.members;
+    Map<String, Builder> members = builderScope.members;
     Builder existing = members[name];
     builder.next = existing;
     if (builder is PrefixBuilder && existing is PrefixBuilder) {
@@ -340,3 +354,56 @@ abstract class SourceLibraryBuilder<T extends TypeBuilder, R>
     return count;
   }
 }
+
+/// Unlike [Scope], this scope is used during construction of builders to
+/// ensure types and members are added to and resolved in the correct location.
+class BuilderScope<T extends TypeBuilder> {

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

Another idea - TypeScope or TypeBuilderScope ?

[ahe, 2017/02/08 13:31:41]

Not a bad idea, but the class does more than handle types. What about
"DeclarationBuilder"? This class helps build declarations.

+  final BuilderScope<T> parent;
+
+  final Map<String, Builder> members;
+
+  final List<T> types = <T>[];
+
+  BuilderScope(this.members, [this.parent]);
+
+  void addMember(String name, MemberBuilder builder) {
+    if (members == null) {
+      parent.addMember(name, builder);
+    } else {
+      members[name] = builder;
+    }
+  }
+
+  MemberBuilder lookupMember(String name) {
+    return members == null ? parent.lookupMember(name) : members[name];
+  }
+
+  void addType(T type) {
+    types.add(type);
+  }
+
+  /// Resolves type variables in [types] and propagate other types to [parent].
+  void resolveTypes(List<TypeVariableBuilder> typeVariables) {
+    if (typeVariables == null) {
+      parent.types.addAll(types);

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

it might be worth adding some comments to explain in what contexts this happens

[ahe, 2017/02/08 16:57:21]

Done.

+    } else {
+      Map<String, TypeVariableBuilder> map = <String, TypeVariableBuilder>{};
+      for (TypeVariableBuilder builder in typeVariables) {
+        map[builder.name] = builder;
+      }
+      for (T type in types) {
+        String name = type.name;
+        TypeVariableBuilder builder;
+        if (name != null) {
+          builder = map[name];
+        }
+        if (builder == null) {
+          parent.addType(type);

[Siggi Cherem (dart-lang), 2017/02/06 18:05:11]

here too.

[ahe, 2017/02/08 16:57:21]

Done.

+        } else {
+          type.bind(builder);
+        }
+      }
+    }
+    types.clear();
+  }
+}