clang-plugin: Enable RecursiveASTVisitor approach by default.

This patch removes the ASTConsumer approach for checking TagDecls in
favor of always using RecursiveASTVisitor. This means that some of
the warnings have to be temporarily suppressed, since the ASTConsumer
approach was never checking them.

R=dcheng, thakis@chromium.org
BUG=436357

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

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

6 files changed, 15 insertions, 41 deletions

diff --git a/tools/clang/plugins/ChromeClassTester.cpp b/tools/clang/plugins/ChromeClassTester.cpp
index a6f9fff..2b22ed9 100644
--- a/tools/clang/plugins/ChromeClassTester.cpp
+++ b/tools/clang/plugins/ChromeClassTester.cpp
@@ -41,18 +41,6 @@ ChromeClassTester::ChromeClassTester(CompilerInstance& instance,
 
 ChromeClassTester::~ChromeClassTester() {}
 
-void ChromeClassTester::HandleTagDeclDefinition(TagDecl* tag) {
-  pending_class_decls_.push_back(tag);
-}
-
-bool ChromeClassTester::HandleTopLevelDecl(DeclGroupRef group_ref) {
-  for (size_t i = 0; i < pending_class_decls_.size(); ++i)
-    CheckTag(pending_class_decls_[i]);
-  pending_class_decls_.clear();
-
-  return true;  // true means continue parsing.
-}
-
 void ChromeClassTester::CheckTag(TagDecl* tag) {
   // We handle class types here where we have semantic information. We can only
   // check structs/classes/enums here, but we get a bunch of nice semantic
diff --git a/tools/clang/plugins/ChromeClassTester.h b/tools/clang/plugins/ChromeClassTester.h
index e3d7e3c..2dcfb42 100644
--- a/tools/clang/plugins/ChromeClassTester.h
+++ b/tools/clang/plugins/ChromeClassTester.h
@@ -15,16 +15,13 @@
 
 // A class on top of ASTConsumer that forwards classes defined in Chromium
 // headers to subclasses which implement CheckChromeClass().
-class ChromeClassTester : public clang::ASTConsumer {
+// TODO(vmpstr): Fold this class into FindBadConstructsConsumer.
+class ChromeClassTester {
  public:
   ChromeClassTester(clang::CompilerInstance& instance,
                     const chrome_checker::Options& options);
   virtual ~ChromeClassTester();
 
-  // clang::ASTConsumer:
-  virtual void HandleTagDeclDefinition(clang::TagDecl* tag);
-  virtual bool HandleTopLevelDecl(clang::DeclGroupRef group_ref);
-
   void CheckTag(clang::TagDecl*);
 
   clang::DiagnosticsEngine::Level getErrorLevel();
@@ -96,9 +93,6 @@ class ChromeClassTester : public clang::ASTConsumer {
 
   // List of types that we don't check.
   std::set<std::string> ignored_record_names_;
-
-  // List of decls to check once the current top-level decl is parsed.
-  std::vector<clang::TagDecl*> pending_class_decls_;
 };
 
 #endif  // TOOLS_CLANG_PLUGINS_CHROMECLASSTESTER_H_
diff --git a/tools/clang/plugins/FindBadConstructsAction.cpp b/tools/clang/plugins/FindBadConstructsAction.cpp
index 6762297..e8f103a 100644
--- a/tools/clang/plugins/FindBadConstructsAction.cpp
+++ b/tools/clang/plugins/FindBadConstructsAction.cpp
@@ -36,9 +36,7 @@ FindBadConstructsAction::FindBadConstructsAction() {
 std::unique_ptr<ASTConsumer> FindBadConstructsAction::CreateASTConsumer(
     CompilerInstance& instance,
     llvm::StringRef ref) {
-  if (options_.with_ast_visitor)
-    return llvm::make_unique<PluginConsumer>(&instance, options_);
-  return llvm::make_unique<FindBadConstructsConsumer>(instance, options_);
+  return llvm::make_unique<PluginConsumer>(&instance, options_);
 }
 
 bool FindBadConstructsAction::ParseArgs(const CompilerInstance& instance,
@@ -57,12 +55,12 @@ bool FindBadConstructsAction::ParseArgs(const CompilerInstance& instance,
       // TODO(tsepez): Enable this by default once http://crbug.com/356815
       // and http://crbug.com/356816 are fixed.
       options_.check_enum_last_value = true;
-    } else if (args[i] == "with-ast-visitor") {
-      options_.with_ast_visitor = true;
     } else if (args[i] == "check-templates") {
       options_.check_templates = true;
     } else if (args[i] == "follow-macro-expansion") {
       options_.follow_macro_expansion = true;
+    } else if (args[i] == "check-implicit-copy-ctors") {
+      options_.check_implicit_copy_ctors = true;
     } else {
       parsed = false;
       llvm::errs() << "Unknown clang plugin argument: " << args[i] << "\n";
diff --git a/tools/clang/plugins/FindBadConstructsConsumer.cpp b/tools/clang/plugins/FindBadConstructsConsumer.cpp
index c79a764..b7e1886 100644
--- a/tools/clang/plugins/FindBadConstructsConsumer.cpp
+++ b/tools/clang/plugins/FindBadConstructsConsumer.cpp
@@ -288,7 +288,7 @@ void FindBadConstructsConsumer::CheckCtorDtorWeight(
         // The current check is buggy. An implicit copy constructor does not
         // have an inline body, so this check never fires for classes with a
         // user-declared out-of-line constructor.
-        if (it->hasInlineBody()) {
+        if (it->hasInlineBody() && options_.check_implicit_copy_ctors) {
           if (it->isCopyConstructor() &&
               !record->hasUserDeclaredCopyConstructor()) {
             // In general, implicit constructors are generated on demand.  But
diff --git a/tools/clang/plugins/Options.h b/tools/clang/plugins/Options.h
index bb5857f..58c66f4 100644
--- a/tools/clang/plugins/Options.h
+++ b/tools/clang/plugins/Options.h
@@ -8,21 +8,15 @@
 namespace chrome_checker {
 
 struct Options {
-  Options()
-      : check_base_classes(false),
-        enforce_in_pdf(false),
-        enforce_in_thirdparty_webkit(false),
-        check_enum_last_value(false),
-        with_ast_visitor(false),
-        check_templates(false) {}
-
-  bool check_base_classes;
-  bool enforce_in_pdf;
-  bool enforce_in_thirdparty_webkit;  // Use in Blink code itself
-  bool check_enum_last_value;
-  bool with_ast_visitor;
-  bool check_templates;
+  bool check_base_classes = false;
+  bool enforce_in_pdf = false;
+  bool enforce_in_thirdparty_webkit = false;  // Use in Blink code itself
+  bool check_enum_last_value = false;
+  bool check_templates = false;
   bool follow_macro_expansion = false;
+  // This is needed during the migration from ASTConsumer approach to the
+  // RecursiveASTVisitor approach. See https://crbug.com/436357 for details.
+  bool check_implicit_copy_ctors = false;
 };
 
 }  // namespace chrome_checker
diff --git a/tools/clang/plugins/tests/test.py b/tools/clang/plugins/tests/test.py
index 30721e9..3e8b94b 100755
--- a/tools/clang/plugins/tests/test.py
+++ b/tools/clang/plugins/tests/test.py
@@ -19,7 +19,7 @@ class ChromeStylePluginTest(plugin_testing.ClangPluginTest):
   """Test harness for the Chrome style plugin."""
 
   def AdjustClangArguments(self, clang_cmd):
-    self.AddPluginArg(clang_cmd, 'with-ast-visitor')
+    self.AddPluginArg(clang_cmd, 'check-implicit-copy-ctors')
     self.AddPluginArg(clang_cmd, 'follow-macro-expansion')
     clang_cmd.extend([
         # Skip code generation