Generate getters and setters dynamically.

lib/compiler/implementation/emitter.dart

Index: lib/compiler/implementation/emitter.dart
diff --git a/lib/compiler/implementation/emitter.dart b/lib/compiler/implementation/emitter.dart
index 707638d806acb2111531b2466e6eceefa36a1e6a..4c6bdc360fe944067df45e4ab41a09d139ab9dc0 100644
--- a/lib/compiler/implementation/emitter.dart
+++ b/lib/compiler/implementation/emitter.dart
@@ -53,6 +53,8 @@ class CodeEmitterTask extends CompilerTask {
       => '${namer.ISOLATE}.${namer.ISOLATE_PROPERTIES}';
 
   String get defineClassFunction() {
+    final String GETTER = "getter";
+    final String SETTER = "setter";
     // First the class name, then the super class name, followed by the fields
     // (in an array) and the members (inside an Object literal).
     // The caller can also pass in the constructor as a function if needed.
@@ -67,17 +69,35 @@ class CodeEmitterTask extends CompilerTask {
     //  },
     // });
     return """
-function(cls, superclass, fields, prototype) {
+function(cls, superclass, fields, fieldBitSet, prototype) {

[ngeoffray, 2012/05/09 08:50:28]

Not sure we need the two following lines. They kind of make it harder to
understand this blob.

[floitsch, 2012/05/09 13:58:36]

inlined.

+  ${namer.getDynamicFieldGetterNamerFunction(GETTER)}
+  ${namer.getDynamicFieldSetterNamerFunction(SETTER)}
   var constructor;
   if (typeof fields == 'function') {
     constructor = fields;
+    prototype = fieldBitSet;

[ngeoffray, 2012/05/09 08:50:28]

This is starting to be confusing. Maybe add another defineClass method for these
callers?

[floitsch, 2012/05/09 13:58:36]

removed.

   } else {
+    if (typeof fieldBitSet !== 'number') {
+      prototype = fieldBitSet;
+      fieldBitSet = 0;
+    } 
     var str = "(function " + cls + "(";
     var body = "";
     for (var i = 0; i < fields.length; i++) {

[kasperl, 2012/05/09 07:53:15]

Have you thought about encoding the need for a getter or a setter in the field
name instead? That would make it more understandable and still pretty compact.

   ["a=", "b?", "c?=", "d"]

You also don't risk running out of bits.

[floitsch, 2012/05/09 13:58:36]

Done.

       if (i != 0) str += ", ";
-      str += fields[i];
+      var field = fields[i];
+      str += field;
       body += "this." + fields[i] + " = " + fields[i] + ";\\n";

[kasperl, 2012/05/09 07:53:15]

Use field instead of fields[i].

[floitsch, 2012/05/09 13:58:36]

Done.

+      if (fieldBitSet & 1) {
+        var getterString = "return this." + field + ";";
+        prototype[$GETTER(field)] = new Function(getterString);
+      }
+      fieldBitSet >>>= 1;
+      if (fieldBitSet & 1) {
+        var setterString = "this." + field + " = v;";
+        prototype[$SETTER(field)] = new Function("v", setterString);
+      }
+      fieldBitSet >>>= 1;
     }
     str += ") {" + body + "})";
     constructor = eval(str);

[sra1, 2012/05/08 17:59:42]

Should this new new Function too?

[floitsch, 2012/05/09 13:58:36]

I would like it to be a "new Function", but then we would lose the name which is
important for debugging.

@@ -326,7 +346,26 @@ function() {
     }
   }
 
+  bool instanceFieldNeedsGetter(Element member) {
+    assert(member.kind === ElementKind.FIELD);
+    return compiler.universe.hasGetter(member, compiler);
+  }
+
+  bool instanceFieldNeedsSetter(Element member) {
+    assert(member.kind === ElementKind.FIELD);
+    return (member.modifiers === null || !member.modifiers.isFinal()) &&
+           compiler.universe.hasSetter(member, compiler);

[kasperl, 2012/05/09 07:53:15]

The indentation seems a bit off here.

[floitsch, 2012/05/09 13:58:36]

Done.

+  }
+
+  String compiledFieldName(Element member) {
+    assert(member.kind === ElementKind.FIELD);
+    return member.isNative()
+        ? member.name.slowToString()
+        : namer.getName(member);
+  }
+
   void addInstanceMember(Element member,
+                         Set<Element> dynamicallyEmittedFieldGettersSetters,
                          void defineInstanceMember(String invocationName,
                                                    String definition)) {
     // TODO(floitsch): we don't need to deal with members of
@@ -352,21 +391,19 @@ function() {
     } else if (member.kind === ElementKind.FIELD) {
       // TODO(ngeoffray): Have another class generate the code for the
       // fields.

[ngeoffray, 2012/05/09 08:50:28]

Remove the TODO.

[floitsch, 2012/05/09 13:58:36]

Done.

-      if ((member.modifiers === null || !member.modifiers.isFinal()) &&
-          compiler.universe.hasSetter(member, compiler)) {
-        String setterName = namer.setterName(member.getLibrary(), member.name);
-        String name = member.isNative()
-            ? member.name.slowToString()
-            : namer.getName(member);
-        defineInstanceMember(setterName, "function(v) { this.$name = v; }");
-      }
-      if (compiler.universe.hasGetter(member, compiler)) {
+      if (instanceFieldNeedsGetter(member) &&
+          !dynamicallyEmittedFieldGettersSetters.contains(member)) {
         String getterName = namer.getterName(member.getLibrary(), member.name);
-        String name = member.isNative()
-            ? member.name.slowToString()
-            : namer.getName(member);
+        String name = compiledFieldName(member);
         defineInstanceMember(getterName, "function() { return this.$name; }");
       }
+
+      if (instanceFieldNeedsSetter(member) &&
+          !dynamicallyEmittedFieldGettersSetters.contains(member)) {
+        String setterName = namer.setterName(member.getLibrary(), member.name);
+        String name = compiledFieldName(member);
+        defineInstanceMember(setterName, "function(v) { this.$name = v; }");
+      }
     } else {
       compiler.internalError('unexpected kind: "${member.kind}"',
                              element: member);
@@ -374,18 +411,6 @@ function() {
     emitExtraAccessors(member, defineInstanceMember);
   }
 
-  List<String> generateFieldList(ClassElement classElement) {
-    List<String> result = <String>[];
-    void addField(ClassElement enclosingClass, Element member) {
-      result.add(namer.instanceFieldName(member.getLibrary(), member.name));
-    }
-
-    classElement.forEachInstanceField(addField,
-                                      includeBackendMembers: true,
-                                      includeSuperMembers: true);
-    return result;
-  }
-
   void generateClass(ClassElement classElement, StringBuffer buffer) {
     needsDefineClass = true;
 
@@ -406,20 +431,76 @@ function() {
       superName = namer.getName(superclass);
     }
     String constructorName = namer.safeName(classElement.name.slowToString());
+
+    Set<Element> dynamicallyEmittedGettersSetters = new Set<Element>();
+
     buffer.add('$defineClassName("$className", "$superName", ');
+
     // If the class is never instantiated we still need to set it up for
     // inheritance purposes, but we can simplify its JavaScript constructor.
-    if (!compiler.universe.instantiatedClasses.contains(classElement)) {
-      buffer.add("[]");
-    } else {
-      List<String> fields = generateFieldList(classElement);
-      buffer.add('[');
-      for (int i = 0; i < fields.length; i++) {
-        if (i != 0) buffer.add(", ");
-        buffer.add('"${fields[i]}"');
+    bool isInstantiated =
+        compiler.universe.instantiatedClasses.contains(classElement);
+
+    // We encode 15 field getters and setters in an integer bit field. This way
+    // the integer will fit into a JavaScript Smi.
+    // The getters and setters for the marked fields will be generated
+    // dynamically.
+    final int MAX_DYNAMICALLY_GENERATED_FIELD_GETTERS_SETTERS = 15;
+    int fieldBits = 0;
+    int fieldCounter = 0;
+    buffer.add('[');
+
+    void addField(ClassElement enclosingClass, Element member) {
+      assert(!member.isNative());
+      LibraryElement library = member.getLibrary();
+      SourceString name = member.name;
+      String fieldName = namer.instanceFieldName(library, name);
+      // See, if we can dynamically create getters and setters.

[kasperl, 2012/05/09 07:53:15]

Remove , after See.

[floitsch, 2012/05/09 13:58:36]

Done.

+      // We can only generate getters and setters for [classElement] since
+      // the fields of super classes could be overwritten with getters or
+      // setters.
+      bool needsDynamicGetter = false;
+      bool needsDynamicSetter = false;
+      if (fieldCounter <= MAX_DYNAMICALLY_GENERATED_FIELD_GETTERS_SETTERS &&
+          enclosingClass === classElement) {
+        needsDynamicGetter = instanceFieldNeedsGetter(member);
+        needsDynamicSetter = instanceFieldNeedsSetter(member);
+        // Make sure that the name we would generate dynamically matches the
+        // name we assign to the getter/setter during compilation time.
+        if ((needsDynamicGetter && namer.getterName(library, name) !=

[kasperl, 2012/05/09 07:53:15]

Maybe add a helper on namer for this check (takes library, name, and fieldName
as parameters) to make this check easier to digest.

[floitsch, 2012/05/09 13:58:36]

code removed.

+                                   namer.dynamicGetterName(fieldName))
+            || (needsDynamicSetter && namer.setterName(library, name) !=
+                                      namer.dynamicSetterName(fieldName))) {
+          needsDynamicGetter = false;
+          needsDynamicSetter = false;
+        }
+      }
+
+      if (isInstantiated || needsDynamicGetter || needsDynamicSetter) {
+        fieldCounter++;
+        if (fieldCounter != 1) buffer.add(", ");
+        buffer.add('"$fieldName"');
+        if (needsDynamicGetter || needsDynamicSetter) {
+          dynamicallyEmittedGettersSetters.add(member);
+          if (needsDynamicGetter) {
+            fieldBits |= 1 << ((fieldCounter - 1) * 2);
+          }
+          if (needsDynamicSetter) {
+            fieldBits |= 1 << ((fieldCounter - 1) * 2 + 1);
+          }
+        }
       }
-      buffer.add(']');
     }
+
+    // If a class is not instantiated then we add the field just so we can
+    // generate the field getter/setter dynamically. Since this is only
+    // allowed on fields that are in [classElement] we don't need to visit
+    // superclasses for non-instantiated classes.
+    classElement.forEachInstanceField(addField,
+                                      includeBackendMembers: true,
+                                      includeSuperMembers: isInstantiated);
+    buffer.add(']');
+    if (fieldBits != 0) buffer.add(', $fieldBits');
     buffer.add(', {\n');
 
     void defineInstanceMember(String name, String value) {
@@ -429,7 +510,8 @@ function() {
     classElement.forEachMember(includeBackendMembers: true,
                                f: (ClassElement enclosing, Element member) {
       if (member.isInstanceMember()) {
-        addInstanceMember(member, defineInstanceMember);
+      addInstanceMember(
+          member, dynamicallyEmittedGettersSetters, defineInstanceMember);
       }
     });