[vm/nnbd] Use InitStaticField instruction for late static fields

Issue: dart-lang/sdk#41299 Issue: dart-lang/sdk#41417 Issue: dart-lang/sdk#40796 Change-Id: Ibcc0ea55c6262ccafdda6c3fb1b28f357297c1de Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/143100 Reviewed-by: Martin Kustermann <kustermann@google.com> Reviewed-by: Liam Appelbe <liama@google.com> Benchmark: Vyacheslav Egorov <vegorov@google.com> Commit-Queue: Alexander Markov <alexmarkov@google.com>
goderbauer · Apr 15, 2020 · d5c38cd · d5c38cd
1 parent 188a6a1
commit d5c38cd
Show file tree

Hide file tree

Showing 10 changed files with 58 additions and 53 deletions.
diff --git a/runtime/vm/compiler/backend/il.cc b/runtime/vm/compiler/backend/il.cc
@@ -4069,9 +4069,12 @@ void InitStaticFieldInstr::EmitNativeCode(FlowGraphCompiler* compiler) {
 
   compiler::Label call_runtime, no_call;
   __ CompareObject(temp, Object::sentinel());
-  __ BranchIf(EQUAL, &call_runtime);
 
-  __ CompareObject(temp, Object::transition_sentinel());
+  if (!field().is_late()) {
+    __ BranchIf(EQUAL, &call_runtime);
+    __ CompareObject(temp, Object::transition_sentinel());
+  }
+
   __ BranchIf(NOT_EQUAL, &no_call);
 
   __ Bind(&call_runtime);

diff --git a/runtime/vm/compiler/backend/type_propagator.cc b/runtime/vm/compiler/backend/type_propagator.cc
@@ -1417,10 +1417,6 @@ CompileType LoadStaticFieldInstr::ComputeType() const {
     DEBUG_ASSERT(Isolate::Current()->HasAttemptedReload());
     return CompileType::Dynamic();
   }
-  if (field.is_late() && !is_initialized) {
-    // TODO(dartbug.com/40796): Extend CompileType to handle lateness.
-    is_nullable = CompileType::kNullable;
-  }
   return CompileType(is_nullable, cid, abstract_type);
 }
 

diff --git a/runtime/vm/compiler/frontend/bytecode_scope_builder.cc b/runtime/vm/compiler/frontend/bytecode_scope_builder.cc
@@ -117,12 +117,6 @@ void BytecodeScopeBuilder::BuildScopes() {
     }
     case RawFunction::kImplicitStaticGetter: {
       ASSERT(!IsStaticFieldGetterGeneratedAsInitializer(function, Z));
-      const Field& field = Field::Handle(Z, function.accessor_field());
-      const bool lib_is_nnbd = function.nnbd_mode() == NNBDMode::kOptedInLib;
-      if (field.is_late() || lib_is_nnbd) {
-        // LoadLateField uses expression_temp_var.
-        needs_expr_temp = true;
-      }
       break;
     }
     case RawFunction::kDynamicInvocationForwarder: {

diff --git a/runtime/vm/compiler/frontend/kernel_to_il.cc b/runtime/vm/compiler/frontend/kernel_to_il.cc
@@ -441,15 +441,11 @@ Fragment FlowGraphBuilder::LoadLateField(const Field& field,
   Fragment instructions;
   TargetEntryInstr *is_uninitialized, *is_initialized;
   const TokenPosition position = field.token_pos();
-  const bool is_static = field.is_static();
+  ASSERT(!field.is_static());
 
   // Check whether the field has been initialized already.
-  if (is_static) {
-    instructions += LoadStaticField(field);
-  } else {
-    instructions += LoadLocal(instance);
-    instructions += LoadField(field);
-  }
+  instructions += LoadLocal(instance);
+  instructions += LoadField(field);
 
   LocalVariable* temp = parsed_function_->expression_temp_var();
   instructions += StoreLocal(position, temp);
@@ -467,14 +463,9 @@ Fragment FlowGraphBuilder::LoadLateField(const Field& field,
     Function& init_function =
         Function::ZoneHandle(Z, field.EnsureInitializerFunction());
     Fragment initialize(is_uninitialized);
-    if (is_static) {
-      initialize += StaticCall(position, init_function,
-                               /* argument_count = */ 0, ICData::kNoRebind);
-    } else {
-      initialize += LoadLocal(instance);
-      initialize += StaticCall(position, init_function,
-                               /* argument_count = */ 1, ICData::kNoRebind);
-    }
+    initialize += LoadLocal(instance);
+    initialize += StaticCall(position, init_function,
+                             /* argument_count = */ 1, ICData::kNoRebind);
     initialize += StoreLocal(position, temp);
     initialize += Drop();
     initialize += StoreLateField(field, instance, temp);
@@ -2586,9 +2577,6 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfFieldAccessor(
     field = function.accessor_field();
   }
 
-  const Class& owner = Class::ZoneHandle(Z, field.Owner());
-  const bool lib_is_nnbd = Library::ZoneHandle(Z, owner.library()).is_nnbd();
-
   graph_entry_ =
       new (Z) GraphEntryInstr(*parsed_function_, Compiler::kNoOSRDeoptId);
 
@@ -2659,18 +2647,17 @@ FlowGraph* FlowGraphBuilder::BuildGraphOfFieldAccessor(
     ASSERT(!field.IsUninitialized());
     body += Constant(Instance::ZoneHandle(Z, field.StaticValue()));
   } else {
-    // The field always has an initializer because static fields without
-    // initializers are initialized eagerly and do not have implicit getters.
-    if (lib_is_nnbd) {
-      // In NNBD mode, static fields act like late fields regardless of whether
-      // they're marked late. The only behavioural difference is in compile
-      // errors that are handled by the front end.
-      body += LoadLateField(field, /* instance = */ nullptr);
-    } else {
-      ASSERT(field.has_nontrivial_initializer());
-      body += InitStaticField(field);
-      body += LoadStaticField(field);
-    }
+    // Static fields
+    //  - with trivial initializer
+    //  - without initializer if they are not late
+    // are initialized eagerly and do not have implicit getters.
+    // Static fields with non-trivial initializer need getter to perform
+    // lazy initialization. Late fields without initializer need getter
+    // to make sure they are already initialized.
+    ASSERT(field.has_nontrivial_initializer() ||
+           (field.is_late() && !field.has_initializer()));
+    body += InitStaticField(field);
+    body += LoadStaticField(field);
     if (field.needs_load_guard()) {
 #if defined(PRODUCT)
       UNREACHABLE();

diff --git a/runtime/vm/compiler/frontend/scope_builder.cc b/runtime/vm/compiler/frontend/scope_builder.cc
@@ -328,18 +328,12 @@ ScopeBuildingResult* ScopeBuilder::BuildScopes() {
     case RawFunction::kImplicitStaticGetter: {
       ASSERT(helper_.PeekTag() == kField);
       ASSERT(function.IsStaticFunction());
-      const auto& field = Field::Handle(Z, function.accessor_field());
       // In addition to static field initializers, scopes/local variables
       // are needed for implicit getters of static const fields, in order to
       // be able to evaluate their initializers in constant evaluator.
       if (Field::Handle(Z, function.accessor_field()).is_const()) {
         VisitNode();
       }
-      const bool lib_is_nnbd = function.nnbd_mode() == NNBDMode::kOptedInLib;
-      if (field.is_late() || lib_is_nnbd) {
-        // LoadLateField uses expression_temp_var.
-        needs_expr_temp_ = true;
-      }
       break;
     }
     case RawFunction::kFieldInitializer: {

diff --git a/runtime/vm/exceptions.cc b/runtime/vm/exceptions.cc
@@ -1120,6 +1120,12 @@ void Exceptions::ThrowCompileTimeError(const LanguageError& error) {
   Exceptions::ThrowByType(Exceptions::kCompileTimeError, args);
 }
 
+void Exceptions::ThrowLateInitializationError(const String& name) {
+  const Array& args = Array::Handle(Array::New(1));
+  args.SetAt(0, name);
+  Exceptions::ThrowByType(Exceptions::kLateInitializationError, args);
+}
+
 RawObject* Exceptions::Create(ExceptionType type, const Array& arguments) {
   Library& library = Library::Handle();
   const String* class_name = NULL;
@@ -1207,6 +1213,10 @@ RawObject* Exceptions::Create(ExceptionType type, const Array& arguments) {
       library = Library::CoreLibrary();
       class_name = &Symbols::_CompileTimeError();
       break;
+    case kLateInitializationError:
+      library = Library::CoreLibrary();
+      class_name = &Symbols::LateInitializationError();
+      break;
   }
 
   Thread* thread = Thread::Current();

diff --git a/runtime/vm/exceptions.h b/runtime/vm/exceptions.h
@@ -70,6 +70,7 @@ class Exceptions : AllStatic {
     kAbstractClassInstantiation,
     kCyclicInitializationError,
     kCompileTimeError,
+    kLateInitializationError,
   };
 
   DART_NORETURN static void ThrowByType(ExceptionType type,
@@ -85,6 +86,7 @@ class Exceptions : AllStatic {
                                             intptr_t expected_to);
   DART_NORETURN static void ThrowUnsupportedError(const char* msg);
   DART_NORETURN static void ThrowCompileTimeError(const LanguageError& error);
+  DART_NORETURN static void ThrowLateInitializationError(const String& name);
 
   // Returns a RawInstance if the exception is successfully created,
   // otherwise returns a RawError.

diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
@@ -9929,17 +9929,34 @@ RawError* Field::InitializeStatic() const {
   ASSERT(IsOriginal());
   ASSERT(is_static());
   if (StaticValue() == Object::sentinel().raw()) {
-    SetStaticValue(Object::transition_sentinel());
-    const Object& value = Object::Handle(EvaluateInitializer());
-    if (!value.IsNull() && value.IsError()) {
-      SetStaticValue(Object::null_instance());
-      return Error::Cast(value).raw();
+    auto& value = Object::Handle();
+    if (is_late()) {
+      if (!has_initializer()) {
+        Exceptions::ThrowLateInitializationError(String::Handle(name()));
+        UNREACHABLE();
+      }
+      value = EvaluateInitializer();
+      if (value.IsError()) {
+        return Error::Cast(value).raw();
+      }
+      if (is_final() && (StaticValue() != Object::sentinel().raw())) {
+        Exceptions::ThrowLateInitializationError(String::Handle(name()));
+        UNREACHABLE();
+      }
+    } else {
+      SetStaticValue(Object::transition_sentinel());
+      value = EvaluateInitializer();
+      if (value.IsError()) {
+        SetStaticValue(Object::null_instance());
+        return Error::Cast(value).raw();
+      }
     }
     ASSERT(value.IsNull() || value.IsInstance());
     SetStaticValue(value.IsNull() ? Instance::null_instance()
                                   : Instance::Cast(value));
     return Error::null();
   } else if (StaticValue() == Object::transition_sentinel().raw()) {
+    ASSERT(!is_late());
     const Array& ctor_args = Array::Handle(Array::New(1));
     const String& field_name = String::Handle(name());
     ctor_args.SetAt(0, field_name);

diff --git a/sdk/lib/_internal/vm/lib/errors_patch.dart b/sdk/lib/_internal/vm/lib/errors_patch.dart
@@ -598,6 +598,7 @@ class _DuplicatedFieldInitializerError extends Error {
 
 class _LateInitializationError extends Error
     implements LateInitializationError {
+  @pragma("vm:entry-point")
   _LateInitializationError(this._name);
 
   @pragma("vm:entry-point")

diff --git a/sdk_nnbd/lib/_internal/vm/lib/errors_patch.dart b/sdk_nnbd/lib/_internal/vm/lib/errors_patch.dart
@@ -596,6 +596,7 @@ class _DuplicatedFieldInitializerError extends Error {
 
 class _LateInitializationError extends Error
     implements LateInitializationError {
+  @pragma("vm:entry-point")
   _LateInitializationError(this._name);
 
   @pragma("vm:entry-point")