Support ignorance of base class finalizers.

tools/clang/blink_gc_plugin/RecordInfo.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "Config.h"
 #include "RecordInfo.h"
 
 using namespace clang;
 using std::string;
 
 RecordInfo::RecordInfo(CXXRecordDecl* record, RecordCache* cache)
     : cache_(cache),
       record_(record),
       name_(record->getName()),
       fields_need_tracing_(TracingStatus::Unknown()),
       bases_(0),
       fields_(0),
       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),
       finalize_dispatch_method_(0),
       is_gc_derived_(false),
       base_paths_(0) {}
 
 RecordInfo::~RecordInfo() {
   delete fields_;
   delete bases_;
@@ skipping 362 matching lines @@
             it->second.info()->GetFinalizeDispatchMethod()) {
       assert(!finalize_dispatch_method_ &&
              "Multiple finalize dispatching methods");
       finalize_dispatch_method_ = dispatch;
     }
   }
 }
 
 // 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:
 // - it is allocated on the managed heap,
 // - it is derived from a class that needs tracing, or
 // - it contains fields that need tracing.
 // TODO: Defining NeedsTracing based on whether a class defines a trace method
 // (of the proper signature) over approximates too much. The use of transition
 // types causes some classes to have trace methods without them needing to be
 // traced.
@@ skipping 92 matching lines @@
         edge->members().push_back(member);
       }
       // TODO: Handle the case where we fail to create an edge (eg, if the
       // argument is a primitive type or just not fully known yet).
     }
     return edge;
   }
 
   return new Value(info);
 }