Clang style plugin: add warn-only option and use it on Windows

This will allow us to flip a flag to enable warnings-as-errors once
the Windows code is cleaned up enough without building a new plugin
binary.

Also, try not to compute the diagnostic level in multiple places.

BUG=467287, 483065

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

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

10 files changed, 67 insertions, 29 deletions

diff --git a/tools/clang/plugins/ChromeClassTester.cpp b/tools/clang/plugins/ChromeClassTester.cpp
index 3744ef3..6a4233a 100644
--- a/tools/clang/plugins/ChromeClassTester.cpp
+++ b/tools/clang/plugins/ChromeClassTester.cpp
@@ -18,6 +18,7 @@
 #endif
 
 using namespace clang;
+using chrome_checker::Options;
 
 namespace {
 
@@ -30,8 +31,10 @@ bool ends_with(const std::string& one, const std::string& two) {
 
 }  // namespace
 
-ChromeClassTester::ChromeClassTester(CompilerInstance& instance)
-    : instance_(instance),
+ChromeClassTester::ChromeClassTester(CompilerInstance& instance,
+                                     const Options& options)
+    : options_(options),
+      instance_(instance),
       diagnostic_(instance.getDiagnostics()) {
   BuildBannedLists();
 }
@@ -95,16 +98,13 @@ void ChromeClassTester::emitWarning(SourceLocation loc,
   std::string err;
   err = "[chromium-style] ";
   err += raw_error;
-  // TODO(dcheng): Re-enable -Werror for these diagnostics on Windows once all
-  // the pre-existing warnings are cleaned up. https://crbug.com/467287
-  DiagnosticIDs::Level level =
-#if !defined(LLVM_ON_WIN32)
-      diagnostic().getWarningsAsErrors() ?
-      DiagnosticIDs::Error :
-#endif
-      DiagnosticIDs::Warning;
+
+  DiagnosticIDs::Level level = getErrorLevel() == DiagnosticsEngine::Error
+      ? DiagnosticIDs::Error : DiagnosticIDs::Warning;
+
   unsigned id = diagnostic().getDiagnosticIDs()->getCustomDiagID(level, err);
   DiagnosticBuilder builder = diagnostic().Report(full, id);
+
 }
 
 bool ChromeClassTester::InBannedDirectory(SourceLocation loc) {
@@ -311,3 +311,11 @@ bool ChromeClassTester::GetFilename(SourceLocation loc,
   *filename = ploc.getFilename();
   return true;
 }
+
+DiagnosticsEngine::Level ChromeClassTester::getErrorLevel() {
+  if (options_.warn_only)
+    return DiagnosticsEngine::Warning;
+
+  return diagnostic().getWarningsAsErrors() ? DiagnosticsEngine::Error
+                                            : DiagnosticsEngine::Warning;
+}
diff --git a/tools/clang/plugins/ChromeClassTester.h b/tools/clang/plugins/ChromeClassTester.h
index ed65050..6b5cdf3 100644
--- a/tools/clang/plugins/ChromeClassTester.h
+++ b/tools/clang/plugins/ChromeClassTester.h
@@ -8,6 +8,7 @@
 #include <set>
 #include <vector>
 
+#include "Options.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -16,7 +17,8 @@
 // headers to subclasses which implement CheckChromeClass().
 class ChromeClassTester : public clang::ASTConsumer {
  public:
-  explicit ChromeClassTester(clang::CompilerInstance& instance);
+  ChromeClassTester(clang::CompilerInstance& instance,
+                    const chrome_checker::Options& options);
   virtual ~ChromeClassTester();
 
   // clang::ASTConsumer:
@@ -25,6 +27,8 @@ class ChromeClassTester : public clang::ASTConsumer {
 
   void CheckTag(clang::TagDecl*);
 
+  clang::DiagnosticsEngine::Level getErrorLevel();
+
  protected:
   clang::CompilerInstance& instance() { return instance_; }
   clang::DiagnosticsEngine& diagnostic() { return diagnostic_; }
@@ -50,6 +54,9 @@ class ChromeClassTester : public clang::ASTConsumer {
   // implementation (.cc, .cpp, .mm) file.
   bool InImplementationFile(clang::SourceLocation location);
 
+  // Options.
+  const chrome_checker::Options options_;
+
  private:
   void BuildBannedLists();
 
diff --git a/tools/clang/plugins/FindBadConstructsAction.cpp b/tools/clang/plugins/FindBadConstructsAction.cpp
index 6c39d72..8125084 100644
--- a/tools/clang/plugins/FindBadConstructsAction.cpp
+++ b/tools/clang/plugins/FindBadConstructsAction.cpp
@@ -57,6 +57,8 @@ bool FindBadConstructsAction::ParseArgs(const CompilerInstance& instance,
       options_.with_ast_visitor = true;
     } else if (args[i] == "check-templates") {
       options_.check_templates = true;
+    } else if (args[i] == "warn-only") {
+      options_.warn_only = 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 20919a4..4dc305e 100644
--- a/tools/clang/plugins/FindBadConstructsConsumer.cpp
+++ b/tools/clang/plugins/FindBadConstructsConsumer.cpp
@@ -101,7 +101,7 @@ bool IsPodOrTemplateType(const CXXRecordDecl& record) {
 
 FindBadConstructsConsumer::FindBadConstructsConsumer(CompilerInstance& instance,
                                                      const Options& options)
-    : ChromeClassTester(instance), options_(options) {
+    : ChromeClassTester(instance, options) {
   // Messages for virtual method specifiers.
   diag_method_requires_override_ =
       diagnostic().getCustomDiagID(getErrorLevel(), kMethodRequiresOverride);
@@ -593,19 +593,6 @@ FindBadConstructsConsumer::CheckRecordForRefcountIssue(
   return None;
 }
 
-// Adds either a warning or error, based on the current handling of
-// -Werror.
-DiagnosticsEngine::Level FindBadConstructsConsumer::getErrorLevel() {
-#if defined(LLVM_ON_WIN32)
-  // TODO(dcheng): Re-enable -Werror for these diagnostics on Windows once all
-  // the pre-existing warnings are cleaned up. https://crbug.com/467287
-  return DiagnosticsEngine::Warning;
-#else
-  return diagnostic().getWarningsAsErrors() ? DiagnosticsEngine::Error
-                                            : DiagnosticsEngine::Warning;
-#endif
-}
-
 // Returns true if |base| specifies one of the Chromium reference counted
 // classes (base::RefCounted / base::RefCountedThreadSafe).
 bool FindBadConstructsConsumer::IsRefCountedCallback(
diff --git a/tools/clang/plugins/FindBadConstructsConsumer.h b/tools/clang/plugins/FindBadConstructsConsumer.h
index 6e2b2c9..c52a8f3 100644
--- a/tools/clang/plugins/FindBadConstructsConsumer.h
+++ b/tools/clang/plugins/FindBadConstructsConsumer.h
@@ -71,7 +71,6 @@ class FindBadConstructsConsumer
   static RefcountIssue CheckRecordForRefcountIssue(
       const clang::CXXRecordDecl* record,
       clang::SourceLocation& loc);
-  clang::DiagnosticsEngine::Level getErrorLevel();
   static bool IsRefCountedCallback(const clang::CXXBaseSpecifier* base,
                                    clang::CXXBasePath& path,
                                    void* user_data);
@@ -86,8 +85,6 @@ class FindBadConstructsConsumer
   void CheckWeakPtrFactoryMembers(clang::SourceLocation record_location,
                                   clang::CXXRecordDecl* record);
 
-  const Options options_;
-
   unsigned diag_method_requires_override_;
   unsigned diag_redundant_virtual_specifier_;
   unsigned diag_base_method_virtual_and_final_;
diff --git a/tools/clang/plugins/Options.h b/tools/clang/plugins/Options.h
index 112ec10..5131905 100644
--- a/tools/clang/plugins/Options.h
+++ b/tools/clang/plugins/Options.h
@@ -12,12 +12,14 @@ struct Options {
       : check_base_classes(false),
         check_enum_last_value(false),
         with_ast_visitor(false),
-        check_templates(false) {}
+        check_templates(false),
+        warn_only(false) {}
 
   bool check_base_classes;
   bool check_enum_last_value;
   bool with_ast_visitor;
   bool check_templates;
+  bool warn_only;
 };
 
 }  // namespace chrome_checker
diff --git a/tools/clang/plugins/tests/warn_only.cpp b/tools/clang/plugins/tests/warn_only.cpp
new file mode 100644
index 0000000..1fbefcb
--- /dev/null
+++ b/tools/clang/plugins/tests/warn_only.cpp
@@ -0,0 +1,5 @@
+// Copyright (c) 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 "warn_only.h"
diff --git a/tools/clang/plugins/tests/warn_only.flags b/tools/clang/plugins/tests/warn_only.flags
new file mode 100644
index 0000000..01e7b3b
--- /dev/null
+++ b/tools/clang/plugins/tests/warn_only.flags
@@ -0,0 +1 @@
+-Werror -Xclang -plugin-arg-find-bad-constructs -Xclang warn-only
diff --git a/tools/clang/plugins/tests/warn_only.h b/tools/clang/plugins/tests/warn_only.h
new file mode 100644
index 0000000..fcc0aa1
--- /dev/null
+++ b/tools/clang/plugins/tests/warn_only.h
@@ -0,0 +1,21 @@
+// Copyright (c) 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 WARN_ONLY_H_
+#define WARN_ONLY_H_
+
+#include <string>
+#include <vector>
+
+class InlineCtors {
+ public:
+  InlineCtors() {}
+  ~InlineCtors() {}
+
+ private:
+  std::vector<int> one_;
+  std::vector<std::string> two_;
+};
+
+#endif  // WARN_ONLY_H_
diff --git a/tools/clang/plugins/tests/warn_only.txt b/tools/clang/plugins/tests/warn_only.txt
new file mode 100644
index 0000000..4b5c2d0
--- /dev/null
+++ b/tools/clang/plugins/tests/warn_only.txt
@@ -0,0 +1,8 @@
+In file included from warn_only.cpp:5:
+./warn_only.h:13:3: warning: [chromium-style] Complex constructor has an inlined body.
+  InlineCtors() {}
+  ^
+./warn_only.h:14:3: warning: [chromium-style] Complex destructor has an inline body.
+  ~InlineCtors() {}
+  ^
+2 warnings generated.