Delete patch classes once they are applied

runtime/vm/object.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/object.h"
 
 #include "include/dart_api.h"
 #include "platform/assert.h"
 #include "vm/assembler.h"
 #include "vm/cpu.h"
@@ skipping 2827 matching lines @@
 }
 
 
 // Apply the members from the patch class to the original class.
 bool Class::ApplyPatch(const Class& patch, Error* error) const {
   ASSERT(error != NULL);
   ASSERT(!is_finalized());
   // Shared handles used during the iteration.
   String& member_name = String::Handle();
 
-  const PatchClass& patch_class =
-      PatchClass::Handle(PatchClass::New(*this, patch));
+  const PatchClass& patch_class = PatchClass::Handle(
+      PatchClass::New(*this, Script::Handle(patch.script())));
 
   Array& orig_list = Array::Handle(functions());
   intptr_t orig_len = orig_list.Length();
   Array& patch_list = Array::Handle(patch.functions());
   intptr_t patch_len = patch_list.Length();
 
   // TODO(iposva): Verify that only patching existing methods and adding only
   // new private methods.
   Function& func = Function::Handle();
   Function& orig_func = Function::Handle();
@@ skipping 81 matching lines @@
     }
     new_list.SetAt(i, field);
   }
   for (intptr_t i = 0; i < orig_len; i++) {
     field ^= orig_list.At(i);
     new_list.SetAt(patch_len + i, field);
   }
   SetFields(new_list);
 
   // The functions and fields in the patch class are no longer needed.
+  // The patch class itself is also no longer needed.
   patch.SetFunctions(Object::empty_array());
   patch.SetFields(Object::empty_array());
+  Library::Handle(patch.library()).RemovePatchClass(patch);
   return true;
 }
 
 
 static RawString* BuildClosureSource(const Array& formal_params,
                                      const String& expr) {
   const GrowableObjectArray& src_pieces =
       GrowableObjectArray::Handle(GrowableObjectArray::New());
   String& piece = String::Handle();
   src_pieces.Add(Symbols::LParen());
@@ skipping 609 matching lines @@
   ASSERT(!value.IsNull());
   StorePointer(&raw_ptr()->mixin_, value.raw());
 }
 
 
 bool Class::IsMixinApplication() const {
   return mixin() != Type::null();
 }
 
 
-void Class::SetPatchClass(const Class& cls) const {
-  ASSERT(GetPatchClass() == Class::null());
-  const GrowableObjectArray& patch_classes =
-      GrowableObjectArray::Handle(Library::Handle(library()).patch_classes());
-  patch_classes.Add(cls);
-}
-
-
 RawClass* Class::GetPatchClass() const {
-  const GrowableObjectArray& patch_classes =
-      GrowableObjectArray::Handle(Library::Handle(library()).patch_classes());
-  Class& pc = Class::Handle();
-  for (intptr_t i = 0; i < patch_classes.Length(); i++) {
-    pc ^= patch_classes.At(i);
-    if (pc.Name() == this->Name()) {  // Names are canonicalized.
-      return pc.raw();
-    }
-  }
-  return Class::null();
+  const Library& lib = Library::Handle(library());
+  return lib.GetPatchClass(String::Handle(Name()));
 }
 
 
 void Class::AddDirectSubclass(const Class& subclass) const {
   ASSERT(!subclass.IsNull());
   ASSERT(subclass.SuperClass() == raw());
   // Do not keep track of the direct subclasses of class Object.
   ASSERT(!IsObjectClass());
   GrowableObjectArray& direct_subclasses =
       GrowableObjectArray::Handle(raw_ptr()->direct_subclasses_);
@@ skipping 5727 matching lines @@
     lib = ImportLibraryAt(i);
     error = lib.TransitiveLoadError();
     if (!error.IsNull()) {
       break;
     }
   }
   return error.raw();
 }
 
 
+void Library::AddPatchClass(const Class& cls) const {
+  ASSERT(cls.is_patch());
+  ASSERT(GetPatchClass(String::Handle(cls.Name())) == Class::null());
+  const GrowableObjectArray& patch_classes =
+      GrowableObjectArray::Handle(this->patch_classes());
+  patch_classes.Add(cls);
+}
+
+
+RawClass* Library::GetPatchClass(const String& name) const {
+  const GrowableObjectArray& patch_classes =
+      GrowableObjectArray::Handle(this->patch_classes());
+  Object& obj = Object::Handle();
+  for (intptr_t i = 0; i < patch_classes.Length(); i++) {
+    obj = patch_classes.At(i);
+    if (obj.IsClass() &&
+        (Class::Cast(obj).Name() == name.raw())) {  // Names are canonicalized.
+      return Class::RawCast(obj.raw());
+    }
+  }
+  return Class::null();
+}
+
+
+void Library::RemovePatchClass(const Class& cls) const {
+  ASSERT(cls.is_patch());
+  const GrowableObjectArray& patch_classes =
+      GrowableObjectArray::Handle(this->patch_classes());
+  const intptr_t num_classes = patch_classes.Length();
+  intptr_t i = 0;
+  while (i < num_classes) {
+    if (cls.raw() == patch_classes.At(i)) break;
+    i++;
+  }
+  if (i == num_classes) return;
+  // Replace the entry with the script. We keep the script so that
+  // Library::LoadedScripts() can find it without having to iterate
+  // over the members of each class.
+  ASSERT(i < num_classes);  // We must have found a class.
+  Class& pc = Class::Handle();
+  pc ^= patch_classes.At(i);
+  const Script& patch_script = Script::Handle(pc.script());

[Ivan Posva, 2015/12/08 09:11:14]

Why is this not just
Script::Handle(cls.script());
?

[hausner, 2015/12/08 17:00:15]

D'uh! I changed this code a few times and apparently didn't see the forest for
the trees anymore.

+  patch_classes.SetAt(i, patch_script);
+}
+
+
 static RawString* MakeClassMetaName(const Class& cls) {
   return Symbols::FromConcat(Symbols::At(), String::Handle(cls.Name()));
 }
 
 
 static RawString* MakeFieldMetaName(const Field& field) {
   const String& cname =
       String::Handle(MakeClassMetaName(Class::Handle(field.origin())));
   GrowableHandlePtrArray<const String> pieces(Thread::Current()->zone(), 3);
   pieces.Add(cname);
@@ skipping 414 matching lines @@
         owner_script = Field::Cast(entry).script();
       } else {
         continue;
       }
       AddScriptIfUnique(scripts, owner_script);
     }
 
     // Add all scripts from patch classes.
     GrowableObjectArray& patches = GrowableObjectArray::Handle(patch_classes());
     for (intptr_t i = 0; i < patches.Length(); i++) {
-      cls ^= patches.At(i);
-      owner_script = cls.script();
+      entry = patches.At(i);
+      if (entry.IsClass()) {
+        owner_script = Class::Cast(entry).script();
+      }  else {
+        ASSERT(entry.IsScript());
+        owner_script ^= Script::Cast(entry).raw();

[Ivan Posva, 2015/12/08 09:11:14]

Why is the ^= operator needed here?

[hausner, 2015/12/08 17:00:15]

Good catch. Will remove.

+      }
       AddScriptIfUnique(scripts, owner_script);
     }
 
     cls ^= toplevel_class();
     if (!cls.IsNull()) {
       owner_script = cls.script();
       AddScriptIfUnique(scripts, owner_script);
       // Special case: Scripts that only contain external top-level functions
       // are not included above, but can be referenced through a library's
       // anonymous classes. Example: dart-core:identical.dart.
@@ skipping 12125 matching lines @@
   return tag_label.ToCString();
 }
 
 
 void UserTag::PrintJSONImpl(JSONStream* stream, bool ref) const {
   Instance::PrintJSONImpl(stream, ref);
 }
 
 
 }  // namespace dart