BlinkGCPlugin: Correct trace() detection for CXXDependentScopeMemberExpr* case.

I fixed the trace() function detection logic in CheckTraceBaseCall() in my prior
patch, but I should have fixed the similar logic in CheckCXXDependentScope-
MemberExpr().

This came up as an issue when I compiled some SVG files. Without this patch
applied, the plugin fails to find "SVGPropertyHelper<Derived>::trace(visitor)"
call in SVGListPropertyHelper<Derived, ItemProperty>::traceImpl().

BUG=462511
R=kouhei@chromium.org
CC=zerny@chromium.org, oilpan-reviews@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#318427}

4 files changed, 97 insertions, 17 deletions

diff --git a/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp b/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
index 85cf086..fb87ce8 100644
--- a/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
+++ b/tools/clang/blink_gc_plugin/BlinkGCPlugin.cpp
@@ -408,6 +408,18 @@ class CheckTraceVisitor : public RecursiveASTVisitor<CheckTraceVisitor> {
   }
 
  private:
+  bool IsTraceCallName(const std::string& name) {
+    if (trace_->getName() == kTraceImplName)
+      return name == kTraceName;
+    if (trace_->getName() == kTraceAfterDispatchImplName)
+      return name == kTraceAfterDispatchName;
+    // Currently, a manually dispatched class cannot have mixin bases (having
+    // one would add a vtable which we explicitly check against). This means
+    // that we can only make calls to a trace method of the same name. Revisit
+    // this if our mixin/vtable assumption changes.
+    return name == trace_->getName();
+  }
+
   CXXRecordDecl* GetDependentTemplatedDecl(CXXDependentScopeMemberExpr* expr) {
     NestedNameSpecifier* qual = expr->getQualifier();
     if (!qual)
@@ -445,13 +457,12 @@ class CheckTraceVisitor : public RecursiveASTVisitor<CheckTraceVisitor> {
       return;
 
     // Check for Super<T>::trace(visitor)
-    if (call->getNumArgs() == 1 && fn_name == trace_->getName()) {
+    if (call->getNumArgs() == 1 && IsTraceCallName(fn_name)) {
       RecordInfo::Bases::iterator it = info_->GetBases().begin();
       for (; it != info_->GetBases().end(); ++it) {
         if (it->first->getName() == tmpl->getName())
           it->second.MarkTraced();
       }
-      return;
     }
 
     // Check for TraceIfNeeded<T>::trace(visitor, &field)
@@ -524,23 +535,12 @@ class CheckTraceVisitor : public RecursiveASTVisitor<CheckTraceVisitor> {
       func_name = callee->getMemberName().getAsString();
     }
 
-    if (trace_->getName() == kTraceImplName) {
-      if (func_name != kTraceName)
-        return false;
-    } else if (trace_->getName() == kTraceAfterDispatchImplName) {
-      if (func_name != kTraceAfterDispatchName)
-        return false;
-    } else {
-      // Currently, a manually dispatched class cannot have mixin bases (having
-      // one would add a vtable which we explicitly check against). This means
-      // that we can only make calls to a trace method of the same name. Revisit
-      // this if our mixin/vtable assumption changes.
-      if (func_name != trace_->getName())
-        return false;
-    }
-
     if (!callee_record)
       return false;
+
+    if (!IsTraceCallName(func_name))
+      return false;
+
     RecordInfo::Bases::iterator iter = info_->GetBases().find(callee_record);
     if (iter == info_->GetBases().end())
       return false;
diff --git a/tools/clang/blink_gc_plugin/tests/traceimpl_dependent_scope.cpp b/tools/clang/blink_gc_plugin/tests/traceimpl_dependent_scope.cpp
new file mode 100644
index 0000000..11b576c
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/traceimpl_dependent_scope.cpp
@@ -0,0 +1,13 @@
+// Copyright 2015 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 "traceimpl_dependent_scope.h"
+
+namespace blink {
+
+// Template instantiation.
+template class Derived<int>;
+template class DerivedMissingTrace<int>;
+
+}
diff --git a/tools/clang/blink_gc_plugin/tests/traceimpl_dependent_scope.h b/tools/clang/blink_gc_plugin/tests/traceimpl_dependent_scope.h
new file mode 100644
index 0000000..0d079f6
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/traceimpl_dependent_scope.h
@@ -0,0 +1,62 @@
+// Copyright 2015 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.
+
+#ifndef TRACEIMPL_DEPENDENT_SCOPE_H_
+#define TRACEIMPL_DEPENDENT_SCOPE_H_
+
+#include "heap/stubs.h"
+
+namespace blink {
+
+class X : public GarbageCollected<X> {
+ public:
+  virtual void trace(Visitor*) {}
+};
+
+template <typename T>
+class Base : public GarbageCollected<Base<T> > {
+ public:
+  virtual void trace(Visitor* visitor) { traceImpl(visitor); }
+  virtual void trace(InlinedGlobalMarkingVisitor visitor) {
+    traceImpl(visitor);
+  }
+
+ private:
+  template <typename VisitorDispatcher>
+  void traceImpl(VisitorDispatcher visitor) {}
+};
+
+template <typename T>
+class Derived : public Base<T> {
+ public:
+  void trace(Visitor* visitor) override { traceImpl(visitor); }
+  void trace(InlinedGlobalMarkingVisitor visitor) override {
+    traceImpl(visitor);
+  }
+
+ private:
+  template <typename VisitorDispatcher>
+  void traceImpl(VisitorDispatcher visitor) {
+    Base<T>::trace(visitor);
+  }
+};
+
+template <typename T>
+class DerivedMissingTrace : public Base<T> {
+ public:
+  void trace(Visitor* visitor) override { traceImpl(visitor); }
+  void trace(InlinedGlobalMarkingVisitor visitor) override {
+    traceImpl(visitor);
+  }
+
+ private:
+  template <typename VisitorDispatcher>
+  void traceImpl(VisitorDispatcher visitor) {
+    // Missing Base<T>::trace(visitor).
+  }
+};
+
+}
+
+#endif  // TRACEIMPL_DEPENDENT_SCOPE_H_
diff --git a/tools/clang/blink_gc_plugin/tests/traceimpl_dependent_scope.txt b/tools/clang/blink_gc_plugin/tests/traceimpl_dependent_scope.txt
new file mode 100644
index 0000000..e1aab33
--- /dev/null
+++ b/tools/clang/blink_gc_plugin/tests/traceimpl_dependent_scope.txt
@@ -0,0 +1,5 @@
+In file included from traceimpl_dependent_scope.cpp:5:
+./traceimpl_dependent_scope.h:55:3: warning: [blink-gc] Base class 'Base<int>' of derived class 'DerivedMissingTrace<int>' requires tracing.
+  void traceImpl(VisitorDispatcher visitor) {
+  ^
+1 warning generated.