Support ignorance of base class finalizers.

tools/clang/blink_gc_plugin/RecordInfo.cpp

Index: tools/clang/blink_gc_plugin/RecordInfo.cpp
diff --git a/tools/clang/blink_gc_plugin/RecordInfo.cpp b/tools/clang/blink_gc_plugin/RecordInfo.cpp
index 5250eaa084d8e0ba2c341a563188a05760f297a2..b52a5d4be361c3a1edb2e921c081f503ea338d42 100644
--- a/tools/clang/blink_gc_plugin/RecordInfo.cpp
+++ b/tools/clang/blink_gc_plugin/RecordInfo.cpp
@@ -18,6 +18,7 @@ RecordInfo::RecordInfo(CXXRecordDecl* record, RecordCache* cache)
       is_stack_allocated_(kNotComputed),
       is_non_newable_(kNotComputed),
       is_only_placement_newable_(kNotComputed),
+      does_need_finalization_(kNotComputed),
       determined_trace_methods_(false),
       trace_method_(0),
       trace_dispatch_method_(0),
@@ -400,7 +401,48 @@ void RecordInfo::DetermineTracingMethods() {
 
 // TODO: Add classes with a finalize() method that specialize FinalizerTrait.
 bool RecordInfo::NeedsFinalization() {
-  return record_->hasNonTrivialDestructor();
+  if (does_need_finalization_ == kNotComputed) {
+    // Rely on hasNonTrivialDestructor(), but if the only
+    // identifiable reason why it is true is the presence
+    // of a class with a safely ignorable class as a direct base,
+    // or such a class is being represented here, then it does
+    // not need finalization.
+    does_need_finalization_ =
+        record_->hasNonTrivialDestructor() ? kTrue : kFalse;
+    if (does_need_finalization_) {

[zerny-chromium, 2014/07/08 07:45:03]

Add early exit instead:

if (!does_need_finalization_)
  return does_need_finalization_;

[sof, 2014/07/08 08:47:05]

Rephrased to use early returns instead of fallthrough.

+      // Checking if processing a class with a safely-ignorable destructor.
+      NamespaceDecl* ns =
+          dyn_cast<NamespaceDecl>(record_->getDeclContext());
+      if (ns && Config::HasIgnorableDestructor(ns->getName(), name_))
+        does_need_finalization_ = kFalse;

[zerny-chromium, 2014/07/08 07:45:02]

Do an early return in this block as well and avoid the following conditional.

+    }
+    if (does_need_finalization_) {
+      CXXDestructorDecl* dtor = record_->getDestructor();
+      bool is_candidate = !dtor || !dtor->isUserProvided();
+      for (Fields::iterator it = GetFields().begin();
+           is_candidate && it != GetFields().end();
+           ++it) {
+        if (it->second.edge()->NeedsFinalization())
+          is_candidate = false;

[zerny-chromium, 2014/07/08 07:45:03]

This can just return does_need_finalization_ which is true here.

+      }
+
+      for (Bases::iterator it = GetBases().begin();
+           is_candidate && it != GetBases().end();
+           ++it) {
+        NamespaceDecl* ns = dyn_cast<NamespaceDecl>(
+            it->second.info()->record()->getDeclContext());
+        if (ns &&

[zerny-chromium, 2014/07/08 07:45:03]

This check is not be needed here. The ignored case already happens in the
base-case of an ignored class.

[sof, 2014/07/08 08:47:05]

I was hesitant to remove it, in case the complete type of the ignorable classes
wasn't in case. But in that case (and if that's supported), the forward decl
must be, so it will have been processed already. Hence, ok to do so. Removed.

[sof, 2014/07/08 08:48:49]

s/in case/in scope/. Need more coffee.

+            Config::HasIgnorableDestructor(ns->getName(),
+                                           it->second.info()->name()))
+          continue;
+        if (it->second.info()->NeedsFinalization())
+          is_candidate = false;

[zerny-chromium, 2014/07/08 07:45:02]

Just return does_need_finalization_

+      }
+      if (is_candidate)

[zerny-chromium, 2014/07/08 07:45:02]

Here we can unconditionally set does_need_finalization_ = kFalse.

+        does_need_finalization_ = kFalse;
+    }
+  }
+  return does_need_finalization_;
 }
 
 // A class needs tracing if: