Add an assertion to ensure that the return type and types of parameters are canonical or a TypeParam

runtime/vm/dart_api_impl.cc

Index: runtime/vm/dart_api_impl.cc
diff --git a/runtime/vm/dart_api_impl.cc b/runtime/vm/dart_api_impl.cc
index 9562941a55c1c60df280fc5fbc503b0682092fa2..968ca94758311ff4f841c26156259e5d8fbe9e48 100644
--- a/runtime/vm/dart_api_impl.cc
+++ b/runtime/vm/dart_api_impl.cc
@@ -71,6 +71,44 @@ const char* CanonicalFunction(const char* func) {
 }
 
 
+// An object visitor which will iterate over all the function objects in the
+// heap and check if the result type and parameter types are canonicalized
+// or not. If they are not canonicalized if prints the function name out.

[regis, 2015/06/11 23:53:39]

if -> it
Actually, it does not print, it asserts, and only in debug mode.
Should this class be wrapped in #ifdef DEBUG?

[siva, 2015/06/12 00:23:14]

Done.

+class FunctionVisitor : public ObjectVisitor {
+ public:
+  explicit FunctionVisitor(Isolate* isolate) :
+      ObjectVisitor(isolate),
+      classHandle_(Class::Handle(isolate)),
+      funcHandle_(Function::Handle(isolate)),
+      typeHandle_(AbstractType::Handle(isolate)) {}
+
+  void VisitObject(RawObject* obj) {
+    if (obj->IsFunction()) {
+      funcHandle_ ^= obj;
+      classHandle_ ^= funcHandle_.Owner();
+      // Verify that the result type of a function is canonical or a
+      // TypeParameter.
+      typeHandle_ ^= funcHandle_.result_type();
+      ASSERT(typeHandle_.IsNull() ||
+             typeHandle_.IsTypeParameter() ||
+             typeHandle_.IsCanonical());
+      // Verify that the types in the function signature are all canonical or
+      // a TypeParameter

[regis, 2015/06/11 23:53:39]

period

[siva, 2015/06/12 00:23:14]

Done.

+      const intptr_t num_parameters = funcHandle_.NumParameters();
+      for (intptr_t i = 0; i < num_parameters; i++) {
+        typeHandle_ = funcHandle_.ParameterTypeAt(i);
+        ASSERT(typeHandle_.IsTypeParameter() || typeHandle_.IsCanonical());
+      }
+    }
+  }
+
+ private:
+  Class& classHandle_;
+  Function& funcHandle_;
+  AbstractType& typeHandle_;
+};
+
+
 static RawInstance* GetListInstance(Isolate* isolate, const Object& obj) {
   if (obj.IsInstance()) {
     const Library& core_lib = Library::Handle(Library::CoreLibrary());
@@ -1463,6 +1501,10 @@ DART_EXPORT Dart_Handle Dart_CreateSnapshot(
   if (::Dart_IsError(state)) {
     return state;
   }
+  isolate->heap()->CollectAllGarbage();
+  FunctionVisitor check_canonical(isolate);
+  isolate->heap()->VisitObjects(&check_canonical);

[regis, 2015/06/11 23:53:39]

The 2 lines above have no effect in release mode. Should they be wrapped in #if
DEBUG?

[siva, 2015/06/12 00:23:14]

Yes I wrapped this under #if defined(DEBUG) ... #endif.

I also wrapped the FunctionVisitor class similarly,

+
   // Since this is only a snapshot the root library should not be set.
   isolate->object_store()->set_root_library(Library::Handle(isolate));
   FullSnapshotWriter writer(vm_isolate_snapshot_buffer,