[vm] Fix few issues with field exactness tracking.

- Don't add dependency on fields that are either unitialized or already marked
as non guarded (guarded cid is kIllegalCid in the first case and kDynamicCid
in the second case).
- Exit early from FieldStore if guarded_cid is kDynamicCid (or we are storing
null into a nullable field).
- Don't track exactness of fields which have function type type.

Also move logic for starting to track fields into class_finalizer because we
need resolved field types to make our decisions.

Change-Id: I164b42abe3108a70b0769f4c78d92459b4e78883
Reviewed-on: https://dart-review.googlesource.com/70620
Commit-Queue: Vyacheslav Egorov <[email protected]>
Reviewed-by: Martin Kustermann <[email protected]>

Author: mraleph

runtime/vm/class_finalizer.cc

@@ -1582,6 +1582,25 @@ void ClassFinalizer::FinalizeUpperBounds(const Class& cls,
   }
 }
 
+#if defined(TARGET_ARCH_X64)
+static bool IsPotentialExactGeneric(const AbstractType& type) {
+  // TODO(dartbug.com/34170) Investigate supporting this for fields with types
+  // that depend on type parameters of the enclosing class.
+  if (type.IsType() && !type.IsFunctionType() && !type.IsDartFunctionType() &&
+      type.IsInstantiated()) {
+    const Class& cls = Class::Handle(type.type_class());
+    return cls.IsGeneric();
+  }
+
+  return false;
+}
+#else
+// TODO(dartbug.com/34170) Support other architectures.
+static bool IsPotentialExactGeneric(const AbstractType& type) {
+  return false;
+}
+#endif
+
 void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
   // Note that getters and setters are explicitly listed as such in the list of
   // functions of a class, so we do not need to consider fields as implicitly
@@ -1605,6 +1624,7 @@ void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
   //   instance method.
 
   // Resolve type of fields and check for conflicts in super classes.
+  Isolate* isolate = Isolate::Current();
   Zone* zone = Thread::Current()->zone();
   Array& array = Array::Handle(zone, cls.fields());
   Field& field = Field::Handle(zone);
@@ -1614,11 +1634,16 @@ void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
   String& other_name = String::Handle(zone);
   Class& super_class = Class::Handle(zone);
   const intptr_t num_fields = array.Length();
+  const bool track_exactness = isolate->strong() && isolate->use_field_guards();
   for (intptr_t i = 0; i < num_fields; i++) {
     field ^= array.At(i);
     type = field.type();
     type = FinalizeType(cls, type);
     field.SetFieldType(type);
+    if (track_exactness && IsPotentialExactGeneric(type)) {
+      field.set_static_type_exactness_state(
+          StaticTypeExactnessState::Unitialized());
+    }
     name = field.name();
     if (field.is_static()) {
       getter_name = Field::GetterSymbol(name);
@@ -1676,7 +1701,7 @@ void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
           (!type.IsDynamicType() &&
            !const_value.IsInstanceOf(type, Object::null_type_arguments(),
                                      Object::null_type_arguments(), &error))) {
-        if (Isolate::Current()->error_on_bad_type()) {
+        if (isolate->error_on_bad_type()) {
           const AbstractType& const_value_type =
               AbstractType::Handle(zone, const_value.GetType(Heap::kNew));
           const String& const_value_type_name =
@@ -1718,7 +1743,6 @@ void ClassFinalizer::ResolveAndFinalizeMemberTypes(const Class& cls) {
   // If we check for bad overrides, collect interfaces, super interfaces, and
   // super classes of this class.
   GrowableArray<const Class*> interfaces(zone, 4);
-  Isolate* isolate = Isolate::Current();
   if (isolate->error_on_bad_override() && !isolate->strong()) {
     CollectInterfaces(cls, &interfaces);
     // Include superclasses in list of interfaces and super interfaces.

runtime/vm/compiler/backend/il.h

@@ -5164,6 +5164,7 @@ class LoadFieldInstr : public TemplateDefinition<1, NoThrow> {
         native_field_(nullptr),
         field_(field),
         token_pos_(token_pos) {
+    ASSERT(Class::Handle(field->Owner()).is_finalized());
     ASSERT(field->IsZoneHandle());
     // May be null if field is not an instance.
     ASSERT(type.IsZoneHandle() || type.IsReadOnlyHandle());

runtime/vm/kernel_loader.cc

@@ -1239,50 +1239,6 @@ Class& KernelLoader::LoadClass(const Library& library,
   return klass;
 }
 
-#if defined(TARGET_ARCH_X64)
-static bool ReferencesAnyTypeArguments(const AbstractType& type) {
-  if (type.IsTypeRef()) {
-    return false;
-  }
-
-  if (type.IsType()) {
-    const TypeArguments& args =
-        TypeArguments::Handle(Type::Cast(type).arguments());
-    AbstractType& arg = AbstractType::Handle();
-    for (intptr_t i = 0; i < args.Length(); i++) {
-      arg = args.TypeAt(i);
-      if (ReferencesAnyTypeArguments(arg)) {
-        return true;
-      }
-    }
-  }
-
-  return type.IsTypeParameter();
-}
-
-static bool IsPotentialExactGeneric(const AbstractType& type) {
-  if (type.IsType()) {
-    const TypeArguments& args =
-        TypeArguments::Handle(Type::Cast(type).arguments());
-    if (args.Length() == 0) {
-      return false;
-    }
-
-    // TODO(dartbug.com/34170) Investigate supporting this for
-    // fields with types that depend on type parameters
-    // of the enclosoing class.
-    return !ReferencesAnyTypeArguments(type);
-  }
-
-  return false;
-}
-#else
-// TODO(dartbug.com/34170) Support other architectures.
-static bool IsPotentialExactGeneric(const AbstractType& type) {
-  return false;
-}
-#endif
-
 void KernelLoader::FinishClassLoading(const Class& klass,
                                       const Library& library,
                                       const Class& toplevel_class,
@@ -1344,11 +1300,6 @@ void KernelLoader::FinishClassLoading(const Class& klass,
           Field::New(name, field_helper.IsStatic(), is_final,
                      field_helper.IsConst(), is_reflectable, script_class, type,
                      field_helper.position_, field_helper.end_position_));
-      if (I->strong() && I->use_field_guards() &&
-          IsPotentialExactGeneric(type)) {
-        field.set_static_type_exactness_state(
-            StaticTypeExactnessState::Unitialized());
-      }
       field.set_kernel_offset(field_offset);
       CheckForInitializer(field);
       field_helper.ReadUntilExcluding(FieldHelper::kInitializer);

runtime/vm/object.cc

@@ -9067,17 +9067,19 @@ const char* Field::GuardedPropertiesAsCString() const {
   }
 
   const char* exactness = "";
-  if (!static_type_exactness_state().IsExactOrUninitialized()) {
-    exactness = " {!exact}";
-  } else if (static_type_exactness_state().IsTriviallyExact()) {
-    exactness = " {trivially-exact}";
-  } else if (static_type_exactness_state().IsHasExactSuperType()) {
-    exactness = " {has-exact-super-type}";
-  } else if (static_type_exactness_state().IsHasExactSuperClass()) {
-    exactness = " {has-exact-super-class}";
-  } else {
-    ASSERT(static_type_exactness_state().IsUninitialized());
-    exactness = " {unknown exactness}";
+  if (static_type_exactness_state().IsTracking()) {
+    if (!static_type_exactness_state().IsExactOrUninitialized()) {
+      exactness = " {!exact}";
+    } else if (static_type_exactness_state().IsTriviallyExact()) {
+      exactness = " {trivially-exact}";
+    } else if (static_type_exactness_state().IsHasExactSuperType()) {
+      exactness = " {has-exact-super-type}";
+    } else if (static_type_exactness_state().IsHasExactSuperClass()) {
+      exactness = " {has-exact-super-class}";
+    } else {
+      ASSERT(static_type_exactness_state().IsUninitialized());
+      exactness = " {unknown exactness}";
+    }
   }
 
   const Class& cls =
@@ -9413,6 +9415,13 @@ void Field::RecordStore(const Object& value) const {
     return;
   }
 
+  if ((guarded_cid() == kDynamicCid) ||
+      (is_nullable() && value.raw() == Object::null())) {
+    // Nothing to do: the field is not guarded or we are storing null into
+    // a nullable field.
+    return;
+  }
+
   if (FLAG_trace_field_guards) {
     THR_Print("Store %s %s <- %s\n", ToCString(), GuardedPropertiesAsCString(),
               value.ToCString());

runtime/vm/parser.cc

@@ -214,9 +214,8 @@ ParsedFunction::ParsedFunction(Thread* thread, const Function& function)
 }
 
 void ParsedFunction::AddToGuardedFields(const Field* field) const {
-  if (((field->guarded_cid() == kDynamicCid) ||
-       (field->guarded_cid() == kIllegalCid)) &&
-      field->static_type_exactness_state().IsExactOrUninitialized()) {
+  if ((field->guarded_cid() == kDynamicCid) ||
+      (field->guarded_cid() == kIllegalCid)) {
     return;
   }