Check consistency of manual trace and finalization dispatching.

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"
 
 #include "clang/AST/Attr.h"
 
 using namespace clang;
@@ skipping 176 matching lines @@
 bool RecordInfo::InheritsNonPureTrace() {
   if (CXXMethodDecl* trace = GetTraceMethod())
     return !trace->isPure();
   for (Bases::iterator it = GetBases().begin(); it != GetBases().end(); ++it) {
     if (it->second.info()->InheritsNonPureTrace())
       return true;
   }
   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;
   if (!record_->hasDefinition())
     return bases;
   for (CXXRecordDecl::base_class_iterator it = record_->bases_begin();
        it != record_->bases_end();
        ++it) {
     const CXXBaseSpecifier& spec = *it;
     RecordInfo* info = cache_->Lookup(spec.getType());
@@ skipping 40 matching lines @@
   if (determined_trace_methods_)
     return;
   determined_trace_methods_ = true;
   CXXMethodDecl* trace = 0;
   CXXMethodDecl* traceAfterDispatch = 0;
   bool isTraceAfterDispatch;
   for (CXXRecordDecl::method_iterator it = record_->method_begin();
        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 {
         trace = *it;
       }
     }
   }
   if (traceAfterDispatch) {
     trace_method_ = traceAfterDispatch;
     trace_dispatch_method_ = trace;
   } else {
     // TODO: Can we never have a dispatch method called trace without the same
     // class defining a traceAfterDispatch method?
     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.
 bool RecordInfo::NeedsFinalization() {
   return record_->hasNonTrivialDestructor();
 }
 
 // A class needs tracing if:
 // - it is allocated on the managed heap,
 // - it is derived from a class that needs tracing, or
@@ skipping 96 matching lines @@
         // We failed to create an edge so abort the entire edge construction.
         delete edge;  // Will delete the already allocated members.
         return 0;
       }
     }
     return edge;
   }
 
   return new Value(info);
 }