[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: [PATCH] D53953: [clang-tidy] Get ClangTidyContext out of the business of storing diagnostics. NFC
From: Sam McCall via Phabricator via cfe-commits <cfe-commits () lists ! llvm ! org>
Date: 2018-10-31 22:59:56
Message-ID: 5099bb9ef2c6baa9704832a698801ba2 () localhost ! localdomain
[Download RAW message or body]
sammccall updated this revision to Diff 172039.
sammccall added a comment.
Remove backwards-compat behavior around ClangdDiagnosticConsumer::finish(),=
it's presumably a bug
Update test to verify we fail but don't crash in that case.
auto -> real type
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53953
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tidy/tool/ClangTidyMain.cpp
test/clang-tidy/clang-tidy-run-with-database.cpp
unittests/clang-tidy/ClangTidyTest.h
["D53953.172039.patch" (text/x-patch)]
Index: unittests/clang-tidy/ClangTidyTest.h
===================================================================
--- unittests/clang-tidy/ClangTidyTest.h
+++ unittests/clang-tidy/ClangTidyTest.h
@@ -118,15 +118,15 @@
Invocation.setDiagnosticConsumer(&DiagConsumer);
if (!Invocation.run()) {
std::string ErrorText;
- for (const auto &Error : Context.getErrors()) {
+ for (const auto &Error : DiagConsumer.take()) {
ErrorText += Error.Message.Message + "\n";
}
llvm::report_fatal_error(ErrorText);
}
- DiagConsumer.finish();
tooling::Replacements Fixes;
- for (const ClangTidyError &Error : Context.getErrors()) {
+ std::vector<ClangTidyError> Diags = DiagConsumer.take();
+ for (const ClangTidyError &Error : Diags) {
for (const auto &FileAndFixes : Error.Fix) {
for (const auto &Fix : FileAndFixes.second) {
auto Err = Fixes.add(Fix);
@@ -139,7 +139,7 @@
}
}
if (Errors)
- *Errors = Context.getErrors();
+ *Errors = std::move(Diags);
auto Result = tooling::applyAllReplacements(Code, Fixes);
if (!Result) {
// FIXME: propogate the error.
Index: test/clang-tidy/clang-tidy-run-with-database.cpp
===================================================================
--- test/clang-tidy/clang-tidy-run-with-database.cpp
+++ test/clang-tidy/clang-tidy-run-with-database.cpp
@@ -8,7 +8,13 @@
// RUN: echo 'int *HP = 0;' > %T/compilation-database-test/include/header.h
// RUN: echo '#include "header.h"' > %T/compilation-database-test/b/d.cpp
// RUN: sed 's|test_dir|%/T/compilation-database-test|g' \
%S/Inputs/compilation-database/template.json > \
%T/compile_commands.json
-// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T \
%T/compilation-database-test/b/not-exist -header-filter=.* +
+// Regression test: shouldn't crash.
+// RUN: not clang-tidy --checks=-*,modernize-use-nullptr -p %T \
%T/compilation-database-test/b/not-exist -header-filter=.* 2>&1 | FileCheck %s \
-check-prefix=CHECK-NOT-EXIST +// CHECK-NOT-EXIST: Error while processing \
{{.*}}/not-exist. +// CHECK-NOT-EXIST: unable to handle compilation
+// CHECK-NOT-EXIST: Found compiler error
+
// RUN: clang-tidy --checks=-*,modernize-use-nullptr -p %T \
%T/compilation-database-test/a/a.cpp %T/compilation-database-test/a/b.cpp \
%T/compilation-database-test/b/b.cpp %T/compilation-database-test/b/c.cpp \
%T/compilation-database-test/b/d.cpp -header-filter=.* -fix // RUN: FileCheck \
-input-file=%T/compilation-database-test/a/a.cpp %s -check-prefix=CHECK-FIX1 // RUN: \
FileCheck -input-file=%T/compilation-database-test/a/b.cpp %s \
-check-prefix=CHECK-FIX2
Index: clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -422,9 +422,9 @@
ClangTidyContext Context(std::move(OwningOptionsProvider),
AllowEnablingAnalyzerAlphaCheckers);
- runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
- EnableCheckProfile, ProfilePrefix);
- ArrayRef<ClangTidyError> Errors = Context.getErrors();
+ std::vector<ClangTidyError> Errors =
+ runClangTidy(Context, OptionsParser.getCompilations(), PathList, BaseFS,
+ EnableCheckProfile, ProfilePrefix);
bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
return E.DiagLevel == ClangTidyError::Error;
}) != Errors.end();
@@ -434,7 +434,7 @@
unsigned WErrorCount = 0;
// -fix-errors implies -fix.
- handleErrors(Context, (FixErrors || Fix) && !DisableFixes, WErrorCount,
+ handleErrors(Errors, Context, (FixErrors || Fix) && !DisableFixes, WErrorCount,
BaseFS);
if (!ExportFixes.empty() && !Errors.empty()) {
Index: clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -160,12 +160,6 @@
/// counters.
const ClangTidyStats &getStats() const { return Stats; }
- /// \brief Returns all collected errors.
- ArrayRef<ClangTidyError> getErrors() const { return Errors; }
-
- /// \brief Clears collected errors.
- void clearErrors() { Errors.clear(); }
-
/// \brief Control profile collection in clang-tidy.
void setEnableProfiling(bool Profile);
bool getEnableProfiling() const { return Profile; }
@@ -203,7 +197,6 @@
/// \brief Store an \p Error.
void storeError(const ClangTidyError &Error);
- std::vector<ClangTidyError> Errors;
DiagnosticsEngine *DiagEngine;
std::unique_ptr<ClangTidyOptionsProvider> OptionsProvider;
@@ -243,13 +236,12 @@
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const Diagnostic &Info) override;
- /// \brief Flushes the internal diagnostics buffer to the ClangTidyContext.
- void finish() override;
+ // Retrieve the diagnostics that were captured.
+ std::vector<ClangTidyError> take();
private:
void finalizeLastError();
-
- void removeIncompatibleErrors(SmallVectorImpl<ClangTidyError> &Errors) const;
+ void removeIncompatibleErrors();
/// \brief Returns the \c HeaderFilter constructed for the options set in the
/// context.
@@ -263,7 +255,8 @@
ClangTidyContext &Context;
bool RemoveIncompatibleErrors;
std::unique_ptr<DiagnosticsEngine> Diags;
- SmallVector<ClangTidyError, 8> Errors;
+ std::vector<ClangTidyError> Errors;
+ std::vector<ClangTidyError> FinalizedErrors;
std::unique_ptr<llvm::Regex> HeaderFilter;
bool LastErrorRelatesToUserCode;
bool LastErrorPassesLineFilter;
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -259,11 +259,6 @@
return WarningAsErrorFilter->contains(CheckName);
}
-/// \brief Store a \c ClangTidyError.
-void ClangTidyContext::storeError(const ClangTidyError &Error) {
- Errors.push_back(Error);
-}
-
StringRef ClangTidyContext::getCheckName(unsigned DiagnosticID) const {
llvm::DenseMap<unsigned, std::string>::const_iterator I =
CheckNamesByDiagnosticID.find(DiagnosticID);
@@ -534,8 +529,7 @@
return HeaderFilter.get();
}
-void ClangTidyDiagnosticConsumer::removeIncompatibleErrors(
- SmallVectorImpl<ClangTidyError> &Errors) const {
+void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() {
// Each error is modelled as the set of intervals in which it applies
// replacements. To detect overlapping replacements, we use a sweep line
// algorithm over these sets of intervals.
@@ -673,18 +667,13 @@
};
} // end anonymous namespace
-// Flushes the internal diagnostics buffer to the ClangTidyContext.
-void ClangTidyDiagnosticConsumer::finish() {
+std::vector<ClangTidyError> ClangTidyDiagnosticConsumer::take() {
finalizeLastError();
std::sort(Errors.begin(), Errors.end(), LessClangTidyError());
Errors.erase(std::unique(Errors.begin(), Errors.end(), EqualClangTidyError()),
Errors.end());
-
if (RemoveIncompatibleErrors)
- removeIncompatibleErrors(Errors);
-
- for (const ClangTidyError &Error : Errors)
- Context.storeError(Error);
- Errors.clear();
+ removeIncompatibleErrors();
+ return std::move(Errors);
}
Index: clang-tidy/ClangTidy.h
===================================================================
--- clang-tidy/ClangTidy.h
+++ clang-tidy/ClangTidy.h
@@ -230,20 +230,22 @@
/// \param StoreCheckProfile If provided, and EnableCheckProfile is true,
/// the profile will not be output to stderr, but will instead be stored
/// as a JSON file in the specified directory.
-void runClangTidy(clang::tidy::ClangTidyContext &Context,
- const tooling::CompilationDatabase &Compilations,
- ArrayRef<std::string> InputFiles,
- llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS,
- bool EnableCheckProfile = false,
- llvm::StringRef StoreCheckProfile = StringRef());
+std::vector<ClangTidyError>
+runClangTidy(clang::tidy::ClangTidyContext &Context,
+ const tooling::CompilationDatabase &Compilations,
+ ArrayRef<std::string> InputFiles,
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS,
+ bool EnableCheckProfile = false,
+ llvm::StringRef StoreCheckProfile = StringRef());
// FIXME: This interface will need to be significantly extended to be useful.
// FIXME: Implement confidence levels for displaying/fixing errors.
//
/// \brief Displays the found \p Errors to the users. If \p Fix is true, \p
/// Errors containing fixes are automatically applied and reformatted. If no
/// clang-format configuration file is found, the given \P FormatStyle is used.
-void handleErrors(ClangTidyContext &Context, bool Fix,
+void handleErrors(llvm::ArrayRef<ClangTidyError> Diags,
+ ClangTidyContext &Context, bool Fix,
unsigned &WarningsAsErrorsCount,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS);
Index: clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -500,11 +500,12 @@
return Factory.getCheckOptions();
}
-void runClangTidy(clang::tidy::ClangTidyContext &Context,
- const CompilationDatabase &Compilations,
- ArrayRef<std::string> InputFiles,
- llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS,
- bool EnableCheckProfile, llvm::StringRef StoreCheckProfile) {
+std::vector<ClangTidyError>
+runClangTidy(clang::tidy::ClangTidyContext &Context,
+ const CompilationDatabase &Compilations,
+ ArrayRef<std::string> InputFiles,
+ llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS,
+ bool EnableCheckProfile, llvm::StringRef StoreCheckProfile) {
ClangTool Tool(Compilations, InputFiles,
std::make_shared<PCHContainerOperations>(), BaseFS);
@@ -586,9 +587,11 @@
ActionFactory Factory(Context);
Tool.run(&Factory);
+ return DiagConsumer.take();
}
-void handleErrors(ClangTidyContext &Context, bool Fix,
+void handleErrors(llvm::ArrayRef<ClangTidyError> Diags,
+ ClangTidyContext &Context, bool Fix,
unsigned &WarningsAsErrorsCount,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) {
ErrorReporter Reporter(Context, Fix, BaseFS);
@@ -598,7 +601,7 @@
if (!InitialWorkingDir)
llvm::report_fatal_error("Cannot get current working path.");
- for (const ClangTidyError &Error : Context.getErrors()) {
+ for (const ClangTidyError &Error : Diags) {
if (!Error.BuildDirectory.empty()) {
// By default, the working directory of file system is the current
// clang-tidy running directory.
[Attachment #4 (text/plain)]
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic