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;
+  final Set<SourceString> selectorSet;
+  final Map<Element, Set<Element>> optimizedStaticFunctions;
+  final Map<Element, FunctionSet> optimizedFunctions;
+
+  FieldTypesRegistry(JavaScriptBackend backend)
+      : fieldInitializerTypeMap = new Map<Element, HType>(),
+        fieldConstructorTypeMap = new Map<Element, HType>(),
+        fieldTypeMap = new Map<Element, HType>(),
+        selectorSet = new Set<SourceString>(),
+        optimizedStaticFunctions = new Map<Element, Set<Element>>(),
+        optimizedFunctions = new Map<Element, FunctionSet>(),
+        this.backend = backend;
+
+  Compiler get compiler => backend.compiler;
+
+  void scheduleRecompilation(Element field) {
+    Set optimizedStatics = optimizedStaticFunctions[field];
+    if (optimizedStatics != null) {
+      optimizedStatics.forEach(backend.scheduleForRecompilation);
+    }
+    optimizedStaticFunctions.remove(field);

[ngeoffray, 2012/09/24 15:39:29]

Put the remove in the if?

[Søren Gjesse, 2012/09/25 13:33:21]

Done.

+    FunctionSet optimized = optimizedFunctions[field];
+    if (optimized != null) {
+      optimized.forEach(backend.scheduleForRecompilation);
+    }
+    optimizedFunctions.remove(field);

[ngeoffray, 2012/09/24 15:39:29]

ditto

[Søren Gjesse, 2012/09/25 13:33:21]

Done.

+  }
+
+  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(Element field, HType type) {
+    HType initializerType = fieldInitializerTypeMap[field];
+    HType constructorType = fieldConstructorTypeMap[field];
+    HType setterType = fieldTypeMap[field];
+    registerFieldType(fieldTypeMap, field, type);
+  }
+
+  void addedDynamicSetter(Selector setter, HType type) {
+    // TODO(sgjesse): Take the type of the setter into account.
+    assert(setter.isSetter());
+    if (selectorSet.contains(setter.name)) return;

[ngeoffray, 2012/09/24 15:39:29]

Please add comments on the field saying that this set contains the fields that
cannot be optimized. Maybe also find a better name for it?
fieldThatCannotBeUsedForOptimization work for me.

[Søren Gjesse, 2012/09/25 13:33:21]

Changed name to setterSelectorsUsed and added comment.

+    selectorSet.add(setter.name);
+    optimizedStaticFunctions.forEach((Element field, _) {
+      if (field.name == setter.name) {
+        scheduleRecompilation(field);
+      }
+    });
+    optimizedFunctions.forEach((Element field, _) {
+      if (field.name == setter.name) {
+        scheduleRecompilation(field);
+      }
+    });
+  }
+
+  HType optimisticFieldType(Element field) {
+    assert(field.isField());
+    assert(field.isMember());
+    if (selectorSet.contains(field.name)) {
+      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);

[ngeoffray, 2012/09/24 15:39:29]

You could avoid these checks and union if you compute the information after
visiting the constructor and constructor body. Thinking about it, why couldn't
you have just one map from element to type, that gets updated the further you go
in the analysis? (First the initializer, then the constructor, then the field
set).

[Søren Gjesse, 2012/09/25 13:33:21]

As far as I can see the generative constructor, generative constructor body and
methods/functions using member fields can be compiled in any order. Therefore it
seems quite difficult to avoid making this computation each time. I could keep
the final result in an additional map so the computation will only have to be
done when new information arrives.

+    return result;
+  }
+
+  void registerOptimizedFunction(FunctionElement element,
+                                 Element field,
+                                 HType type) {
+    assert(field.isField());
+    if (Elements.isStaticOrTopLevel(element)) {
+      optimizedStaticFunctions.putIfAbsent(
+          field, () => new Set<Element>());
+      optimizedStaticFunctions[field].add(element);
+    } else {
+      optimizedFunctions.putIfAbsent(
+          field, () => new FunctionSet(backend.compiler));
+      optimizedFunctions[field].add(element);
+    }
+  }
+
+  void dump() {
+    Set<Element> allFields = new Set<Element>();
+    fieldInitializerTypeMap.getKeys().forEach(allFields.add);
+    fieldConstructorTypeMap.getKeys().forEach(allFields.add);
+    fieldTypeMap.getKeys().forEach(allFields.add);
+    allFields.forEach((Element field) {
+      print("Inferred $field has type ${optimisticFieldType(field)}");
+    });
+  }
+}
+
 class ArgumentTypesRegistry {
   final JavaScriptBackend backend;
 
   /**
    * Documentation wanted -- johnniwinther
    *
    * Invariant: Keys must be declaration elements.
    */
   final Map<Element, HTypeList> staticTypeMap;
 
@@ skipping 142 matching lines @@
                                                   signature,
                                                   defaultValueTypes);
       }
       assert(types.allUnknown || types.length == signature.parameterCount);
       found = (found === null) ? types : found.union(types);
       return !found.allUnknown;
     });
     return found !== null ? found : HTypeList.ALL_UNKNOWN;
   }
 
-  void registerOptimization(Element element,
-                            HTypeList parameterTypes,
-                            OptionalParameterTypes defaultValueTypes) {
-    assert(invariant(element, element.isDeclaration));
+  void registerOptimizedFunction(Element element,
+                                 HTypeList parameterTypes,
+                                 OptionalParameterTypes defaultValueTypes) {
     if (Elements.isStaticOrTopLevelFunction(element)) {
       if (parameterTypes.allUnknown) {
         optimizedStaticFunctions.remove(element);
       } else {
         optimizedStaticFunctions.add(element);
       }
     }
 
     // TODO(kasperl): What kind of non-members do we get here?
     if (!element.isMember()) return;
@@ skipping 34 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;
 
   /**
    * Documentation wanted -- johnniwinther
    *
    * Invariant: Elements must be declaration elements.
    */
   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];
-  }
-
   /**
    * Documentation wanted -- johnniwinther
    *
    * Invariant: [element] must be a declaration element.
    */
   void scheduleForRecompilation(Element element) {
     assert(invariant(element, element.isDeclaration));
     if (compiler.phase == Compiler.PHASE_COMPILING) {
       invalidateAfterCodegen.add(element);
     }
@@ skipping 48 matching lines @@
    * scheduled for recompilation.
    *
    * Invariant: [element] must be a declaration element.
    */
   registerParameterTypesOptimization(
       FunctionElement element,
       HTypeList parameterTypes,
       OptionalParameterTypes defaultValueTypes) {
     assert(invariant(element, element.isDeclaration));
     if (element.parameterCount(compiler) == 0) return;
-    argumentTypes.registerOptimization(
+    argumentTypes.registerOptimizedFunction(
         element, parameterTypes, defaultValueTypes);
   }
 
+  registerFieldTypesOptimization(FunctionElement element,
+                                 Element field,
+                                 HType type) {
+    fieldTypes.registerOptimizedFunction(element, field, type);
+  }
+
   /**
    * Documentation wanted -- johnniwinther
    *
    * Invariant: [element] must be a declaration element.
    */
   void registerReturnType(FunctionElement element, HType returnType) {
     assert(invariant(element, element.isDeclaration));
     ReturnInfo info = returnInfo[element];
     if (info != null) {
       info.update(returnType, scheduleForRecompilation);
@@ skipping 24 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(Element field, HType type) {
+    fieldTypes.registerFieldSetter(field, type);
+  }
+
+  void addedDynamicSetter(Selector setter, HType type) {
+    fieldTypes.addedDynamicSetter(setter, type);
+  }
+
+  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');
       }
     }
   }
 }