[vm] Put covariance attributes into Field flags

alexmarkov · commit-bot@chromium.org · commit 469da726d6af · 2019-03-12T17:30:16.000Z
This change reduces number of places where we need to re-parse kernel field declarations in order to get covariance attributes. Change-Id: I7d33d3787726270b93d16eda59b09345749a38fb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/96407 Commit-Queue: Alexander Markov <alexmarkov@google.com> Auto-Submit: Alexander Markov <alexmarkov@google.com> Reviewed-by: Martin Kustermann <kustermann@google.com> Reviewed-by: Samir Jindel <sjindel@google.com>
diff --git a/runtime/vm/compiler/call_specializer.cc b/runtime/vm/compiler/call_specializer.cc
@@ -1017,14 +1017,10 @@ bool CallSpecializer::TryInlineInstanceSetter(InstanceCallInstr* instr,
     // not in strong mode or if at a dynamic invocation.
     bool needs_check = true;
     if (!instr->interface_target().IsNull() && (field.kernel_offset() >= 0)) {
-      bool is_covariant = false;
-      bool is_generic_covariant = false;
-      field.GetCovarianceAttributes(&is_covariant, &is_generic_covariant);
-
-      if (is_covariant) {
+      if (field.is_covariant()) {
         // Always type check covariant fields.
         needs_check = true;
-      } else if (is_generic_covariant) {
+      } else if (field.is_generic_covariant_impl()) {
         // If field is generic covariant then we don't need to check it
         // if the invocation was marked as unchecked (e.g. receiver of
         // the invocation is also the receiver of the surrounding method).
diff --git a/runtime/vm/compiler/frontend/scope_builder.cc b/runtime/vm/compiler/frontend/scope_builder.cc
@@ -290,12 +290,10 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() {
 
         if (is_method &&
             MethodCanSkipTypeChecksForNonCovariantArguments(function, attrs)) {
-          FieldHelper field_helper(&helper_);
-          field_helper.ReadUntilIncluding(FieldHelper::kFlags);
-
-          if (field_helper.IsCovariant()) {
+          const auto& field = Field::Handle(Z, function.accessor_field());
+          if (field.is_covariant()) {
             result_->setter_value->set_is_explicit_covariant_parameter();
-          } else if (!field_helper.IsGenericCovariantImpl() ||
+          } else if (!field.is_generic_covariant_impl() ||
                      (!attrs.has_non_this_uses && !attrs.has_tearoff_uses)) {
             result_->setter_value->set_type_check_mode(
                 LocalVariable::kTypeCheckedByCaller);
diff --git a/runtime/vm/kernel.cc b/runtime/vm/kernel.cc
@@ -618,10 +618,8 @@ bool NeedsDynamicInvocationForwarder(const Function& function) {
   // Handle setters.
   if (reader_helper.PeekTag() == kField) {
     ASSERT(function.IsImplicitSetterFunction());
-    FieldHelper field_helper(&reader_helper);
-    field_helper.ReadUntilIncluding(FieldHelper::kFlags);
-    return !(field_helper.IsCovariant() ||
-             field_helper.IsGenericCovariantImpl());
+    const auto& field = Field::Handle(zone, function.accessor_field());
+    return !(field.is_covariant() || field.is_generic_covariant_impl());
   }
 
   reader_helper.ReadUntilFunctionNode();
diff --git a/runtime/vm/kernel_loader.cc b/runtime/vm/kernel_loader.cc
@@ -1067,6 +1067,9 @@ void KernelLoader::FinishTopLevelClassLoading(
     // In the VM all const fields are implicitly final whereas in Kernel they
     // are not final because they are not explicitly declared that way.
     const bool is_final = field_helper.IsConst() || field_helper.IsFinal();
+    // Only instance fields could be covariant.
+    ASSERT(!field_helper.IsCovariant() &&
+           !field_helper.IsGenericCovariantImpl());
     const Field& field = Field::Handle(
         Z,
         Field::NewTopLevel(name, is_final, field_helper.IsConst(), script_class,
@@ -1490,6 +1493,9 @@ void KernelLoader::FinishClassLoading(const Class& klass,
                      field_helper.position_, field_helper.end_position_));
       field.set_kernel_offset(field_offset);
       field.set_has_pragma(has_pragma_annotation);
+      field.set_is_covariant(field_helper.IsCovariant());
+      field.set_is_generic_covariant_impl(
+          field_helper.IsGenericCovariantImpl());
       ReadInferredType(field, field_offset + library_kernel_offset_);
       CheckForInitializer(field);
       field_helper.ReadUntilExcluding(FieldHelper::kInitializer);
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
@@ -8279,27 +8279,6 @@ intptr_t Field::KernelDataProgramOffset() const {
   return PatchClass::Cast(obj).library_kernel_offset();
 }
 
-#if !defined(DART_PRECOMPILED_RUNTIME)
-void Field::GetCovarianceAttributes(bool* is_covariant,
-                                    bool* is_generic_covariant) const {
-  Thread* thread = Thread::Current();
-  Zone* zone = Thread::Current()->zone();
-  auto& script = Script::Handle(zone, Script());
-
-  kernel::TranslationHelper translation_helper(thread);
-  translation_helper.InitFromScript(script);
-
-  kernel::KernelReaderHelper kernel_reader_helper(
-      zone, &translation_helper, script,
-      ExternalTypedData::Handle(zone, KernelData()), KernelDataProgramOffset());
-  kernel_reader_helper.SetOffset(kernel_offset());
-  kernel::FieldHelper field_helper(&kernel_reader_helper);
-  field_helper.ReadUntilIncluding(kernel::FieldHelper::kFlags);
-  *is_covariant = field_helper.IsCovariant();
-  *is_generic_covariant = field_helper.IsGenericCovariantImpl();
-}
-#endif
-
 // Called at finalization time
 void Field::SetFieldType(const AbstractType& value) const {
   ASSERT(Thread::Current()->IsMutatorThread());
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
@@ -3055,6 +3055,21 @@ class Field : public Object {
     set_kind_bits(HasPragmaBit::update(value, raw_ptr()->kind_bits_));
   }
 
+  bool is_covariant() const {
+    return CovariantBit::decode(raw_ptr()->kind_bits_);
+  }
+  void set_is_covariant(bool value) const {
+    set_kind_bits(CovariantBit::update(value, raw_ptr()->kind_bits_));
+  }
+
+  bool is_generic_covariant_impl() const {
+    return GenericCovariantImplBit::decode(raw_ptr()->kind_bits_);
+  }
+  void set_is_generic_covariant_impl(bool value) const {
+    set_kind_bits(
+        GenericCovariantImplBit::update(value, raw_ptr()->kind_bits_));
+  }
+
   intptr_t kernel_offset() const {
 #if defined(DART_PRECOMPILED_RUNTIME)
     return 0;
@@ -3073,11 +3088,6 @@ class Field : public Object {
 
   intptr_t KernelDataProgramOffset() const;
 
-#if !defined(DART_PRECOMPILED_RUNTIME)
-  void GetCovarianceAttributes(bool* is_covariant,
-                               bool* is_generic_covariant) const;
-#endif
-
   inline intptr_t Offset() const;
   // Called during class finalization.
   inline void SetOffset(intptr_t offset_in_bytes) const;
@@ -3313,6 +3323,8 @@ class Field : public Object {
     kDoubleInitializedBit,
     kInitializerChangedAfterInitializatonBit,
     kHasPragmaBit,
+    kCovariantBit,
+    kGenericCovariantImplBit,
   };
   class ConstBit : public BitField<uint16_t, bool, kConstBit, 1> {};
   class StaticBit : public BitField<uint16_t, bool, kStaticBit, 1> {};
@@ -3330,6 +3342,9 @@ class Field : public Object {
                         kInitializerChangedAfterInitializatonBit,
                         1> {};
   class HasPragmaBit : public BitField<uint16_t, bool, kHasPragmaBit, 1> {};
+  class CovariantBit : public BitField<uint16_t, bool, kCovariantBit, 1> {};
+  class GenericCovariantImplBit
+      : public BitField<uint16_t, bool, kGenericCovariantImplBit, 1> {};
 
   // Update guarded cid and guarded length for this field. Returns true, if
   // deoptimization of dependent code is required.
