Blink GC plugin: disallow pure-virtual trace methods.

BUG=334149
R=ager@chromium.org
NOTRY=true

Review URL: https://codereview.chromium.org/278513005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269547 0039d316-1c4b-4281-b951-d872f2087c98

tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp

@@ -120,6 +120,10 @@ const char kClassOverridesNew[] =
     "[blink-gc] Garbage collected class %0"
     " is not permitted to override its new operator.";
 
+const char kClassDeclaresPureVirtualTrace[] =
+    "[blink-gc] Garbage collected class %0"
+    " is not permitted to declare a pure-virtual trace method.";
+
 struct BlinkGCPluginOptions {
   BlinkGCPluginOptions() : enable_oilpan(false), dump_graph(false) {}
   bool enable_oilpan;
@@ -570,6 +574,8 @@ class BlinkGCPluginConsumer : public ASTConsumer {
         diagnostic_.getCustomDiagID(getErrorLevel(), kDerivesNonStackAllocated);
     diag_class_overrides_new_ =
         diagnostic_.getCustomDiagID(getErrorLevel(), kClassOverridesNew);
+    diag_class_declares_pure_virtual_trace_ = diagnostic_.getCustomDiagID(
+        getErrorLevel(), kClassDeclaresPureVirtualTrace);
 
     // Register note messages.
     diag_field_requires_tracing_note_ = diagnostic_.getCustomDiagID(
@@ -692,8 +698,12 @@ class BlinkGCPluginConsumer : public ASTConsumer {
       }
     }
 
-    if (info->RequiresTraceMethod() && !info->GetTraceMethod())
+    if (CXXMethodDecl* trace = info->GetTraceMethod()) {
+      if (trace->isPure())
+        ReportClassDeclaresPureVirtualTrace(info, trace);
+    } else if (info->RequiresTraceMethod()) {
       ReportClassRequiresTraceMethod(info);
+    }
 
     {
       CheckFieldsVisitor visitor(options_);
@@ -1252,6 +1262,15 @@ class BlinkGCPluginConsumer : public ASTConsumer {
     diagnostic_.Report(full_loc, diag_class_overrides_new_) << info->record();
   }
 
+  void ReportClassDeclaresPureVirtualTrace(RecordInfo* info,
+                                           CXXMethodDecl* trace) {
+    SourceLocation loc = trace->getLocStart();
+    SourceManager& manager = instance_.getSourceManager();
+    FullSourceLoc full_loc(loc, manager);
+    diagnostic_.Report(full_loc, diag_class_declares_pure_virtual_trace_)
+        << info->record();
+  }
+
   void NoteManualDispatchMethod(CXXMethodDecl* dispatch) {
     SourceLocation loc = dispatch->getLocStart();
     SourceManager& manager = instance_.getSourceManager();
@@ -1333,6 +1352,7 @@ class BlinkGCPluginConsumer : public ASTConsumer {
   unsigned diag_missing_finalize_dispatch_;
   unsigned diag_derives_non_stack_allocated_;
   unsigned diag_class_overrides_new_;
+  unsigned diag_class_declares_pure_virtual_trace_;
 
   unsigned diag_field_requires_tracing_note_;
   unsigned diag_raw_ptr_to_gc_managed_class_note_;

tools/clang/blink_gc_plugin/RecordInfo.cpp

@@ -272,11 +272,11 @@ RecordInfo::Bases& RecordInfo::GetBases() {
   return *bases_;
 }
 
-bool RecordInfo::InheritsNonPureTrace() {
-  if (CXXMethodDecl* trace = GetTraceMethod())
-    return !trace->isPure();
+bool RecordInfo::InheritsTrace() {
+  if (GetTraceMethod())
+    return true;
   for (Bases::iterator it = GetBases().begin(); it != GetBases().end(); ++it) {
-    if (it->second.info()->InheritsNonPureTrace())
+    if (it->second.info()->InheritsTrace())
       return true;
   }
   return false;
@@ -323,7 +323,7 @@ RecordInfo::Bases* RecordInfo::CollectBases() {
     if (!info)
       continue;
     CXXRecordDecl* base = info->record();
-    TracingStatus status = info->InheritsNonPureTrace()
+    TracingStatus status = info->InheritsTrace()
                                ? TracingStatus::Needed()
                                : TracingStatus::Unneeded();
     bases->insert(std::make_pair(base, BasePoint(spec, info, status)));

tools/clang/blink_gc_plugin/RecordInfo.h

@@ -109,7 +109,7 @@ class RecordInfo {
   Fields* CollectFields();
   Bases* CollectBases();
   void DetermineTracingMethods();
-  bool InheritsNonPureTrace();
+  bool InheritsTrace();
 
   Edge* CreateEdge(const clang::Type* type);
 

tools/clang/blink_gc_plugin/tests/pure_virtual_trace.cpp

@@ -4,11 +4,4 @@
 
 #include "pure_virtual_trace.h"
 
-namespace WebCore {
-
-void C::trace(Visitor* visitor) {
-    visitor->trace(m_a);
-    // Is not required to trace base classes B and A.
-}
-
-}
+// Nothing to define

tools/clang/blink_gc_plugin/tests/pure_virtual_trace.h

@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef PURE_VIRTUAL_BASE_H_
-#define PURE_VIRTUAL_BASE_H_
+#ifndef PURE_VIRTUAL_TRACE_H_
+#define PURE_VIRTUAL_TRACE_H_
 
 #include "heap/stubs.h"
 
@@ -14,18 +14,6 @@ public:
     virtual void trace(Visitor*) = 0;
 };
 
-class B : public A {
-public:
-    // Does not need a trace method.
-};
-
-class C : public B {
-public:
-    void trace(Visitor*);
-private:
-    Member<A> m_a;
-};
-
 }
 
 #endif

tools/clang/blink_gc_plugin/tests/pure_virtual_trace.txt

-
+In file included from pure_virtual_trace.cpp:5:
+./pure_virtual_trace.h:14:5: warning: [blink-gc] Garbage collected class 'A' is not permitted to declare a pure-virtual trace method.
+    virtual void trace(Visitor*) = 0;
+    ^
+1 warning generated.