1. Fix the type arguments recursion problem that gets introduced when canonicalization of type argu…

runtime/vm/snapshot.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/snapshot.h"
 
 #include "platform/assert.h"
 #include "vm/bootstrap.h"
 #include "vm/class_finalizer.h"
 #include "vm/dart.h"
@@ skipping 196 matching lines @@
   LongJumpScope jump;
   if (setjmp(*jump.Set()) == 0) {
     PassiveObject& obj =
         PassiveObject::Handle(isolate(), ReadObjectImpl(kAsInlinedObject));
     for (intptr_t i = 0; i < backward_references_->length(); i++) {
       if (!(*backward_references_)[i].is_deserialized()) {
         ReadObjectImpl(kAsInlinedObject);
         (*backward_references_)[i].set_state(kIsDeserialized);
       }
     }
-    ProcessDeferredCanonicalizations();
+    if (kind() != Snapshot::kFull) {
+      ProcessDeferredCanonicalizations();
+    }
     return obj.raw();
   } else {
     // An error occurred while reading, return the error object.
     const Error& err = Error::Handle(isolate()->object_store()->sticky_error());
     isolate()->object_store()->clear_sticky_error();
     return err.raw();
   }
 }
 
 
@@ skipping 920 matching lines @@
     ASSERT(index >= max_vm_isolate_object_id_);
     index -= max_vm_isolate_object_id_;
     ASSERT(index < backward_references_->length());
     BackRefNode& ref = (*backward_references_)[index];
     ref.AddPatchRecord(patch_object_id, patch_offset);
   }
 }
 
 
 void SnapshotReader::ProcessDeferredCanonicalizations() {
-  AbstractType& typeobj = AbstractType::Handle();
+  Type& typeobj = Type::Handle();
   TypeArguments& typeargs = TypeArguments::Handle();
   Object& newobj = Object::Handle();
   for (intptr_t i = 0; i < backward_references_->length(); i++) {
     BackRefNode& backref = (*backward_references_)[i];
     if (backref.defer_canonicalization()) {
       Object* objref = backref.reference();
+      bool needs_patching = false;
       // Object should either be an abstract type or a type argument.
-      if (objref->IsAbstractType()) {
+      if (objref->IsType()) {
         typeobj ^= objref->raw();
-        typeobj.ClearCanonical();
         newobj = typeobj.Canonicalize();

[regis, 2015/08/06 16:36:08]

Ideally, in the case of a resursive type, Canonicalize() should return the input
node, and not a different (but equivalent) node in the cyclic graph. This would
make patching safe in all cases.

This would be easier to use than IsDisjoint() or IsEquivalentButDisjoint() new
functions as we discussed. In other words, this new functionality should be
encapsulated in the Canonicalize() call.

I'll work on that next.

[siva, 2015/08/06 17:28:44]

I was thinking of another approach wherein we always use TypeRef nodes in a
recursive implementation so that it is always uniform and do not make a Type
node be equivalent to a type ref node pointing to the type node. We can discuss
this in person.

[regis, 2015/08/06 18:24:35]

We still have to make sure type tests succeed. Yes, let's talk.

+        if ((newobj.raw() != typeobj.raw()) && !typeobj.IsRecursive()) {
+          needs_patching = true;
+        } else {
+          // Set Canonical bit.
+          objref->SetCanonical();
+        }
       } else {
         ASSERT(objref->IsTypeArguments());
         typeargs ^= objref->raw();
-        typeargs.ClearCanonical();
         newobj = typeargs.Canonicalize();
+        if ((newobj.raw() != typeargs.raw()) && !typeargs.IsRecursive()) {
+          needs_patching = true;
+        } else {
+          // Set Canonical bit.
+          objref->SetCanonical();
+        }
       }
-      if (newobj.raw() == objref->raw()) {
-        // Restore Canonical bit.
-        objref->SetCanonical();
-      } else {
+      if (needs_patching) {
         ZoneGrowableArray<intptr_t>* patches = backref.patch_records();
         ASSERT(newobj.IsCanonical());
         ASSERT(patches != NULL);
         for (intptr_t j = 0; j < patches->length(); j+=2) {
           NoSafepointScope no_safepoint;
           intptr_t patch_object_id = (*patches)[j];
           intptr_t patch_offset = (*patches)[j + 1];
           Object* target = GetBackRef(patch_object_id);
           RawObject** rawptr =
               reinterpret_cast<RawObject**>(target->raw()->ptr());
@@ skipping 1207 matching lines @@
     NoSafepointScope no_safepoint;
     WriteObject(obj.raw());
     UnmarkAll();
   } else {
     ThrowException(exception_type(), exception_msg());
   }
 }
 
 
 }  // namespace dart