dart2js OldEmitter: inline FinishClasses.

pkg/compiler/lib/src/js_emitter/old_emitter/emitter.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.
 
 part of dart2js.js_emitter;
 
 
 class OldEmitter implements Emitter {
   final Compiler compiler;
   final CodeEmitterTask task;
 
   final ContainerBuilder containerBuilder = new ContainerBuilder();
   final ClassEmitter classEmitter = new ClassEmitter();
   final NsmEmitter nsmEmitter = new NsmEmitter();
   final TypeTestEmitter typeTestEmitter = new TypeTestEmitter();
   final InterceptorEmitter interceptorEmitter = new InterceptorEmitter();
   final MetadataEmitter metadataEmitter = new MetadataEmitter();
 
   final Set<ConstantValue> cachedEmittedConstants;
   final CodeBuffer cachedEmittedConstantsBuffer = new CodeBuffer();
   final Map<Element, ClassBuilder> cachedClassBuilders;
   final Set<Element> cachedElements;
 
-  bool needsDefineClass = false;
+  bool hasClasses = false;

[floitsch, 2014/12/29 18:00:22]

I prefer to stay consistent: needsClassSupport ?

[zarah, 2014/12/30 14:02:43]

Done.

   bool needsMixinSupport = false;
   bool needsLazyInitializer = false;
   final Namer namer;
   ConstantEmitter constantEmitter;
   NativeEmitter get nativeEmitter => task.nativeEmitter;
   TypeTestRegistry get typeTestRegistry => task.typeTestRegistry;
 
   // The full code that is written to each hunk part-file.
   Map<OutputUnit, CodeBuffer> outputBuffers = new Map<OutputUnit, CodeBuffer>();
   final CodeBuffer deferredConstants = new CodeBuffer();
@@ skipping 105 matching lines @@
   jsAst.Expression constantReference(ConstantValue value) {
     return constantEmitter.reference(value);
   }
 
   jsAst.Expression constantInitializerExpression(ConstantValue value) {
     return constantEmitter.initializationExpression(value);
   }
 
   String get name => 'CodeEmitter';
 
-  String get currentGenerateAccessorName
-      => '${namer.currentIsolate}.\$generateAccessor';
-  String get generateAccessorHolder
-      => '$isolatePropertiesName.\$generateAccessor';
-  String get finishClassesProperty
-      => r'$finishClasses';
-  String get finishClassesName
-      => '${namer.isolateName}.$finishClassesProperty';
   String get finishIsolateConstructorName
       => '${namer.isolateName}.\$finishIsolateConstructor';
   String get isolatePropertiesName
       => '${namer.isolateName}.${namer.isolatePropertiesName}';
   String get lazyInitializerProperty
       => r'$lazy';
   String get lazyInitializerName
       => '${namer.isolateName}.${lazyInitializerProperty}';
   String get initName => 'init';
   String get makeConstListProperty
@@ skipping 46 matching lines @@
   }
 
   jsAst.PropertyAccess classAccess(Element element) {
         return globalPropertyAccess(element);
   }
 
   jsAst.PropertyAccess typedefAccess(Element element) {
        return globalPropertyAccess(element);
   }
 
+  List<jsAst.Statement> buildTrivialNsmHandlers(){
+    return nsmEmitter.buildTrivialNsmHandlers();
+  }
+
   jsAst.FunctionDeclaration get generateAccessorFunction {
     const RANGE1_SIZE = RANGE1_LAST - RANGE1_FIRST + 1;
     const RANGE2_SIZE = RANGE2_LAST - RANGE2_FIRST + 1;
     const RANGE1_ADJUST = - (FIRST_FIELD_CODE - RANGE1_FIRST);
     const RANGE2_ADJUST = - (FIRST_FIELD_CODE + RANGE1_SIZE - RANGE2_FIRST);
     const RANGE3_ADJUST =
         - (FIRST_FIELD_CODE + RANGE1_SIZE + RANGE2_SIZE - RANGE3_FIRST);
 
     String receiverParamName = compiler.enableMinification ? "r" : "receiver";
     String valueParamName = compiler.enableMinification ? "v" : "value";
@@ skipping 103 matching lines @@
           if (typeof defineClass.name != "string") {
             str += name + ".name=\\"" + name + "\\";\\n";
           }
           if (#hasIsolateSupport) {
             str += name + ".$fieldNamesProperty=[" + fieldNames + "];\\n";
           }
           str += accessors.join("");
     
           return str;
         }''', { 'hasIsolateSupport': hasIsolateSupport });
-

[floitsch, 2014/12/29 18:00:21]

nit. I would keep this new line.

[zarah, 2014/12/30 14:02:43]

Done.

     // Declare a function called "generateAccessor".  This is used in
-    // defineClassFunction (it's a local declaration in init()).
+    // defineClassFunction.
     List result = <jsAst.Node>[
         generateAccessorFunction,
-        js('$generateAccessorHolder = generateAccessor'),
         new jsAst.FunctionDeclaration(
             new jsAst.VariableDeclaration('defineClass'), defineClass) ];
 
     if (compiler.hasIncrementalSupport) {
       result.add(
           js(r'#.defineClass = defineClass', [namer.accessIncrementalHelper]));
     }
 
     if (hasIsolateSupport) {
       jsAst.Expression classIdExtractorAccess =
@@ skipping 73 matching lines @@
           };
         }()
       ''');
     if (compiler.hasIncrementalSupport) {
       result = js(
           r'#.inheritFrom = #', [namer.accessIncrementalHelper, result]);
     }
     return js(r'var inheritFrom = #', [result]);
   }
 
-  /// Code that needs to be run before first invocation of
-  /// [finishClassesFunction], but should only be run once.
-  jsAst.Expression get initFinishClasses {
+  /// Code that needs to be run before processing reflection data,
+  /// but should only be run once.

[floitsch, 2014/12/29 18:00:21]

specify that the "once" refers to deferred libraries.

[zarah, 2014/12/30 14:02:43]

Done.

+  jsAst.Expression get initProcessStatics {

[floitsch, 2014/12/29 18:00:21]

was already a bad name: "initX" looks/sounds like a function that does the init.
Here it is a getter, and only provides the init-code.

Since it's really small and only used in one place (which is even smaller), just
inline it at use-site.

[zarah, 2014/12/30 14:02:43]

Done.

     jsAst.Expression allClassesAccess =
         generateEmbeddedGlobalAccess(embeddedNames.ALL_CLASSES);
     jsAst.Expression interceptorsByTagAccess =
         generateEmbeddedGlobalAccess(embeddedNames.INTERCEPTORS_BY_TAG);
     jsAst.Expression leafTagsAccess =
         generateEmbeddedGlobalAccess(embeddedNames.LEAF_TAGS);
     jsAst.Expression finishedClassesAccess =
         generateEmbeddedGlobalAccess(embeddedNames.FINISHED_CLASSES);
 
     return js('''
         (function(){

[floitsch, 2014/12/29 18:00:22]

I don't see any reason why there should be the anonymous function around the
inits.
But can/should be done in a different CL.

[zarah, 2014/12/30 14:02:43]

Acknowledged.

          # = Object.create(null);  // embedded allClasses.
          # = Object.create(null);  // embedded interceptorsByTag.
          # = Object.create(null);  // embedded leafTags.
          # = Object.create(null);  // embedded finishedClasses
         })()
       ''', [allClassesAccess,
             interceptorsByTagAccess,
             leafTagsAccess,
             finishedClassesAccess]);
   }
 
-  jsAst.Fun get finishClassesFunction {
-    // Class descriptions are collected in a JS object.
-    // 'finishClasses' takes all collected descriptions and sets up
-    // the prototype.
-    // Once set up, the constructors prototype field satisfy:
-    //  - it contains all (local) members.
-    //  - its internal prototype (__proto__) points to the superclass'
-    //    prototype field.
-    //  - the prototype's constructor field points to the JavaScript
-    //    constructor.
-    // For engines where we have access to the '__proto__' we can manipulate
-    // the object literal directly. For other engines we have to create a new
-    // object and copy over the members.
-
-    String reflectableField = namer.reflectableField;
-    jsAst.Expression allClassesAccess =
-        generateEmbeddedGlobalAccess(embeddedNames.ALL_CLASSES);
-    jsAst.Expression metadataAccess =
-        generateEmbeddedGlobalAccess(embeddedNames.METADATA);
-    String signaturePropertyName = namer.operatorSignature;
-
-    return js('''
-      function(collectedClasses, isolateProperties, existingIsolateProperties) {
-        var pendingClasses = Object.create(null);
-        var allClasses = #allClasses;
-        var constructors;
-
-        if (#debugFastObjects)
-          print("Number of classes: " +
-              Object.getOwnPropertyNames(\$\$).length);
-
-        var hasOwnProperty = Object.prototype.hasOwnProperty;
-
-        if (#inCspMode) {
-          constructors = dart_precompiled(collectedClasses);
-        }
-
-        if (#notInCspMode) {
-          var combinedConstructorFunction =
-             "function \$reflectable(fn){fn.$reflectableField=1;return fn};\\n"+
-             "var \$desc;\\n";
-          var constructorsList = [];
-        }
-
-        for (var cls in collectedClasses) {
-          var desc = collectedClasses[cls];
-          if (desc instanceof Array) desc = desc[1];
-
-          /* The 'fields' are either a constructor function or a
-           * string encoding fields, constructor and superclass. Gets the
-           * superclass and fields in the format
-           *   'Super;field1,field2'
-           * from the CLASS_DESCRIPTOR_PROPERTY property on the descriptor.
-           */
-          var classData = desc["${namer.classDescriptorProperty}"],
-              supr, fields = classData;
-          if (#hasRetainedMetadata)
-            if (typeof classData == "object" &&
-                classData instanceof Array) {
-              classData = fields = classData[0];
-            }
-          // ${ClassBuilder.fieldEncodingDescription}.
-          var s = fields.split(";");
-          fields = s[1] == "" ? [] : s[1].split(",");
-          supr = s[0];
-          // ${ClassBuilder.functionTypeEncodingDescription}.
-          split = supr.split(":");
-          if (split.length == 2) {
-            supr = split[0];
-            var functionSignature = split[1];
-            if (functionSignature)
-              desc.$signaturePropertyName = (function(s) {
-                  return function(){ return #metadata[s]; };
-                })(functionSignature);
-          }
-
-          if (#notInCspMode) {
-            combinedConstructorFunction += defineClass(cls, fields);
-            constructorsList.push(cls);
-          }
-          if (supr) pendingClasses[cls] = supr;
-        }
-
-        if (#notInCspMode) {
-          combinedConstructorFunction +=
-            "return [\\n  " + constructorsList.join(",\\n  ") + "\\n]";
-          var constructors =
-            new Function("\$collectedClasses", combinedConstructorFunction)
-            (collectedClasses);
-          combinedConstructorFunction = null;
-        }
-
-        for (var i = 0; i < constructors.length; i++) {
-          var constructor = constructors[i];
-          var cls = constructor.name;
-          var desc = collectedClasses[cls];
-          var globalObject = isolateProperties;
-          if (desc instanceof Array) {
-            globalObject = desc[0] || isolateProperties;
-            desc = desc[1];
-          }
-          if (#isTreeShakingDisabled)
-            constructor["${namer.metadataField}"] = desc;
-          allClasses[cls] = constructor;
-          globalObject[cls] = constructor;
-        }
-
-        constructors = null;
-
-        #finishClassFunction;
-
-        #trivialNsmHandlers;
-
-        for (var cls in pendingClasses) finishClass(cls);
-      }''', { 'allClasses': allClassesAccess,
-              'debugFastObjects': DEBUG_FAST_OBJECTS,
-              'hasRetainedMetadata': backend.hasRetainedMetadata,
-              'metadata': metadataAccess,
-              'isTreeShakingDisabled': backend.isTreeShakingDisabled,
-              'finishClassFunction': buildFinishClass(),
-              'trivialNsmHandlers': nsmEmitter.buildTrivialNsmHandlers(),
-              'inCspMode': compiler.useContentSecurityPolicy,
-              'notInCspMode': !compiler.useContentSecurityPolicy});
-  }
-
   jsAst.Node optional(bool condition, jsAst.Node node) {
     return condition ? node : new jsAst.EmptyStatement();
   }
 
   jsAst.Statement buildFinishClass() {
     String specProperty = '"${namer.nativeSpecProperty}"';  // "%"
 
     jsAst.Expression finishedClassesAccess =
         generateEmbeddedGlobalAccess(embeddedNames.FINISHED_CLASSES);
     jsAst.Expression interceptorsByTagAccess =
         generateEmbeddedGlobalAccess(embeddedNames.INTERCEPTORS_BY_TAG);
     jsAst.Expression leafTagsAccess =
         generateEmbeddedGlobalAccess(embeddedNames.LEAF_TAGS);
 
     return js.statement('''
     {
       var finishedClasses = #finishedClassesAccess;
 
       function finishClass(cls) {
 
         if (finishedClasses[cls]) return;
         finishedClasses[cls] = true;
 
-        var superclass = pendingClasses[cls];
+        var superclass = processedClasses.pending[cls];
 
         if (#needsMixinSupport) {
           if (superclass && superclass.indexOf("+") > 0) {
             var s = superclass.split("+");
             superclass = s[0];
             var mixinClass = s[1];
             finishClass(mixinClass);
             var mixin = allClasses[mixinClass];
             // TODO(21896): this test shouldn't be necessary. Without it
             // we have a crash in language/mixin_only_for_rti and
@@ skipping 120 matching lines @@
 
           // Now, after being a fast map we can set the lazies again.
           for (var lazyInit in lazies) {
             var lazyInitName = lazies[lazyInit];
             this[lazyInitName] = isolateProperties[lazyInitName];
           }
         }
         Isolate.prototype = oldIsolate.prototype;
         Isolate.prototype.constructor = Isolate;
         Isolate.#isolatePropertiesName = isolateProperties;
-        if (#needsDefineClass)
-          Isolate.#finishClassesProperty = oldIsolate.#finishClassesProperty;
         if (#outputContainsConstantList)
           Isolate.#makeConstListProperty = oldIsolate.#makeConstListProperty;
         if (#hasIncrementalSupport)
           Isolate.#lazyInitializerProperty =
               oldIsolate.#lazyInitializerProperty;
          return Isolate;
       }
 """,
         { 'isolatePropertiesName': namer.isolatePropertiesName,
-          'needsDefineClass': needsDefineClass,
-          'finishClassesProperty': finishClassesProperty,
           'outputContainsConstantList': task.outputContainsConstantList,
           'makeConstListProperty': makeConstListProperty,
           'hasIncrementalSupport': compiler.hasIncrementalSupport,
           'lazyInitializerProperty': lazyInitializerProperty,
         });
   }
 
   jsAst.Fun get lazyInitializerFunction {
     String isolate = namer.currentIsolate;
     jsAst.Expression cyclicThrow =
@@ skipping 33 matching lines @@
           } finally {
             $isolate[getterName] = function() { return this[fieldName]; };
           }
         }
       }
     ''', [laziesAccess, laziesAccess,
           laziesAccess,
           cyclicThrow]);
   }
 
-  List buildDefineClassAndFinishClassFunctionsIfNecessary() {
-    if (!needsDefineClass) return [];
-    return defineClassFunction
-        ..add(buildInheritFrom())
-        ..add(js('$finishClassesName = #', finishClassesFunction))
-        ..add(initFinishClasses);
-  }
-
   List buildLazyInitializerFunctionIfNecessary() {
     if (!needsLazyInitializer) return [];
 
     return [js('# = #', [js(lazyInitializerName), lazyInitializerFunction])];
   }
 
   List buildFinishIsolateConstructor() {
     return [
       js('$finishIsolateConstructorName = #', finishIsolateConstructorFunction)
     ];
@@ skipping 166 matching lines @@
         invariant(classElement, builder.functionType == null);
         invariant(classElement, builder.fieldMetadata == null);
         properties.properties.addAll(builder.properties);
       } else {
         classEmitter.generateClass(
             classElement, properties, additionalProperties[classElement]);
       }
     });
   }
 
-  void emitFinishClassesInvocationIfNecessary(CodeBuffer buffer) {
-    if (needsDefineClass) {
-      buffer.write('$finishClassesName($classesCollector,'
-                   '$_$isolateProperties,'
-                   '${_}null)$N');
-
-      // Reset the map.
-      buffer.write("$classesCollector$_=${_}null$N$n");
-    }
-  }
-
   void emitStaticFunctions(List<Element> staticFunctions) {
     for (Element element in staticFunctions) {
       ClassBuilder builder = new ClassBuilder(element, namer);
       containerBuilder.addMember(element, builder);
       getElementDescriptor(element).properties.addAll(builder.properties);
     }
   }
 
   void emitStaticNonFinalFieldInitializations(CodeBuffer buffer,
                                               OutputUnit outputUnit) {
@@ skipping 302 matching lines @@
     buffer.write(N);
     addComment('END invoke [main].', buffer);
   }
 
   void emitInitFunction(CodeBuffer buffer) {
     jsAst.FunctionDeclaration decl = js.statement('''
       function init() {
         $isolateProperties = Object.create(null);
         #; #; #;
       }''', [
-          buildDefineClassAndFinishClassFunctionsIfNecessary(),
+          initProcessStatics,
           buildLazyInitializerFunctionIfNecessary(),

[floitsch, 2014/12/29 18:00:21]

just inline all three functions.
Use "if (#) ..." to optionally build code.

[zarah, 2014/12/30 14:02:43]

Done.

           buildFinishIsolateConstructor()]);
 
     buffer.write(jsAst.prettyPrint(decl,
                  compiler, monitor: compiler.dumpInfoTask).getText());
     if (compiler.enableMinification) buffer.write('\n');
   }
 
   void emitConvertToFastObjectFunction() {
     List<jsAst.Statement> debugCode = <jsAst.Statement>[];
     if (DEBUG_FAST_OBJECTS) {
@@ skipping 302 matching lines @@
       mainBuffer
         .write('${globalsHolder}.${namer.isolateName}$_=$_'
                '${namer.isolateName}$N'
                '${globalsHolder}.$initName$_=${_}$initName$N');
     }
     mainBuffer.add('init()$N$n');
     mainBuffer.add('$isolateProperties$_=$_$isolatePropertiesName$N');
 
     emitStaticFunctions(task.outputStaticLists[mainOutputUnit]);
 
-    // Only output the classesCollector if we actually have any classes.
-    if (needsDefineClass ||
-        !(nativeClasses.isEmpty &&
-          compiler.codegenWorld.staticFunctionsNeedingGetter.isEmpty &&
-          outputClassLists.values.every((classList) => classList.isEmpty) &&
-          typedefsNeededForReflection.isEmpty)) {
-      // Shorten the code by using "$$" as temporary.
-      classesCollector = r"$$";
-      mainBuffer.add('var $classesCollector$_=${_}Object.create(null)$N$n');
-    }
-
     if (!nativeClasses.isEmpty) {
       addComment('Native classes', mainBuffer);
     }
 
     List<ClassElement> classes = task.outputClassLists[mainOutputUnit];
     if (classes != null) {
       for (ClassElement element in classes) {
         generateClass(element, getElementDescriptor(element));
       }
     }
@@ skipping 15 matching lines @@
       CodeBuffer libraryBuffer = new CodeBuffer();
       for (LibraryElement library in Elements.sortedByPosition(libraries)) {
         writeLibraryDescriptors(libraryBuffer, library);
         elementDescriptors.remove(library);
       }
 
       mainBuffer
           ..write('(')
           ..write(
               jsAst.prettyPrint(
-                  getReflectionDataParser(classesCollector, backend),
+                  getReflectionDataParser(this, backend),
                   compiler))
           ..write(')')
           ..write('([$n')
           ..add(libraryBuffer)
           ..write('])$N');
-
-      emitFinishClassesInvocationIfNecessary(mainBuffer);
     }
 
     interceptorEmitter.emitGetInterceptorMethods(mainBuffer);
     interceptorEmitter.emitOneShotInterceptors(mainBuffer);
 
     if (task.outputContainsConstantList) {
       emitMakeConstantList(mainBuffer);
     }
 
     // Constants in checked mode call into RTI code to set type information
@@ skipping 272 matching lines @@
     nativeEmitter.finishGenerateNativeClasses();
     nativeEmitter.assembleCode(nativeBuffer);
 
     return nativeBuffer;
   }
 
   void emitProgram(Program program) {
     // Shorten the code by using [namer.currentIsolate] as temporary.
     isolateProperties = namer.currentIsolate;
 
-    classesCollector = r"$$";
-
     // Emit deferred units first, so we have their hashes.
     // Map from OutputUnit to a hash of its content. The hash uniquely
     // identifies the code of the output-unit. It does not include
     // boilerplate JS code, like the sourcemap directives or the hash
     // itself.
     Map<OutputUnit, String> deferredLoadHashes = emitDeferredOutputUnits();
     CodeBuffer nativeBuffer = buildNativesBuffer();
     emitMainOutputUnit(deferredLoadHashes, nativeBuffer);
 
     if (backend.requiresPreamble &&
@@ skipping 128 matching lines @@
           ..write('var init$_=$_${globalsHolder}.init$N')
           ..write('var ${namer.isolateName}$_=$_'
                     '${globalsHolder}.${namer.isolateName}$N');
       if (libraryDescriptorBuffer != null) {
       // TODO(ahe): This defines a lot of properties on the
       // Isolate.prototype object.  We know this will turn it into a
       // slow object in V8, so instead we should do something similar
       // to Isolate.$finishIsolateConstructor.
          outputBuffer
            ..write('var ${namer.currentIsolate}$_=$_$isolatePropertiesName$N')
-           // The classesCollector object ($$).
-           ..write('$classesCollector$_=${_}Object.create(null);$n')
            ..write('(')
            ..write(
                jsAst.prettyPrint(
-                   getReflectionDataParser(classesCollector, backend),
+                   getReflectionDataParser(this, backend),
                    compiler, monitor: compiler.dumpInfoTask))
            ..write(')')
            ..write('([$n')
            ..addBuffer(libraryDescriptorBuffer)
            ..write('])$N');
 
-        if (outputClassLists.containsKey(outputUnit)) {
-          outputBuffer.write(
-              '$finishClassesName($classesCollector,$_${namer.currentIsolate},'
-              '$_$isolatePropertiesName)$N');
-        }
-
       }
 
       // Set the currentIsolate variable to the current isolate (which is
       // provided as second argument).
       // We need to do this, because we use the same variable for setting up
       // the isolate-properties and for storing the current isolate. During
       // the setup (the code above this lines) we must set the variable to
       // the isolate-properties.
-      // After we have done the setup (finishing with `finishClasses`) it must
-      // point to the current Isolate. Otherwise all methods/functions
-      // accessing isolate variables will access the wrong object.
+      // After we have done the setup it must point to the current Isolate.
+      // Otherwise all methods/functions accessing isolate variables will
+      // access the wrong object.
       outputBuffer.write("${namer.currentIsolate}$_=${_}arguments[1]$N");
 
       emitCompileTimeConstants(outputBuffer, outputUnit);
       emitStaticNonFinalFieldInitializations(outputBuffer, outputUnit);
       outputBuffer.write('}$N');
 
       if (compiler.useContentSecurityPolicy) {
         jsAst.FunctionDeclaration precompiledFunctionAst =
             buildCspPrecompiledFunctionFor(outputUnit);
 
@@ skipping 76 matching lines @@
     for (Element element in compiler.enqueuer.codegen.newlyEnqueuedElements) {
       if (element.isInstanceMember) {
         cachedClassBuilders.remove(element.enclosingClass);
 
         nativeEmitter.cachedBuilders.remove(element.enclosingClass);
 
       }
     }
   }
 }