Support ignorance of base class finalizers.

tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp

Index: tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
diff --git a/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp b/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
index d1d3a468ec88b1af5cdd8d145716e059d8239862..bc7229c294eecb8bbd0fcc08620a8a831b5c2efa 100644
--- a/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
+++ b/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
@@ -136,6 +136,7 @@ struct BlinkGCPluginOptions {
   std::set<std::string> ignored_classes;
   std::set<std::string> checked_namespaces;
   std::vector<std::string> ignored_directories;
+  std::set<std::string> ignored_base_class_finalizers;
 };
 
 typedef std::vector<CXXRecordDecl*> RecordVector;
@@ -582,6 +583,10 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     // Ignore GC implementation files.
     options_.ignored_directories.push_back("/heap/");
 
+    // Following http://crrev.com/369633033 (Blink r177436),
+    // ignore WebCore::ScriptWrappable.
+    options_.ignored_base_class_finalizers.insert("ScriptWrappable::WebCore");

[zerny-chromium, 2014/07/07 09:04:33]

Lets swap this so it is ns::record

+
     // Register warning/error messages.
     diag_class_must_left_mostly_derive_gc_ = diagnostic_.getCustomDiagID(
         getErrorLevel(), kClassMustLeftMostlyDeriveGC);
@@ -860,6 +865,19 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     if (info->IsGCMixin())
       return;
 
+    // If class has no destructor and all base class destructors can
+    // safely be ignored, do not require finalization.
+    bool has_relevant_dtors = dtor && dtor->isUserProvided();
+    for (RecordInfo::Bases::iterator it = info->GetBases().begin();
+         !has_relevant_dtors && it != info->GetBases().end();
+         ++it) {
+      if (it->second.info()->NeedsFinalization())
+        has_relevant_dtors = !IsIgnoredFinalizableBaseClass(it->second.info());
+    }
+
+    if (!has_relevant_dtors)
+      return;

[zerny-chromium, 2014/07/07 09:04:33]

This will allow erroneous code to pass the plugin, since a class declaring
fields that need finalization will not necessarily cause the class to need
finalization anymore.

class Foo : public GarbageCollected<Foo>, public ScriptWrappable {
  // ... and no user declared dtor.
private:
  String m_str;
};

I think this definition of Foo will not give a compile error despite the String
requiring Foo to have finalization support. Also, say Foo did not have the
string, then still a subclass would still require finalization since we only
ignore on the direct bases:

class Foo : public GarbageCollected<Foo>, public ScriptWrappable { };
class Bar : public Foo { };

Ie, this definition of Bar will give an error I think.

+
     // Report the finalization error, and proceed to print possible causes for
     // the finalization requirement.
     ReportClassRequiresFinalization(info);
@@ -871,7 +889,8 @@ class BlinkGCPluginConsumer : public ASTConsumer {
          it != info->GetBases().end();
          ++it) {
       if (it->second.info()->NeedsFinalization())
-        NoteBaseRequiresFinalization(&it->second);
+        if (!IsIgnoredFinalizableBaseClass(it->second.info()))
+          NoteBaseRequiresFinalization(&it->second);
     }
 
     for (RecordInfo::Fields::iterator it = info->GetFields().begin();
@@ -1084,6 +1103,18 @@ class BlinkGCPluginConsumer : public ASTConsumer {
            options_.ignored_classes.end();
   }
 
+  bool IsIgnoredFinalizableBaseClass(RecordInfo* info) {
+    NamespaceDecl* ns =
+        dyn_cast<NamespaceDecl>(info->record()->getDeclContext());
+    if (!ns)
+      return false;
+
+    string qualified_name =
+        string(info->name()).append("::").append(ns->getName());
+    return options_.ignored_base_class_finalizers.find(qualified_name) !=
+           options_.ignored_base_class_finalizers.end();
+  }
+
   bool InIgnoredDirectory(RecordInfo* info) {
     string filename;
     if (!GetFilename(info->record()->getLocStart(), &filename))