Change the type inference for fields in dart2js

lib/compiler/implementation/js_backend/backend.dart

 // Copyright (c) 2012, 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.
 
 typedef void Recompile(Element element);
 
 class ReturnInfo {
   HType returnType;
   List<Element> compiledFunctions;
 
@@ skipping 205 matching lines @@
       }
       index++;
     });
     return result;
   }
 
   String toString() =>
       allUnknown ? "HTypeList.ALL_UNKNOWN" : "HTypeList $types";
 }
 
+class FieldTypesRegistry {
+  final JavaScriptBackend backend;
+  final Map<Element, HType> fieldInitializerTypeMap;
+  final Map<Element, HType> fieldConstructorTypeMap;
+  final Map<Element, HType> fieldTypeMap;

[ngeoffray, 2012/09/20 14:43:01]

Why do you need three maps and not one?

[Søren Gjesse, 2012/09/21 13:49:44]

I need to separate the types set by the initializers and the types set by the
constructors to completely override the initializer type if a type is set in the
constructor. This is to avoid the XXX_OR_NULL type if the initializer just
initialize to null.

+  final Map<Element, FunctionSet> optimizedFunctions;
+
+  FieldTypesRegistry(JavaScriptBackend backend)
+      : fieldInitializerTypeMap = new Map<Element, HType>(),
+        fieldConstructorTypeMap = new Map<Element, HType>(),
+        fieldTypeMap = new Map<Element, HType>(),
+        optimizedFunctions = new Map<Element, FunctionSet>(),
+        this.backend = backend;
+
+  Compiler get compiler => backend.compiler;
+
+  void scheduleRecompilation(Element field) {
+    FunctionSet optimized = optimizedFunctions[field];
+    if (optimized != null) {
+      optimized.forEach(backend.scheduleForRecompilation);
+    }
+    optimizedFunctions.remove(field);
+  }
+
+  void registerFieldType(Map<Element, HType> typeMap,
+                         Element field,
+                         HType type) {
+    assert(field.isField());
+    HType oldType = typeMap[field];
+    HType newType;
+
+    if (oldType != null) {
+      newType = oldType.union(type);
+    } else {
+      newType = type;
+    }
+    typeMap[field] = newType;
+    if (oldType != newType) {
+      scheduleRecompilation(field);
+    }
+  }
+
+  void registerFieldInitializer(Element field, HType type) {
+    registerFieldType(fieldInitializerTypeMap, field, type);
+  }
+
+  void registerFieldConstructor(Element field, HType type) {
+    registerFieldType(fieldConstructorTypeMap, field, type);
+  }
+
+  void registerFieldSetter(HFieldSet node, HTypeMap types) {
+    Element field = node.element;
+    HType initializerType = fieldInitializerTypeMap[field];
+    HType constructorType = fieldConstructorTypeMap[field];
+    HType setterType = fieldTypeMap[field];
+    HType type = types[node.value];
+    if (type == HType.UNKNOWN &&
+        initializerType == null &&
+        constructorType == null &&
+        setterType == null) {

[ngeoffray, 2012/09/20 14:43:01]

Not sure what all these checks really mean. If the type is unknown shouldn't you
invalidate the type in the maps?

[Søren Gjesse, 2012/09/21 13:49:44]

This checks whether there is any type information for the field. If not then we
don't register the type as having no information leads to type UNKNOWN. If type
information is revealed later i the compilation process we will start
registering it.

Added comment.

+      return;
+    }
+    registerFieldType(fieldTypeMap, field, type);
+  }
+
+  void addedDynamicSetter(Selector setter) {
+    optimizedFunctions.getKeys().forEach((Element field) {

[ngeoffray, 2012/09/20 14:43:01]

I would do optimizedFunctions.forEach((Element field, _) { ...});

[Søren Gjesse, 2012/09/21 13:49:44]

Done.

+      if (field.name == setter.name) {
+        if (compiler.codegenWorld.hasInvokedSetter(field, compiler)) {
+          scheduleRecompilation(field);
+        }
+      }
+    });
+  }
+
+  HType optimisticFieldType(Element field) {
+    assert(field.isField());
+    assert(field.isMember());
+    if (compiler.codegenWorld.hasInvokedSetter(field, compiler)) {
+      return HType.UNKNOWN;
+    }
+    HType initializerType = fieldInitializerTypeMap[field];
+    HType constructorType = fieldConstructorTypeMap[field];
+    if (initializerType === null && constructorType === null) {
+      return HType.UNKNOWN;
+    }
+    HType result = constructorType != null ? constructorType : initializerType;
+    HType type = fieldTypeMap[field];
+    if (type !== null) result = result.union(type);
+    return result;
+  }
+
+  void registerOptimization(FunctionElement element,

[ngeoffray, 2012/09/20 14:43:01]

registerOptimization -> registerOptimizedFunction

[Søren Gjesse, 2012/09/21 13:49:44]

Done.

+                            Element field,
+                            HType type) {
+    assert(field.isField());
+    optimizedFunctions.putIfAbsent(

[ngeoffray, 2012/09/20 14:43:01]

FunctionSet set = optimizedFunctions...
set.add(element)

[Søren Gjesse, 2012/09/21 13:49:44]

This pattern is used in many places. Is it the performance penalty by making two
lookups in the Map you don't like?

+        field, () => new FunctionSet(backend.compiler));
+    optimizedFunctions[field].add(element);
+  }
+
+  void dump() {
+    optimizedFunctions.getKeys().forEach((Element field) {
+      print("Inferred $field has type ${optimisticFieldType(field)}");
+    });
+  }
+}
+
 class ArgumentTypesRegistry {
   final JavaScriptBackend backend;
   final Map<Element, HTypeList> staticTypeMap;
   final Set<Element> optimizedStaticFunctions;
   final SelectorMap<HTypeList> selectorTypeMap;
   final FunctionSet optimizedFunctions;
   final Map<Element, HTypeList> optimizedTypes;
   final Map<Element, OptionalParameterTypes> optimizedDefaultValueTypes;
 
   ArgumentTypesRegistry(JavaScriptBackend backend)
@@ skipping 178 matching lines @@
 
   final Namer namer;
 
   /**
    * Interface used to determine if an object has the JavaScript
    * indexing behavior. The interface is only visible to specific
    * libraries.
    */
   ClassElement jsIndexingBehaviorInterface;
 
-  final Map<Element, Map<Element, HType>> fieldInitializers;
-  final Map<Element, Map<Element, HType>> fieldConstructorSetters;
-  final Map<Element, Map<Element, HType>> fieldSettersType;
-
   final Map<Element, ReturnInfo> returnInfo;
 
   final List<Element> invalidateAfterCodegen;
   ArgumentTypesRegistry argumentTypes;
+  FieldTypesRegistry fieldTypes;
 
   List<CompilerTask> get tasks {
     return <CompilerTask>[builder, optimizer, generator, emitter];
   }
 
   JavaScriptBackend(Compiler compiler, bool generateSourceMap)
-      : fieldInitializers = new Map<Element, Map<Element, HType>>(),
-        fieldConstructorSetters = new Map<Element, Map<Element, HType>>(),
-        fieldSettersType = new Map<Element, Map<Element, HType>>(),
-        namer = new Namer(compiler),
+      : namer = new Namer(compiler),
         returnInfo = new Map<Element, ReturnInfo>(),
         invalidateAfterCodegen = new List<Element>(),
         super(compiler, constantSystem: JAVA_SCRIPT_CONSTANT_SYSTEM) {
     emitter = new CodeEmitterTask(compiler, namer, generateSourceMap);
     builder = new SsaBuilderTask(this);
     optimizer = new SsaOptimizerTask(this);
     generator = new SsaCodeGeneratorTask(this);
     argumentTypes = new ArgumentTypesRegistry(this);
+    fieldTypes = new FieldTypesRegistry(this);
   }
 
   Element get cyclicThrowHelper {
     return compiler.findHelper(const SourceString("throwCyclicInit"));
   }
 
   JavaScriptItemCompilationContext createItemCompilationContext() {
     return new JavaScriptItemCompilationContext();
   }
 
@@ skipping 45 matching lines @@
 
   void processNativeClasses(Enqueuer world,
                             Collection<LibraryElement> libraries) {
     native.processNativeClasses(world, emitter, libraries);
   }
 
   void assembleProgram() {
     emitter.assembleProgram();
   }
 
-  void updateFieldInitializers(Element field, HType propagatedType) {
-    assert(field.isField());
-    assert(field.isMember());
-    Map<Element, HType> fields =
-        fieldInitializers.putIfAbsent(
-          field.getEnclosingClass(), () => new Map<Element, HType>());
-    if (!fields.containsKey(field)) {
-      fields[field] = propagatedType;
-    } else {
-      fields[field] = fields[field].union(propagatedType);
-    }
-  }
-
-  HType typeFromInitializersSoFar(Element field) {
-    assert(field.isField());
-    assert(field.isMember());
-    if (!fieldInitializers.containsKey(field.getEnclosingClass())) {
-      return HType.CONFLICTING;
-    }
-    Map<Element, HType> fields = fieldInitializers[field.getEnclosingClass()];
-    return fields[field];
-  }
-
-  void updateFieldConstructorSetters(Element field, HType type) {
-    assert(field.isField());
-    assert(field.isMember());
-    Map<Element, HType> fields =
-        fieldConstructorSetters.putIfAbsent(
-            field.getEnclosingClass(), () => new Map<Element, HType>());
-    if (!fields.containsKey(field)) {
-      fields[field] = type;
-    } else {
-      fields[field] = fields[field].union(type);
-    }
-  }
-
-  // Check if this field is set in the constructor body.
-  bool hasConstructorBodyFieldSetter(Element field) {
-    ClassElement enclosingClass = field.getEnclosingClass();
-    if (!fieldConstructorSetters.containsKey(enclosingClass)) {
-      return false;
-    }
-    return fieldConstructorSetters[enclosingClass][field] != null;
-  }
-
-  // Provide an optimistic estimate of the type of a field after construction.
-  // If the constructor body has setters for fields returns HType.UNKNOWN.
-  // This only takes the initializer lists and field assignments in the
-  // constructor body into account. The constructor body might have method calls
-  // that could alter the field.
-  HType optimisticFieldTypeAfterConstruction(Element field) {
-    assert(field.isField());
-    assert(field.isMember());
-
-    ClassElement classElement = field.getEnclosingClass();
-    if (hasConstructorBodyFieldSetter(field)) {
-      // If there are field setters but there is only constructor then the type
-      // of the field is determined by the assignments in the constructor
-      // body.
-      var constructors = classElement.constructors;
-      if (constructors.head !== null && constructors.tail.isEmpty()) {
-        return fieldConstructorSetters[classElement][field];
-      } else {
-        return HType.UNKNOWN;
-      }
-    } else if (fieldInitializers.containsKey(classElement)) {
-      HType type = fieldInitializers[classElement][field];
-      return type == null ? HType.CONFLICTING : type;
-    } else {
-      return HType.CONFLICTING;
-    }
-  }
-
-  void updateFieldSetters(Element field, HType type) {
-    assert(field.isField());
-    assert(field.isMember());
-    Map<Element, HType> fields =
-        fieldSettersType.putIfAbsent(
-          field.getEnclosingClass(), () => new Map<Element, HType>());
-    if (!fields.containsKey(field)) {
-      fields[field] = type;
-    } else {
-      fields[field] = fields[field].union(type);
-    }
-  }
-
-  // Returns the type that field setters are setting the field to based on what
-  // have been seen during compilation so far.
-  HType fieldSettersTypeSoFar(Element field) {
-    assert(field.isField());
-    assert(field.isMember());
-    ClassElement enclosingClass = field.getEnclosingClass();
-    if (!fieldSettersType.containsKey(enclosingClass)) {
-      return HType.CONFLICTING;
-    }
-    Map<Element, HType> fields = fieldSettersType[enclosingClass];
-    if (!fields.containsKey(field)) return HType.CONFLICTING;
-    return fields[field];
-  }
-
   void scheduleForRecompilation(Element element) {
     if (compiler.phase == Compiler.PHASE_COMPILING) {
       invalidateAfterCodegen.add(element);
     }
   }
 
   /**
    *  Register a dynamic invocation and collect the provided types for the
    *  named selector.
    */
@@ skipping 40 matching lines @@
    */
   registerParameterTypesOptimization(
       FunctionElement element,
       HTypeList parameterTypes,
       OptionalParameterTypes defaultValueTypes) {
     if (element.parameterCount(compiler) == 0) return;
     argumentTypes.registerOptimization(
         element, parameterTypes, defaultValueTypes);
   }
 
+  registerFieldTypesOptimization(FunctionElement element,
+                                 Element field,
+                                 HType type) {
+    fieldTypes.registerOptimization(element, field, type);
+  }
+
   void registerReturnType(FunctionElement element, HType returnType) {
     ReturnInfo info = returnInfo[element];
     if (info != null) {
       info.update(returnType, scheduleForRecompilation);
     } else {
       returnInfo[element] = new ReturnInfo(returnType);
     }
   }
 
   /**
@@ skipping 14 matching lines @@
   }
 
   void dumpReturnTypes() {
     returnInfo.forEach((Element element, ReturnInfo info) {
       if (info.returnType != HType.UNKNOWN) {
         print("Inferred $element has return type ${info.returnType}");
       }
     });
   }
 
+  void registerFieldInitializer(Element field, HType type) {
+    fieldTypes.registerFieldInitializer(field, type);
+  }
+
+  void registerFieldConstructor(Element field, HType type) {
+    fieldTypes.registerFieldConstructor(field, type);
+  }
+
+  void registerFieldSetter(HFieldSet node, HTypeMap types) {
+    fieldTypes.registerFieldSetter(node, types);
+  }
+
+  void addedDynamicSetter(Selector setter) {
+    fieldTypes.addedDynamicSetter(setter);
+  }
+
+  HType optimisticFieldType(Element element) {
+    return fieldTypes.optimisticFieldType(element);
+  }
+
   SourceString getCheckedModeHelper(DartType type) {
     Element element = type.element;
     bool nativeCheck =
           emitter.nativeEmitter.requiresNativeIsCheck(element);
     if (element == compiler.stringClass) {
       return const SourceString('stringTypeCheck');
     } else if (element == compiler.doubleClass) {
       return const SourceString('doubleTypeCheck');
     } else if (element == compiler.numClass) {
       return const SourceString('numTypeCheck');
@@ skipping 19 matching lines @@
             ? const SourceString('listSuperNativeTypeCheck')
             : const SourceString('listSuperTypeCheck');
       } else {
         return nativeCheck
             ? const SourceString('callTypeCheck')
             : const SourceString('propertyTypeCheck');
       }
     }
   }
 }