Check consistency of manual trace and finalization dispatching.

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 ed1817d5576f54ca19317f9da29e014c9c920aa0..fe84dcc49ccc8f36b81ceb7d544fc7c92ccd3108 100644
--- a/tools/clang/blink_gc_plugin/RecordInfo.cpp
+++ b/tools/clang/blink_gc_plugin/RecordInfo.cpp
@@ -194,6 +194,34 @@ bool RecordInfo::InheritsNonPureTrace() {
   return false;
 }
 
+CXXMethodDecl* RecordInfo::InheritsNonVirtualTrace() {
+  if (CXXMethodDecl* trace = GetTraceMethod())
+    return trace->isVirtual() ? 0 : trace;
+  for (Bases::iterator it = GetBases().begin(); it != GetBases().end(); ++it) {
+    if (CXXMethodDecl* trace = it->second.info()->InheritsNonVirtualTrace())
+      return trace;
+  }
+  return 0;
+}
+
+// A (non-virtual) class is considered abstract in Blink if it has
+// no public constructors and no create methods.
+bool RecordInfo::IsConsideredAbstract() {
+  for (CXXRecordDecl::ctor_iterator it = record_->ctor_begin();
+       it != record_->ctor_end();
+       ++it) {
+    if (!it->isCopyOrMoveConstructor() && it->getAccess() == AS_public)
+      return false;
+  }
+  for (CXXRecordDecl::method_iterator it = record_->method_begin();
+       it != record_->method_end();
+       ++it) {
+    if (it->getNameAsString() == kCreateName)
+      return false;
+  }
+  return true;
+}
+
 RecordInfo::Bases* RecordInfo::CollectBases() {
   // Compute the collection locally to avoid inconsistent states.
   Bases* bases = new Bases;
@@ -254,7 +282,6 @@ void RecordInfo::DetermineTracingMethods() {
        it != record_->method_end();
        ++it) {
     if (Config::IsTraceMethod(*it, &isTraceAfterDispatch)) {
-      // TODO: Test that the formal parameter is of type Visitor*.
       if (isTraceAfterDispatch) {
         traceAfterDispatch = *it;
       } else {
@@ -271,6 +298,16 @@ void RecordInfo::DetermineTracingMethods() {
     trace_method_ = trace;
     trace_dispatch_method_ = 0;
   }
+  if (trace_dispatch_method_)
+    return;

[Mads Ager (chromium), 2014/03/18 09:19:32]

How about removing this and instead moving the below for loop into the else
branch above?

[zerny-chromium, 2014/03/18 10:03:41]

The for loop might need to run in either branch since trace_dispatch_method_ can
be 0 in both cases (trace might be 0).

+  // If this class does not define a trace dispatch method search inherit it.

[Mads Ager (chromium), 2014/03/18 09:19:32]

search the inheritance hierarchy for it?

[zerny-chromium, 2014/03/18 10:03:41]

Yes. Thanks.

+  for (Bases::iterator it = GetBases().begin(); it != GetBases().end(); ++it) {
+    if (CXXMethodDecl* dispatch = it->second.info()->GetTraceDispatchMethod()) {
+      // TODO: Does it make sense to inherit multiple dispatch methods?
+      assert(!trace_dispatch_method_ && "Multiple trace dispatching methods");
+      trace_dispatch_method_ = dispatch;
+    }
+  }
 }
 
 // TODO: Add classes with a finalize() method that specialize FinalizerTrait.