dart2js: store fields in the model and make the emitters use it.

pkg/compiler/lib/src/js_emitter/program_builder.dart

 // Copyright (c) 2014, 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 dart2js.js_emitter.program_builder;
 
 import 'js_emitter.dart' show computeMixinClass;
 import 'model.dart';
 
 import '../common.dart';
 import '../js/js.dart' as js;
 
 import '../js_backend/js_backend.dart' show
     Namer,
     JavaScriptBackend,
     JavaScriptConstantCompiler;
 
-import '../closure.dart' show ClosureFieldElement;
-
 import 'js_emitter.dart' as emitterTask show
     CodeEmitterTask,
     Emitter,
     InterceptorStubGenerator,
     TypeTestGenerator,
     TypeTestProperties;
 
 import '../universe/universe.dart' show Universe;
 import '../deferred_load.dart' show DeferredLoadTask, OutputUnit;
 
@@ skipping 220 matching lines @@
     if (library == backend.interceptorsLibrary) {
       statics.addAll(_generateGetInterceptorMethods());
       statics.addAll(_generateOneShotInterceptors());
     }
 
     List<Class> classes = elements
         .where((e) => e is ClassElement)
         .map(_buildClass)
         .toList(growable: false);
 
-    return new Library(library, uri, statics, classes);
+    bool visitStatics = true;
+    List<Field> staticFieldsForReflection = _buildFields(library, visitStatics);
+
+    return new Library(library, uri, statics, classes,
+                       staticFieldsForReflection);
   }
 
   Class _buildClass(ClassElement element) {
+    bool onlyForRti = _task.typeTestRegistry.rtiNeededClasses.contains(element);
+
     List<Method> methods = [];
-    List<InstanceField> fields = [];
 
     void visitMember(ClassElement enclosing, Element member) {
       assert(invariant(element, member.isDeclaration));
       assert(invariant(element, element == enclosing));
 
       if (Elements.isNonAbstractInstanceMember(member)) {
         js.Expression code = backend.generatedCode[member];
         // TODO(kasperl): Figure out under which conditions code is null.
         if (code != null) methods.add(_buildMethod(member, code));
-      } else if (member.isField && !member.isStatic) {
-        fields.add(_buildInstanceField(member, enclosing));
       }
     }
 
     ClassElement implementation = element.implementation;
 
     // MixinApplications run through the members of their mixin. Here, we are
     // only interested in direct members.
     if (!element.isMixinApplication) {
       implementation.forEachMember(visitMember, includeBackendMembers: true);
     }
 
+    List<Field> instanceFields =
+        onlyForRti ? const <Field>[] : _buildFields(element, false);
+    List<Field> staticFieldsForReflection =
+        onlyForRti ? const <Field>[] : _buildFields(element, true);
+
     emitterTask.TypeTestGenerator generator =
         new emitterTask.TypeTestGenerator(_compiler, _task, namer);
     emitterTask.TypeTestProperties typeTests =
         generator.generateIsTests(
             element,
             storeFunctionTypeInMetadata: _storeFunctionTypesInMetadata);
 
     List<StubMethod> isChecks = <StubMethod>[];
     typeTests.properties.forEach((String name, js.Node code) {
       isChecks.add(_buildStubMethod(name, code));
     });
 
     String name = namer.getNameOfClass(element);
     String holderName = namer.globalObjectFor(element);
     Holder holder = _registry.registerHolder(holderName);
-    bool onlyForRti = _task.typeTestRegistry.rtiNeededClasses.contains(element);
     bool isInstantiated =
         _compiler.codegenWorld.directlyInstantiatedClasses.contains(element);
 
     Class result;
     if (element.isMixinApplication && !onlyForRti) {
       assert(!element.isNative);
       assert(methods.isEmpty);
-      assert(fields.isEmpty);
 
       result = new MixinApplication(element,
-                                    name, holder, isChecks,
+                                    name, holder,
+                                    instanceFields,
+                                    staticFieldsForReflection,
+                                    isChecks,
                                     typeTests.functionTypeIndex,
                                     isDirectlyInstantiated: isInstantiated,
                                     onlyForRti: onlyForRti);
     } else {
       result = new Class(element,
-                         name, holder, methods, fields, isChecks,
+                         name, holder, methods, instanceFields,
+                         staticFieldsForReflection,
+                         isChecks,
                          typeTests.functionTypeIndex,
                          isDirectlyInstantiated: isInstantiated,
                          onlyForRti: onlyForRti,
                          isNative: element.isNative);
     }
     _classes[element] = result;
     return result;
   }
 
   Method _buildMethod(FunctionElement element, js.Expression code) {
@@ skipping 32 matching lines @@
         backend.specializedGetInterceptors;
     List<String> names = specializedGetInterceptors.keys.toList()..sort();
     return names.map((String name) {
       Set<ClassElement> classes = specializedGetInterceptors[name];
       js.Expression code = stubGenerator.generateGetInterceptorMethod(classes);
       // TODO(floitsch): compute `needsTearOff`.
       return new StaticStubMethod(name, holder, code, needsTearOff: false);
     });
   }
 
-  bool _fieldNeedsGetter(VariableElement field) {

[floitsch, 2015/01/22 15:57:39]

These predicates were copied from the classEmitter. The program-builder is now
using the classEmitter's `visitField` (line 384), and therefore doesn't need it
anymore.

-    assert(field.isField);
-    if (_fieldAccessNeverThrows(field)) return false;
-    return backend.shouldRetainGetter(field)
-        || _compiler.codegenWorld.hasInvokedGetter(field, _compiler.world);
-  }
+  List<Field> _buildFields(Element holder, bool visitStatics) {
+    List<Field> fields = <Field>[];
+    _task.oldEmitter.classEmitter.visitFields(
+        holder, visitStatics, (VariableElement field,
+                               String name,
+                               String accessorName,
+                               bool needsGetter,
+                               bool needsSetter,
+                               bool needsCheckedSetter) {
+      assert(invariant(field, field.isDeclaration));
 
-  bool _fieldNeedsSetter(VariableElement field) {
-    assert(field.isField);
-    if (_fieldAccessNeverThrows(field)) return false;
-    return (!field.isFinal && !field.isConst)
-        && (backend.shouldRetainSetter(field)
-            || _compiler.codegenWorld.hasInvokedSetter(field, _compiler.world));
-  }
+      int getterFlags = 0;
+      if (needsGetter) {
+        if (visitStatics || !backend.fieldHasInterceptedGetter(field)) {
+          getterFlags = 1;
+        } else {
+          getterFlags += 2;
+          // TODO(sra): 'isInterceptorClass' might not be the correct test
+          // for methods forced to use the interceptor convention because
+          // the method's class was elsewhere mixed-in to an interceptor.
+          if (!backend.isInterceptorClass(holder)) {
+            getterFlags += 1;
+          }
+        }
+      }
 
-  // We never access a field in a closure (a captured variable) without knowing
-  // that it is there.  Therefore we don't need to use a getter (that will throw
-  // if the getter method is missing), but can always access the field directly.
-  bool _fieldAccessNeverThrows(VariableElement field) {
-    return field is ClosureFieldElement;
-  }
+      int setterFlags = 0;
+      if (needsSetter) {
+        if (visitStatics || !backend.fieldHasInterceptedSetter(field)) {
+          setterFlags = 1;
+        } else {
+          setterFlags += 2;
+          if (!backend.isInterceptorClass(holder)) {
+            setterFlags += 1;
+          }
+        }
+      }
 
-  InstanceField _buildInstanceField(VariableElement field,
-                                    ClassElement holder) {
-    assert(invariant(field, field.isDeclaration));
-    String name = namer.fieldPropertyName(field);
+      fields.add(new Field(field, name, accessorName,
+                           getterFlags, setterFlags,
+                           needsCheckedSetter));
+    });
 
-    int getterFlags = 0;
-    if (_fieldNeedsGetter(field)) {
-      bool isIntercepted = backend.fieldHasInterceptedGetter(field);
-      if (isIntercepted) {
-        getterFlags += 2;
-        if (!backend.isInterceptorClass(holder)) {
-          getterFlags += 1;
-        }
-      } else {
-        getterFlags = 1;
-      }
-    }
-
-    int setterFlags = 0;
-    if (_fieldNeedsSetter(field)) {
-      bool isIntercepted = backend.fieldHasInterceptedSetter(field);
-      if (isIntercepted) {
-        setterFlags += 2;
-        if (!backend.isInterceptorClass(holder)) {
-          setterFlags += 1;
-        }
-      } else {
-        setterFlags = 1;
-      }
-    }
-
-    return new InstanceField(field, name, getterFlags, setterFlags);
+    return fields;
   }
 
   Iterable<StaticMethod> _generateOneShotInterceptors() {
     emitterTask.InterceptorStubGenerator stubGenerator =
         new emitterTask.InterceptorStubGenerator(_compiler, namer, backend);
 
     String holderName = namer.globalObjectFor(backend.interceptorsLibrary);
     Holder holder = _registry.registerHolder(holderName);
 
     List<String> names = backend.oneShotInterceptors.keys.toList()..sort();
@@ skipping 23 matching lines @@
       _registry.registerConstant(outputUnit, constantValue);
       assert(!_constants.containsKey(constantValue));
       String name = namer.constantName(constantValue);
       String constantObject = namer.globalObjectForConstant(constantValue);
       Holder holder = _registry.registerHolder(constantObject);
       Constant constant = new Constant(name, holder, constantValue);
       _constants[constantValue] = constant;
     }
   }
 }