[vm/bytecode] Fix writing and reading of nested function types in bytecode

alexmarkov · commit-bot@chromium.org · commit 96906936b418 · 2019-07-12T00:15:32.000Z
Fixes #37486 Change-Id: Id6b10288cf9afeddb8a9972eb1dd455184b15789 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108740 Reviewed-by: Régis Crelier <regis@google.com> Reviewed-by: Ryan Macnak <rmacnak@google.com> Commit-Queue: Alexander Markov <alexmarkov@google.com>
diff --git a/pkg/vm/lib/bytecode/object_table.dart b/pkg/vm/lib/bytecode/object_table.dart
@@ -2012,11 +2012,10 @@ class _NodeVisitor extends Visitor<ObjectHandle> {
 
   @override
   ObjectHandle visitFunctionType(FunctionType node) {
-    final typeParameters = new List<_TypeParameterHandle>.generate(
-        node.typeParameters.length,
-        (i) => objectTable.getOrAddObject(new _TypeParameterHandle(null, i)));
+    final int numEnclosingTypeParameters = _typeParameters.length;
     for (int i = 0; i < node.typeParameters.length; ++i) {
-      _typeParameters[node.typeParameters[i]] = typeParameters[i];
+      _typeParameters[node.typeParameters[i]] = objectTable.getOrAddObject(
+          new _TypeParameterHandle(null, numEnclosingTypeParameters + i));
     }
 
     final positionalParams = new List<_TypeHandle>();
diff --git a/runtime/vm/compiler/frontend/bytecode_reader.cc b/runtime/vm/compiler/frontend/bytecode_reader.cc
@@ -201,6 +201,10 @@ void BytecodeReaderHelper::ReadCode(const Function& function,
   ASSERT(code_offset > 0);
 
   AlternativeReadingScope alt(&reader_, code_offset);
+  // This scope is needed to set active_class_->enclosing_ which is used to
+  // assign parent function for function types.
+  ActiveEnclosingFunctionScope active_enclosing_function(active_class_,
+                                                         &function);
 
   const intptr_t flags = reader_.ReadUInt();
   const bool has_exceptions_table =
@@ -251,7 +255,7 @@ void BytecodeReaderHelper::ReadCode(const Function& function,
     // TODO(alexmarkov): allocate deopt_ids for closures separately
     DeoptIdScope deopt_id_scope(thread_, 0);
 
-    ReadConstantPool(function, pool);
+    ReadConstantPool(function, pool, 0);
   }
 
   // Read bytecode and attach to function.
@@ -653,7 +657,17 @@ void BytecodeReaderHelper::ReadTypeParametersDeclaration(
     // Do not set type parameters for factories, as VM uses class type
     // parameters instead.
     parameterized_function.set_type_parameters(type_parameters);
-    function_type_type_parameters_ = &type_parameters;
+    if (parameterized_function.IsSignatureFunction()) {
+      if (function_type_type_parameters_ == nullptr) {
+        function_type_type_parameters_ = &type_parameters;
+      } else {
+        function_type_type_parameters_ = &TypeArguments::Handle(
+            Z, function_type_type_parameters_->ConcatenateTypeParameters(
+                   Z, type_parameters));
+      }
+    } else {
+      ASSERT(function_type_type_parameters_ == nullptr);
+    }
   }
 
   // Step b) Fill in the bounds of all [TypeParameter]s.
@@ -664,8 +678,9 @@ void BytecodeReaderHelper::ReadTypeParametersDeclaration(
   }
 }
 
-void BytecodeReaderHelper::ReadConstantPool(const Function& function,
-                                            const ObjectPool& pool) {
+intptr_t BytecodeReaderHelper::ReadConstantPool(const Function& function,
+                                                const ObjectPool& pool,
+                                                intptr_t start_index) {
   TIMELINE_DURATION(Thread::Current(), CompilerVerbose,
                     "BytecodeReaderHelper::ReadConstantPool");
 
@@ -721,7 +736,7 @@ void BytecodeReaderHelper::ReadConstantPool(const Function& function,
   String& name = String::Handle(Z);
   const String* simpleInstanceOf = nullptr;
   const intptr_t obj_count = pool.Length();
-  for (intptr_t i = 0; i < obj_count; ++i) {
+  for (intptr_t i = start_index; i < obj_count; ++i) {
     const intptr_t tag = reader_.ReadTag();
     switch (tag) {
       case ConstantPoolTag::kInvalid:
@@ -799,11 +814,30 @@ void BytecodeReaderHelper::ReadConstantPool(const Function& function,
         intptr_t closure_index = reader_.ReadUInt();
         obj = closures_->At(closure_index);
         ASSERT(obj.IsFunction());
-      } break;
+        // Set current entry.
+        pool.SetTypeAt(i, ObjectPool::EntryType::kTaggedObject,
+                       ObjectPool::Patchability::kNotPatchable);
+        pool.SetObjectAt(i, obj);
+
+        // This scope is needed to set active_class_->enclosing_ which is used
+        // to assign parent function for function types.
+        ActiveEnclosingFunctionScope active_enclosing_function(
+            active_class_, &Function::Cast(obj));
+
+        // Read constant pool until corresponding EndClosureFunctionScope.
+        i = ReadConstantPool(function, pool, i + 1);
+
+        // Proceed with the rest of entries.
+        continue;
+      }
       case ConstantPoolTag::kEndClosureFunctionScope: {
-        // Entry is not used and set to null.
+        // EndClosureFunctionScope entry is not used and set to null.
         obj = Object::null();
-      } break;
+        pool.SetTypeAt(i, ObjectPool::EntryType::kTaggedObject,
+                       ObjectPool::Patchability::kNotPatchable);
+        pool.SetObjectAt(i, obj);
+        return i;
+      }
       case ConstantPoolTag::kNativeEntry: {
         name = ReadString();
         obj = NativeEntry(function, name);
@@ -854,6 +888,8 @@ void BytecodeReaderHelper::ReadConstantPool(const Function& function,
                    ObjectPool::Patchability::kNotPatchable);
     pool.SetObjectAt(i, obj);
   }
+
+  return obj_count - 1;
 }
 
 RawBytecode* BytecodeReaderHelper::ReadBytecode(const ObjectPool& pool) {
@@ -1405,6 +1441,11 @@ RawObject* BytecodeReaderHelper::ReadObjectContents(uint32_t header) {
                                                 : Function::null_function(),
                                             TokenPosition::kNoSource));
 
+      // This scope is needed to set active_class_->enclosing_ which is used
+      // to assign parent function for function types.
+      ActiveEnclosingFunctionScope active_enclosing_function(
+          active_class_, &signature_function);
+
       return ReadFunctionSignature(
           signature_function, (flags & kFlagHasOptionalPositionalParams) != 0,
           (flags & kFlagHasOptionalNamedParams) != 0,
@@ -1710,6 +1751,12 @@ RawObject* BytecodeReaderHelper::ReadType(intptr_t tag) {
                                                 ? *active_class_->enclosing
                                                 : Function::null_function(),
                                             TokenPosition::kNoSource));
+
+      // This scope is needed to set active_class_->enclosing_ which is used to
+      // assign parent function for function types.
+      ActiveEnclosingFunctionScope active_enclosing_function(
+          active_class_, &signature_function);
+
       // TODO(alexmarkov): skip type finalization
       return ReadFunctionSignature(
           signature_function, (flags & kFlagHasOptionalPositionalParams) != 0,
@@ -3014,8 +3061,6 @@ RawError* BytecodeReader::ReadFunctionBytecode(Thread* thread,
 
         ActiveClassScope active_class_scope(&active_class, &klass);
         ActiveMemberScope active_member(&active_class, &outermost_function);
-        ActiveTypeParametersScope active_type_params(&active_class, function,
-                                                     zone);
 
         BytecodeComponentData bytecode_component(
             &Array::Handle(zone, translation_helper.GetBytecodeComponent()));
diff --git a/runtime/vm/compiler/frontend/bytecode_reader.h b/runtime/vm/compiler/frontend/bytecode_reader.h
@@ -183,7 +183,13 @@ class BytecodeReaderHelper : public ValueObject {
   void ReadTypeParametersDeclaration(const Class& parameterized_class,
                                      const Function& parameterized_function);
 
-  void ReadConstantPool(const Function& function, const ObjectPool& pool);
+  // Read portion of constant pool corresponding to one function/closure.
+  // Start with [start_index], and stop when reaching EndClosureFunctionScope.
+  // Return index of the last read constant pool entry.
+  intptr_t ReadConstantPool(const Function& function,
+                            const ObjectPool& pool,
+                            intptr_t start_index);
+
   RawBytecode* ReadBytecode(const ObjectPool& pool);
   void ReadExceptionsTable(const Bytecode& bytecode, bool has_exceptions_table);
   void ReadSourcePositions(const Bytecode& bytecode, bool has_source_positions);
diff --git a/runtime/vm/compiler/frontend/kernel_translation_helper.h b/runtime/vm/compiler/frontend/kernel_translation_helper.h
@@ -1199,6 +1199,23 @@ class ActiveMemberScope {
   DISALLOW_COPY_AND_ASSIGN(ActiveMemberScope);
 };
 
+class ActiveEnclosingFunctionScope {
+ public:
+  ActiveEnclosingFunctionScope(ActiveClass* active_class,
+                               const Function* enclosing)
+      : active_class_(active_class), saved_(*active_class) {
+    active_class_->enclosing = enclosing;
+  }
+
+  ~ActiveEnclosingFunctionScope() { *active_class_ = saved_; }
+
+ private:
+  ActiveClass* active_class_;
+  ActiveClass saved_;
+
+  DISALLOW_COPY_AND_ASSIGN(ActiveEnclosingFunctionScope);
+};
+
 class ActiveTypeParametersScope {
  public:
   // Set the local type parameters of the ActiveClass to be exactly all type
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
@@ -4957,6 +4957,26 @@ RawTypeArguments* TypeArguments::Prepend(Zone* zone,
   return result.Canonicalize();
 }
 
+RawTypeArguments* TypeArguments::ConcatenateTypeParameters(
+    Zone* zone,
+    const TypeArguments& other) const {
+  ASSERT(!IsNull() && !other.IsNull());
+  const intptr_t this_len = Length();
+  const intptr_t other_len = other.Length();
+  const auto& result = TypeArguments::Handle(
+      zone, TypeArguments::New(this_len + other_len, Heap::kNew));
+  auto& type = AbstractType::Handle(zone);
+  for (intptr_t i = 0; i < this_len; ++i) {
+    type = TypeAt(i);
+    result.SetTypeAt(i, type);
+  }
+  for (intptr_t i = 0; i < other_len; ++i) {
+    type = other.TypeAt(i);
+    result.SetTypeAt(this_len + i, type);
+  }
+  return result.raw();
+}
+
 RawString* TypeArguments::SubvectorName(intptr_t from_index,
                                         intptr_t len,
                                         NameVisibility name_visibility) const {
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
@@ -6536,6 +6536,10 @@ class TypeArguments : public Instance {
                             intptr_t other_length,
                             intptr_t total_length) const;
 
+  // Concatenate [this] and [other] vectors of type parameters.
+  RawTypeArguments* ConcatenateTypeParameters(Zone* zone,
+                                              const TypeArguments& other) const;
+
   // Check if the subvector of length 'len' starting at 'from_index' of this
   // type argument vector consists solely of DynamicType, ObjectType, or
   // VoidType.
diff --git a/tests/corelib_2/corelib_2.status b/tests/corelib_2/corelib_2.status
@@ -104,6 +104,7 @@ apply_generic_function_test: SkipByDesign # Function.apply with named args
 apply_test: Skip # Uses new Symbol via symbolMapToStringMap helper
 dynamic_nosuchmethod_test: SkipByDesign # Expects names in NSM
 error_stack_trace1_test: SkipByDesign # Expects unobfuscated stack trace
+type_tostring_test: SkipByDesign # Expects names in Type.toString()
 
 # All static_tests have expected compile-time errors.
 [ $compiler != app_jitk && $compiler != dart2analyzer && $compiler != dart2js && $compiler != dartdevc && $compiler != dartdevk && $compiler != dartk && $compiler != dartkb && $compiler != dartkp && $compiler != fasta ]
