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

runtime/vm/snapshot.cc

Index: runtime/vm/snapshot.cc
diff --git a/runtime/vm/snapshot.cc b/runtime/vm/snapshot.cc
index 98ae976d4424bf11fb740323dde44e6b7e2c3067..c02a03f6465553dba1f9b264265984240bf759cc 100644
--- a/runtime/vm/snapshot.cc
+++ b/runtime/vm/snapshot.cc
@@ -214,7 +214,9 @@ RawObject* SnapshotReader::ReadObject() {
         (*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.
@@ -1155,28 +1157,36 @@ void SnapshotReader::AddPatchRecord(intptr_t object_id,
 
 
 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);