From 348cae8e2b5122dc3af5886afa5b6f094c260e40 Mon Sep 17 00:00:00 2001 From: Alexander Kornienko Date: Mon, 2 Jun 2014 20:44:32 +0000 Subject: [PATCH] Never filter-out compile errors in clang-tidy, display them as errors. Summary: No filters should affect the display of errors. Fixed a few tests, which had compile errors. We need to think what we should do with mapped errors (-Werror). Reviewers: klimek Reviewed By: klimek Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D3982 llvm-svn: 210044 --- clang-tools-extra/clang-tidy/ClangTidy.cpp | 5 +++-- .../clang-tidy/ClangTidyDiagnosticConsumer.cpp | 16 +++++++++++++--- .../clang-tidy/ClangTidyDiagnosticConsumer.h | 9 ++++++++- clang-tools-extra/test/clang-tidy/deduplication.cpp | 8 ++++---- clang-tools-extra/test/clang-tidy/diagnostic.cpp | 6 +++--- .../test/clang-tidy/redundant-smartptr-get.cpp | 5 +++-- 6 files changed, 34 insertions(+), 15 deletions(-) diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 57bfc5e..65c10f4 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -336,8 +336,9 @@ ClangTidyStats runClangTidy(const ClangTidyOptions &Options, void handleErrors(const std::vector &Errors, bool Fix) { ErrorReporter Reporter(Fix); for (const ClangTidyError &Error : Errors) { - Reporter.reportDiagnostic(Error.Message, DiagnosticsEngine::Warning, - &Error.Fix); + Reporter.reportDiagnostic( + Error.Message, static_cast(Error.DiagLevel), + &Error.Fix); for (const ClangTidyMessage &Note : Error.Notes) Reporter.reportDiagnostic(Note, DiagnosticsEngine::Note); } diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index a61b8fc..625f92ae 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -109,7 +109,9 @@ ClangTidyMessage::ClangTidyMessage(StringRef Message, FileOffset = Sources.getFileOffset(Loc); } -ClangTidyError::ClangTidyError(StringRef CheckName) : CheckName(CheckName) {} +ClangTidyError::ClangTidyError(StringRef CheckName, + ClangTidyError::Level DiagLevel) + : CheckName(CheckName), DiagLevel(DiagLevel) {} // Returns true if GlobList starts with the negative indicator ('-'), removes it // from the GlobList. @@ -214,7 +216,8 @@ ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx) void ClangTidyDiagnosticConsumer::finalizeLastError() { if (!Errors.empty()) { ClangTidyError &Error = Errors.back(); - if (!Context.getChecksFilter().isCheckEnabled(Error.CheckName)) { + if (!Context.getChecksFilter().isCheckEnabled(Error.CheckName) && + Error.DiagLevel != ClangTidyError::Error) { ++Context.Stats.ErrorsIgnoredCheckFilter; Errors.pop_back(); } else if (!LastErrorRelatesToUserCode) { @@ -246,7 +249,14 @@ void ClangTidyDiagnosticConsumer::HandleDiagnostic( ? ("clang-diagnostic-" + WarningOption).str() : Context.getCheckName(Info.getID()).str(); - Errors.push_back(ClangTidyError(CheckName)); + ClangTidyError::Level Level = ClangTidyError::Warning; + if (DiagLevel == DiagnosticsEngine::Error || + DiagLevel == DiagnosticsEngine::Fatal) { + Level = ClangTidyError::Error; + LastErrorRelatesToUserCode = true; + LastErrorPassesLineFilter = true; + } + Errors.push_back(ClangTidyError(CheckName, Level)); } // FIXME: Provide correct LangOptions for each file. diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h index 1240ab9..926a3b4 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -49,12 +49,19 @@ struct ClangTidyMessage { /// /// FIXME: Make Diagnostics flexible enough to support this directly. struct ClangTidyError { - ClangTidyError(StringRef CheckName); + enum Level { + Warning = DiagnosticsEngine::Warning, + Error = DiagnosticsEngine::Error + }; + + ClangTidyError(StringRef CheckName, Level DiagLevel); std::string CheckName; ClangTidyMessage Message; tooling::Replacements Fix; SmallVector Notes; + + Level DiagLevel; }; /// \brief Filters checks by name. diff --git a/clang-tools-extra/test/clang-tidy/deduplication.cpp b/clang-tools-extra/test/clang-tidy/deduplication.cpp index 1723dac..36334ca 100644 --- a/clang-tools-extra/test/clang-tidy/deduplication.cpp +++ b/clang-tools-extra/test/clang-tidy/deduplication.cpp @@ -1,12 +1,12 @@ // RUN: clang-tidy -checks='-*,google-explicit-constructor' %s -- | FileCheck %s template -class A { A(T); }; -// CHECK: :[[@LINE-1]]:11: warning: Single-argument constructors must be explicit [google-explicit-constructor] +struct A { A(T); }; +// CHECK: :[[@LINE-1]]:12: warning: Single-argument constructors must be explicit [google-explicit-constructor] // CHECK-NOT: warning: void f() { - A a; - A b; + A a(0); + A b(0); } diff --git a/clang-tools-extra/test/clang-tidy/diagnostic.cpp b/clang-tools-extra/test/clang-tidy/diagnostic.cpp index c10e493..86e5ede 100644 --- a/clang-tools-extra/test/clang-tidy/diagnostic.cpp +++ b/clang-tools-extra/test/clang-tidy/diagnostic.cpp @@ -1,4 +1,4 @@ -// RUN: clang-tidy %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 %s +// RUN: clang-tidy -checks='-*,misc-use-override' %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 %s // RUN: clang-tidy -checks='google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 %s // RUN: clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK3 %s // RUN: clang-tidy -checks='-*,misc-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE | FileCheck -check-prefix=CHECK4 %s @@ -7,8 +7,8 @@ // CHECK2-NOT: warning // CHECK3-NOT: warning -// CHECK1: warning: error reading '{{.*}}.nonexistent.cpp' -// CHECK2: warning: unknown argument: '-fan-unknown-option' +// CHECK1: error: error reading '{{.*}}.nonexistent.cpp' +// CHECK2: error: unknown argument: '-fan-unknown-option' // CHECK2: :[[@LINE+2]]:9: warning: implicit conversion from 'double' to 'int' changes value // CHECK3: :[[@LINE+1]]:9: warning: implicit conversion from 'double' to 'int' changes value diff --git a/clang-tools-extra/test/clang-tidy/redundant-smartptr-get.cpp b/clang-tools-extra/test/clang-tidy/redundant-smartptr-get.cpp index 22decca..a492306 100644 --- a/clang-tools-extra/test/clang-tidy/redundant-smartptr-get.cpp +++ b/clang-tools-extra/test/clang-tidy/redundant-smartptr-get.cpp @@ -6,20 +6,21 @@ namespace std { template -class unique_ptr { +struct unique_ptr { T& operator*() const; T* operator->() const; T* get() const; }; template -class shared_ptr { +struct shared_ptr { T& operator*() const; T* operator->() const; T* get() const; }; } // namespace std +#define NULL __null struct int_ptr { int* get(); -- 2.7.4