[kernel] In strong mode, change covariant parameter types to Object in the

implicit closure of a method compiled by kernel (fixes #31305).

Change-Id: Iabeee2e382bf07ff78645054e453e0bd1afd02ab
Reviewed-on: https://dart-review.googlesource.com/24362
Commit-Queue: Régis Crelier <[email protected]>
Reviewed-by: Samir Jindel <[email protected]>

Author: crelier

runtime/vm/compiler/frontend/kernel_binary_flowgraph.cc

@@ -727,13 +727,12 @@ StreamingScopeBuilder::StreamingScopeBuilder(ParsedFunction* parsed_function)
       needs_expr_temp_(false),
       builder_(new StreamingFlowGraphBuilder(
           &translation_helper_,
-          parsed_function->function().script(),
+          Script::Handle(Z, parsed_function->function().script()),
           zone_,
           TypedData::Handle(Z, parsed_function->function().KernelData()),
           parsed_function->function().KernelDataProgramOffset())),
       type_translator_(builder_, /*finalize=*/true) {
-  Script& script = Script::Handle(Z, parsed_function->function().script());
-  H.InitFromScript(script);
+  H.InitFromScript(builder_->script());
   type_translator_.active_class_ = &active_class_;
 }
 
@@ -2506,7 +2505,7 @@ StreamingConstantEvaluator::StreamingConstantEvaluator(
       zone_(builder_->zone_),
       translation_helper_(builder_->translation_helper_),
       type_translator_(builder_->type_translator_),
-      script_(Script::Handle(zone_, builder_->Script())),
+      script_(builder_->script()),
       result_(Instance::Handle(zone_)) {}
 
 bool StreamingConstantEvaluator::IsCached(intptr_t offset) {
@@ -6442,8 +6441,7 @@ intptr_t StreamingFlowGraphBuilder::ArgumentCheckBitsForSetter(
       AlternativeReadingScope r(reader_, &kernel_data,
                                 target_field.kernel_offset());
       AlternativeScriptScope s(&translation_helper_,
-                               Script::Handle(target_field.Script()),
-                               Script::Handle(Script()));
+                               Script::Handle(target_field.Script()), script());
 
       FieldHelper helper(this);
       helper.ReadUntilIncluding(FieldHelper::kFlags);
@@ -6510,7 +6508,7 @@ void StreamingFlowGraphBuilder::ArgumentCheckBitsForInvocation(
                                  interface_target.kernel_offset());
       AlternativeScriptScope _s(&translation_helper_,
                                 Script::Handle(interface_target.script()),
-                                Script::Handle(Script()));
+                                script());
       ReadUntilFunctionNode();
 
       FunctionNodeHelper fn_helper(this);
@@ -8888,10 +8886,6 @@ Fragment StreamingFlowGraphBuilder::BuildFunctionNode(
   return instructions;
 }
 
-RawScript* StreamingFlowGraphBuilder::Script() {
-  return script_;
-}
-
 void StreamingFlowGraphBuilder::LoadAndSetupTypeParameters(
     ActiveClass* active_class,
     const Object& set_on,

runtime/vm/compiler/frontend/kernel_binary_flowgraph.h

@@ -106,6 +106,7 @@ class VariableDeclarationHelper {
   enum Flag {
     kFinal = 1 << 0,
     kConst = 1 << 1,
+    kCovariant = 1 << 3,
     kIsGenericCovariantImpl = 1 << 5,
     kIsGenericCovariantInterface = 1 << 6
   };
@@ -126,6 +127,7 @@ class VariableDeclarationHelper {
 
   bool IsConst() { return (flags_ & kConst) != 0; }
   bool IsFinal() { return (flags_ & kFinal) != 0; }
+  bool IsCovariant() { return (flags_ & kCovariant) != 0; }
 
   bool IsGenericCovariantInterface() {
     return (flags_ & kIsGenericCovariantInterface) != 0;
@@ -839,7 +841,7 @@ class StreamingConstantEvaluator {
   TranslationHelper& translation_helper_;
   StreamingDartTypeTranslator& type_translator_;
 
-  Script& script_;
+  const Script& script_;
   Instance& result_;
 };
 
@@ -852,7 +854,7 @@ class StreamingFlowGraphBuilder {
         translation_helper_(flow_graph_builder->translation_helper_),
         zone_(flow_graph_builder->zone_),
         reader_(new Reader(data)),
-        script_(parsed_function()->function().script()),
+        script_(Script::Handle(zone_, parsed_function()->function().script())),
         constant_evaluator_(this),
         type_translator_(this, /* finalize= */ true),
         data_program_offset_(data_program_offset),
@@ -872,7 +874,7 @@ class StreamingFlowGraphBuilder {
         translation_helper_(*translation_helper),
         zone_(zone),
         reader_(new Reader(data_buffer, buffer_length)),
-        script_(Script::null()),
+        script_(Script::Handle(zone_)),
         constant_evaluator_(this),
         type_translator_(this, /* finalize= */ true),
         data_program_offset_(data_program_offset),
@@ -884,7 +886,7 @@ class StreamingFlowGraphBuilder {
         metadata_scanned_(false) {}
 
   StreamingFlowGraphBuilder(TranslationHelper* translation_helper,
-                            RawScript* script,
+                            const Script& script,
                             Zone* zone,
                             const TypedData& data,
                             intptr_t data_program_offset)
@@ -921,6 +923,8 @@ class StreamingFlowGraphBuilder {
   String& GetSourceFor(intptr_t index);
   RawTypedData* GetLineStartsFor(intptr_t index);
   void SetOffset(intptr_t offset);
+  void ReadUntilFunctionNode();
+  intptr_t ReadListLength();
 
   enum DispatchCategory { Interface, ViaThis, Closure, DynamicDispatch };
 
@@ -934,7 +938,6 @@ class StreamingFlowGraphBuilder {
                                  const Function& function,
                                  Function* outermost_function);
 
-  void ReadUntilFunctionNode();
   StringIndex GetNameFromVariableDeclaration(intptr_t kernel_offset,
                                              const Function& function);
 
@@ -961,7 +964,6 @@ class StreamingFlowGraphBuilder {
   uint32_t ReadUInt32();
   uint32_t PeekUInt();
   uint32_t PeekListLength();
-  intptr_t ReadListLength();
   StringIndex ReadStringReference();
   NameIndex ReadCanonicalNameReference();
   StringIndex ReadNameAsStringIndex();
@@ -1256,7 +1258,7 @@ class StreamingFlowGraphBuilder {
       intptr_t* argument_bits,
       intptr_t* type_argument_bits);
 
-  RawScript* Script();
+  const Script& script() { return script_; }
 
   // Scan through metadata mappings section and cache offsets for recognized
   // metadata kinds.
@@ -1266,7 +1268,7 @@ class StreamingFlowGraphBuilder {
   TranslationHelper& translation_helper_;
   Zone* zone_;
   Reader* reader_;
-  RawScript* script_;
+  const Script& script_;
   StreamingConstantEvaluator constant_evaluator_;
   StreamingDartTypeTranslator type_translator_;
   // Some items like variables are specified in the kernel binary as

runtime/vm/compiler/frontend/kernel_to_il.cc

@@ -2492,7 +2492,7 @@ RawObject* EvaluateMetadata(const Field& metadata_field) {
     helper.InitFromScript(script);
 
     StreamingFlowGraphBuilder streaming_flow_graph_builder(
-        &helper, metadata_field.Script(), zone_,
+        &helper, Script::Handle(Z, metadata_field.Script()), Z,
         TypedData::Handle(Z, metadata_field.KernelData()),
         metadata_field.KernelDataProgramOffset());
     return streaming_flow_graph_builder.EvaluateMetadata(
@@ -2516,7 +2516,7 @@ RawObject* BuildParameterDescriptor(const Function& function) {
     helper.InitFromScript(script);
 
     StreamingFlowGraphBuilder streaming_flow_graph_builder(
-        &helper, function.script(), zone_,
+        &helper, Script::Handle(Z, function.script()), Z,
         TypedData::Handle(Z, function.KernelData()),
         function.KernelDataProgramOffset());
     return streaming_flow_graph_builder.BuildParameterDescriptor(
@@ -2582,7 +2582,7 @@ static void ProcessTokenPositionsEntry(
   }
 
   StreamingFlowGraphBuilder streaming_flow_graph_builder(
-      helper, script.raw(), zone_, data, data_kernel_offset);
+      helper, script, zone_, data, data_kernel_offset);
   streaming_flow_graph_builder.CollectTokenPositionsFor(
       script.kernel_script_index(), entry_script.kernel_script_index(),
       kernel_offset, token_positions, yield_positions);

runtime/vm/kernel.cc

@@ -19,7 +19,8 @@ bool FieldHasFunctionLiteralInitializer(const Field& field,
   TranslationHelper translation_helper(Thread::Current());
   translation_helper.InitFromScript(script);
 
-  StreamingFlowGraphBuilder builder(&translation_helper, field.Script(), zone,
+  StreamingFlowGraphBuilder builder(&translation_helper,
+                                    Script::Handle(zone, field.Script()), zone,
                                     TypedData::Handle(zone, field.KernelData()),
                                     field.KernelDataProgramOffset());
   builder.SetOffset(field.kernel_offset());

runtime/vm/kernel_loader.cc

@@ -347,7 +347,7 @@ KernelLoader::KernelLoader(const Script& script,
       kernel_program_info_(
           KernelProgramInfo::ZoneHandle(zone_, script.kernel_program_info())),
       translation_helper_(this, thread_),
-      builder_(&translation_helper_, script.raw(), zone_, kernel_data, 0),
+      builder_(&translation_helper_, script, zone_, kernel_data, 0),
       external_name_class_(Class::Handle(Z)),
       external_name_field_(Field::Handle(Z)),
       potential_natives_(GrowableObjectArray::Handle(Z)) {

runtime/vm/object.cc

@@ -14,6 +14,7 @@
 #include "vm/compiler/aot/precompiler.h"
 #include "vm/compiler/assembler/assembler.h"
 #include "vm/compiler/assembler/disassembler.h"
+#include "vm/compiler/frontend/kernel_binary_flowgraph.h"
 #include "vm/compiler/frontend/kernel_to_il.h"
 #include "vm/compiler/intrinsifier.h"
 #include "vm/compiler/jit/compiler.h"
@@ -82,7 +83,6 @@ DEFINE_FLAG(bool,
             "Remove script timestamps to allow for deterministic testing.");
 
 DECLARE_FLAG(bool, show_invisible_frames);
-DECLARE_FLAG(bool, strong);
 DECLARE_FLAG(bool, trace_deoptimization);
 DECLARE_FLAG(bool, trace_deoptimization_verbose);
 DECLARE_FLAG(bool, trace_reload);
@@ -6924,6 +6924,11 @@ RawFunction* Function::ImplicitClosureFunction() const {
   if (implicit_closure_function() != Function::null()) {
     return implicit_closure_function();
   }
+#if defined(DART_PRECOMPILED_RUNTIME)
+  // In AOT mode all implicit closures are pre-created.
+  UNREACHABLE();
+  return Function::null();
+#else
   ASSERT(!IsSignatureFunction() && !IsClosureFunction());
   Thread* thread = Thread::Current();
   Zone* zone = thread->zone();
@@ -6985,6 +6990,50 @@ RawFunction* Function::ImplicitClosureFunction() const {
   }
   closure_function.set_kernel_offset(kernel_offset());
 
+  // In strong mode, change covariant parameter types to Object in the implicit
+  // closure of a method compiled by kernel.
+  // The VM's parser erases covariant types immediately in strong mode.
+  if (thread->isolate()->strong() && !is_static() && (kernel_offset() > 0)) {
+    const Script& function_script = Script::Handle(zone, script());
+    kernel::TranslationHelper translation_helper(thread);
+    kernel::StreamingFlowGraphBuilder builder(
+        &translation_helper, function_script, zone,
+        TypedData::Handle(zone, KernelData()), KernelDataProgramOffset());
+    translation_helper.InitFromScript(function_script);
+    builder.SetOffset(kernel_offset());
+
+    builder.ReadUntilFunctionNode();
+    kernel::FunctionNodeHelper fn_helper(&builder);
+
+    // Check the positional parameters, including the optional positional ones.
+    fn_helper.ReadUntilExcluding(
+        kernel::FunctionNodeHelper::kPositionalParameters);
+    intptr_t num_pos_params = builder.ReadListLength();
+    ASSERT(num_pos_params ==
+           num_fixed_params - 1 + (has_opt_pos_params ? num_opt_params : 0));
+    const Type& object_type = Type::Handle(zone, Type::ObjectType());
+    for (intptr_t i = 0; i < num_pos_params; ++i) {
+      kernel::VariableDeclarationHelper var_helper(&builder);
+      var_helper.ReadUntilExcluding(kernel::VariableDeclarationHelper::kEnd);
+      if (var_helper.IsCovariant() || var_helper.IsGenericCovariantImpl()) {
+        closure_function.SetParameterTypeAt(i + 1, object_type);
+      }
+    }
+    fn_helper.SetJustRead(kernel::FunctionNodeHelper::kPositionalParameters);
+
+    // Check the optional named parameters.
+    fn_helper.ReadUntilExcluding(kernel::FunctionNodeHelper::kNamedParameters);
+    intptr_t num_named_params = builder.ReadListLength();
+    ASSERT(num_named_params == (has_opt_pos_params ? 0 : num_opt_params));
+    for (intptr_t i = 0; i < num_named_params; ++i) {
+      kernel::VariableDeclarationHelper var_helper(&builder);
+      var_helper.ReadUntilExcluding(kernel::VariableDeclarationHelper::kEnd);
+      if (var_helper.IsCovariant() || var_helper.IsGenericCovariantImpl()) {
+        closure_function.SetParameterTypeAt(num_pos_params + 1 + i,
+                                            object_type);
+      }
+    }
+  }
   const Type& signature_type =
       Type::Handle(zone, closure_function.SignatureType());
   if (!signature_type.IsFinalized()) {
@@ -6993,6 +7042,7 @@ RawFunction* Function::ImplicitClosureFunction() const {
   set_implicit_closure_function(closure_function);
   ASSERT(closure_function.IsImplicitClosureFunction());
   return closure_function.raw();
+#endif  // defined(DART_PRECOMPILED_RUNTIME)
 }
 
 void Function::DropUncompiledImplicitClosureFunction() const {
@@ -7066,7 +7116,7 @@ void Function::DropUncompiledImplicitClosureFunction() const {
 // and take the rest of the arguments from the invocation.
 //
 // Converted closure functions in VM follow same discipline as implicit closure
-// functions, because they are similar in many ways. For further deatils, please
+// functions, because they are similar in many ways. For further details, please
 // refer to the following methods:
 //   -> Function::ConvertedClosureFunction
 //   -> StreamingFlowGraphBuilder::BuildGraphOfConvertedClosureFunction
@@ -7077,9 +7127,6 @@ void Function::DropUncompiledImplicitClosureFunction() const {
 //
 // Function::ConvertedClosureFunction method follows the logic of
 // Function::ImplicitClosureFunction method.
-//
-// TODO(29181): Adjust the method to correctly pass type parameters after they
-// are enabled in closure conversion.
 RawFunction* Function::ConvertedClosureFunction() const {
   // Return the existing converted closure function if any.
   if (converted_closure_function() != Function::null()) {