[vm] Remove Closure::signature().

Revert the changes in 0920ac88 to use Closure::signature() and
remove the previously existing uses.

Refactor the majority of Object_haveSameRuntimeType out into a helper
function, which lifts the various Bool::Get()s out of the return
statements for easier reading. Also delay retrieving the class out of
the instance until after checking for closure, since we can check the
retrieved cids against kClosureCid instead.

Also move ASSERTs from debug printing of SubtypeTestCache entries
to UpdateTypeTestCache, to ensure they are checked when
--trace-type-checks is not enabled.

TEST=Refactoring, so existing tests. Checked manually on a separate
     CL which triggered a case where Closure::signature() was
     erroneously returning null due to the direct trip through ptrs.

Change-Id: Iff9b5c8e3f6426cc7deb5ce73c02606597d99d5b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/207124
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Martin Kustermann <kustermann@google.com>
diff --git a/runtime/lib/object.cc b/runtime/lib/object.cc
index cdc2a24..f172831 100644
--- a/runtime/lib/object.cc
+++ b/runtime/lib/object.cc
@@ -91,57 +91,56 @@
   return instance.GetType(Heap::kNew);
 }
 
-DEFINE_NATIVE_ENTRY(Object_haveSameRuntimeType, 0, 2) {
-  const Instance& left =
-      Instance::CheckedHandle(zone, arguments->NativeArgAt(0));
-  const Instance& right =
-      Instance::CheckedHandle(zone, arguments->NativeArgAt(1));
-
+static bool HaveSameRuntimeTypeHelper(Zone* zone,
+                                      const Instance& left,
+                                      const Instance& right) {
   const intptr_t left_cid = left.GetClassId();
   const intptr_t right_cid = right.GetClassId();
 
   if (left_cid != right_cid) {
     if (IsIntegerClassId(left_cid)) {
-      return Bool::Get(IsIntegerClassId(right_cid)).ptr();
-    } else if (IsStringClassId(left_cid)) {
-      return Bool::Get(IsStringClassId(right_cid)).ptr();
-    } else if (IsTypeClassId(left_cid)) {
-      return Bool::Get(IsTypeClassId(right_cid)).ptr();
-    } else {
-      return Bool::False().ptr();
+      return IsIntegerClassId(right_cid);
     }
+    if (IsStringClassId(left_cid)) {
+      return IsStringClassId(right_cid);
+    }
+    if (IsTypeClassId(left_cid)) {
+      return IsTypeClassId(right_cid);
+    }
+    return false;
   }
 
-  const Class& cls = Class::Handle(left.clazz());
-  if (cls.IsClosureClass()) {
+  if (left_cid == kClosureCid) {
     const auto& left_closure = Closure::Cast(left);
     const auto& right_closure = Closure::Cast(right);
     // If all the components that make up the instantiated signature are equal,
     // then no need to instantiate.
-    if (left_closure.signature() == right_closure.signature() &&
-        left_closure.function_type_arguments() ==
+    if (left_closure.function_type_arguments() ==
             right_closure.function_type_arguments() &&
         left_closure.delayed_type_arguments() ==
             right_closure.delayed_type_arguments() &&
         left_closure.instantiator_type_arguments() ==
             right_closure.instantiator_type_arguments()) {
-      return Bool::True().ptr();
+      const auto& left_fun = Function::Handle(zone, left_closure.function());
+      const auto& right_fun = Function::Handle(zone, right_closure.function());
+      if (left_fun.signature() == right_fun.signature()) {
+        return true;
+      }
     }
     const AbstractType& left_type =
         AbstractType::Handle(zone, left.GetType(Heap::kNew));
     const AbstractType& right_type =
         AbstractType::Handle(zone, right.GetType(Heap::kNew));
-    return Bool::Get(
-               left_type.IsEquivalent(right_type, TypeEquality::kSyntactical))
-        .ptr();
+    return left_type.IsEquivalent(right_type, TypeEquality::kSyntactical);
   }
 
+  const Class& cls = Class::Handle(zone, left.clazz());
   if (!cls.IsGeneric()) {
-    return Bool::True().ptr();
+    return true;
   }
 
   if (left.GetTypeArguments() == right.GetTypeArguments()) {
-    return Bool::True().ptr();
+    return true;
   }
   const TypeArguments& left_type_arguments =
       TypeArguments::Handle(zone, left.GetTypeArguments());
@@ -149,10 +148,17 @@
       TypeArguments::Handle(zone, right.GetTypeArguments());
   const intptr_t num_type_args = cls.NumTypeArguments();
   const intptr_t num_type_params = cls.NumTypeParameters();
-  return Bool::Get(left_type_arguments.IsSubvectorEquivalent(
-                       right_type_arguments, num_type_args - num_type_params,
-                       num_type_params, TypeEquality::kSyntactical))
-      .ptr();
+  return left_type_arguments.IsSubvectorEquivalent(
+      right_type_arguments, num_type_args - num_type_params, num_type_params,
+      TypeEquality::kSyntactical);
+}
+
+DEFINE_NATIVE_ENTRY(Object_haveSameRuntimeType, 0, 2) {
+  const Instance& left =
+      Instance::CheckedHandle(zone, arguments->NativeArgAt(0));
+  const Instance& right =
+      Instance::CheckedHandle(zone, arguments->NativeArgAt(1));
+  return Bool::Get(HaveSameRuntimeTypeHelper(zone, left, right)).ptr();
 }
 
 DEFINE_NATIVE_ENTRY(Object_instanceOf, 0, 4) {
diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc
index 2b71427..182c090 100644
--- a/runtime/vm/isolate_reload.cc
+++ b/runtime/vm/isolate_reload.cc
@@ -2113,6 +2113,7 @@
         type_(AbstractType::Handle(zone)),
         cache_(SubtypeTestCache::Handle(zone)),
         entries_(Array::Handle(zone)),
+        closure_function_(Function::Handle(zone)),
         instantiator_type_arguments_(TypeArguments::Handle(zone)),
         function_type_arguments_(TypeArguments::Handle(zone)),
         instance_cid_or_signature_(Object::Handle(zone)),
@@ -2217,7 +2218,8 @@
     const intptr_t cid = cls_.id();
     if (cid == kClosureCid) {
       const auto& closure = Closure::Cast(value);
-      instance_cid_or_signature_ = closure.signature();
+      closure_function_ = closure.function();
+      instance_cid_or_signature_ = closure_function_.signature();
       instance_type_arguments_ = closure.instantiator_type_arguments();
       parent_function_type_arguments_ = closure.function_type_arguments();
       delayed_function_type_arguments_ = closure.delayed_type_arguments();
@@ -2292,6 +2294,7 @@
   AbstractType& type_;
   SubtypeTestCache& cache_;
   Array& entries_;
+  Function& closure_function_;
   TypeArguments& instantiator_type_arguments_;
   TypeArguments& function_type_arguments_;
   Object& instance_cid_or_signature_;
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index 65a4b51..7ebeb28 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -18243,7 +18243,6 @@
     buffer->Printf("%sclass id: %" Pd "", separator,
                    Smi::Cast(instance_class_id_or_signature).Value());
   } else {
-    ASSERT(instance_class_id_or_signature.IsFunctionType());
     buffer->Printf(
         "%ssignature: %s", separator,
         FunctionType::Cast(instance_class_id_or_signature).ToCString());
@@ -18282,12 +18281,10 @@
                    function_type_arguments.ToCString());
   }
   if (!instance_parent_function_type_arguments.IsNull()) {
-    ASSERT(instance_class_id_or_signature.IsFunctionType());
     buffer->Printf("%sclosure parent function type arguments: %s", separator,
                    instance_parent_function_type_arguments.ToCString());
   }
   if (!instance_delayed_type_arguments.IsNull()) {
-    ASSERT(instance_class_id_or_signature.IsFunctionType());
     buffer->Printf("%sclosure delayed function type arguments: %s", separator,
                    instance_delayed_type_arguments.ToCString());
   }
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
index 87e5b93..3046b1a 100644
--- a/runtime/vm/object.h
+++ b/runtime/vm/object.h
@@ -11242,17 +11242,6 @@
     return closure.untag()->function();
   }
 
-#if defined(DART_PRECOMPILER)
-  FunctionTypePtr signature() const {
-    return FunctionType::RawCast(WeakSerializationReference::Unwrap(
-        untag()->function()->untag()->signature()));
-  }
-#else
-  FunctionTypePtr signature() const {
-    return untag()->function()->untag()->signature();
-  }
-#endif
-
   ContextPtr context() const { return untag()->context(); }
   static intptr_t context_offset() {
     return OFFSET_OF(UntaggedClosure, context_);
diff --git a/runtime/vm/runtime_entry.cc b/runtime/vm/runtime_entry.cc
index 6bedc9e..b23182e 100644
--- a/runtime/vm/runtime_entry.cc
+++ b/runtime/vm/runtime_entry.cc
@@ -758,7 +758,9 @@
   auto& instance_delayed_type_arguments = TypeArguments::Handle(zone);
   if (instance_class.IsClosureClass()) {
     const auto& closure = Closure::Cast(instance);
-    instance_class_id_or_signature = closure.signature();
+    const auto& function = Function::Handle(zone, closure.function());
+    instance_class_id_or_signature = function.signature();
+    ASSERT(instance_class_id_or_signature.IsFunctionType());
     instance_type_arguments = closure.instantiator_type_arguments();
     instance_parent_function_type_arguments = closure.function_type_arguments();
     instance_delayed_type_arguments = closure.delayed_type_arguments();