njames93 updated this revision to Diff 308153.
njames93 added a comment.
Fix documentation.
Ping??
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84924/new/
https://reviews.llvm.org/D84924
Files:
clang-tools-extra/clang-tidy/ClangTidy.cpp
clang-tools-extra/clang-tidy/ClangTidy.h
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/index.rst
clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
Index: clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/alternative-fixes.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s "llvm-namespace-comment,clang-diagnostic-*" %t
+// RUN: %check_clang_tidy %s "llvm-namespace-comment,clang-diagnostic-*" %t -- -fix-notes
void foo(int a) {
if (a = 1) {
// CHECK-NOTES: [[@LINE-1]]:9: warning: using the result of an assignment as a condition without parentheses [clang-diagnostic-parentheses]
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-using-decls.cpp
@@ -81,7 +81,6 @@
// eol-comments aren't removed (yet)
using n::A; // A
// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: using decl 'A' is unused
-// CHECK-MESSAGES: :[[@LINE-2]]:10: note: remove the using
// CHECK-FIXES: {{^}}// A
using n::B;
using n::C;
Index: clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-definitions-in-headers.hpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t -- -fix-notes
int f() {
// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers]
Index: clang-tools-extra/docs/clang-tidy/index.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -171,6 +171,10 @@
errors were found. If compiler errors have
attached fix-its, clang-tidy will apply them as
well.
+ --fix-notes -
+ If a warning has no fix, but has notes attached
+ which contain fixes, apply the first fix found
+ in any notes.
--format-style=<string> -
Style for formatting code around applied fixes:
- 'none' (default) turns off formatting
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,9 @@
Improvements to clang-tidy
--------------------------
+ - Added command line option `-fix-notes` to apply fixes found in notes
+ attached to warnings.
+
- Checks that allow configuring names of headers to include now support wrapping
the include in angle brackets to create a system include. For example,
:doc:`cppcoreguidelines-init-variables
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===================================================================
--- clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
@@ -127,6 +127,13 @@
)"),
cl::init(false), cl::cat(ClangTidyCategory));
+static cl::opt<bool> FixNotes("fix-notes", cl::desc(R"(
+If a warning has no fix, but has notes attached
+which contain fixes, apply the first fix found
+in any notes.
+)"),
+ cl::init(false), cl::cat(ClangTidyCategory));
+
static cl::opt<std::string> FormatStyle("format-style", cl::desc(R"(
Style for formatting code around applied fixes:
- 'none' (default) turns off formatting
@@ -481,18 +488,18 @@
AllowEnablingAnalyzerAlphaCheckers);
std::vector<ClangTidyError> Errors =
runClangTidy(Context, OptionsParser->getCompilations(), PathList, BaseFS,
- EnableCheckProfile, ProfilePrefix);
+ FixNotes, EnableCheckProfile, ProfilePrefix);
bool FoundErrors = llvm::find_if(Errors, [](const ClangTidyError &E) {
return E.DiagLevel == ClangTidyError::Error;
}) != Errors.end();
- const bool DisableFixes = Fix && FoundErrors && !FixErrors;
+ const bool DisableFixes = (Fix || FixNotes) && FoundErrors && !FixErrors;
unsigned WErrorCount = 0;
// -fix-errors implies -fix.
- handleErrors(Errors, Context, (FixErrors || Fix) && !DisableFixes, WErrorCount,
- BaseFS);
+ handleErrors(Errors, Context, (FixErrors || Fix || FixNotes) && !DisableFixes,
+ FixNotes, WErrorCount, BaseFS);
if (!ExportFixes.empty() && !Errors.empty()) {
std::error_code EC;
Index: clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
@@ -219,21 +219,12 @@
return ParamInfo.SourceName;
};
- auto ParamDiag =
- Check->diag(Location,
- "differing parameters are named here: (%0), in %1: (%2)",
- DiagnosticIDs::Level::Note)
+ Check->diag(Location,
+ "differing parameters are named here: (%0), in %1: (%2)",
+ DiagnosticIDs::Level::Note)
<< joinParameterNames(DifferingParams, ChooseOtherName)
<< OtherDeclarationDescription
<< joinParameterNames(DifferingParams, ChooseSourceName);
-
- for (const DifferingParamInfo &ParamInfo : DifferingParams) {
- if (ParamInfo.GenerateFixItHint) {
- ParamDiag << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(ParamInfo.OtherNameRange),
- ParamInfo.SourceName);
- }
- }
}
void formatDiagnosticsForDeclarations(
@@ -241,11 +232,24 @@
const FunctionDecl *ParameterSourceDeclaration,
const FunctionDecl *OriginalDeclaration,
const InconsistentDeclarationsContainer &InconsistentDeclarations) {
- Check->diag(
- OriginalDeclaration->getLocation(),
- "function %q0 has %1 other declaration%s1 with different parameter names")
- << OriginalDeclaration
- << static_cast<int>(InconsistentDeclarations.size());
+ {
+ auto MainDiag = Check->diag(OriginalDeclaration->getLocation(),
+ "function %q0 has %1 other declaration%s1 with "
+ "different parameter names")
+ << OriginalDeclaration
+ << static_cast<int>(InconsistentDeclarations.size());
+ for (const InconsistentDeclarationInfo &InconsistentDeclaration :
+ InconsistentDeclarations) {
+ for (const DifferingParamInfo &ParamInfo :
+ InconsistentDeclaration.DifferingParams) {
+ if (ParamInfo.GenerateFixItHint) {
+ MainDiag << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(ParamInfo.OtherNameRange),
+ ParamInfo.SourceName);
+ }
+ }
+ }
+ }
int Count = 1;
for (const InconsistentDeclarationInfo &InconsistentDeclaration :
InconsistentDeclarations) {
@@ -270,11 +274,21 @@
StringRef FunctionDescription, StringRef ParameterSourceDescription) {
for (const InconsistentDeclarationInfo &InconsistentDeclaration :
InconsistentDeclarations) {
- Check->diag(InconsistentDeclaration.DeclarationLocation,
- "%0 %q1 has a %2 with different parameter names")
- << FunctionDescription << OriginalDeclaration
- << ParameterSourceDescription;
-
+ {
+ auto MainDiag =
+ Check->diag(InconsistentDeclaration.DeclarationLocation,
+ "%0 %q1 has a %2 with different parameter names")
+ << FunctionDescription << OriginalDeclaration
+ << ParameterSourceDescription;
+ for (const DifferingParamInfo &ParamInfo :
+ InconsistentDeclaration.DifferingParams) {
+ if (ParamInfo.GenerateFixItHint) {
+ MainDiag << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(ParamInfo.OtherNameRange),
+ ParamInfo.SourceName);
+ }
+ }
+ }
Check->diag(ParameterSourceDeclaration->getLocation(), "the %0 seen here",
DiagnosticIDs::Level::Note)
<< ParameterSourceDescription;
Index: clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
@@ -181,10 +181,7 @@
for (const auto &Context : Contexts) {
if (!Context.IsUsed) {
diag(Context.FoundUsingDecl->getLocation(), "using decl %0 is unused")
- << Context.FoundUsingDecl;
- // Emit a fix and a fix description of the check;
- diag(Context.FoundUsingDecl->getLocation(),
- /*Description=*/"remove the using", DiagnosticIDs::Note)
+ << Context.FoundUsingDecl
<< FixItHint::CreateRemoval(Context.UsingDeclRange);
}
}
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -218,6 +218,13 @@
const Diagnostic &Info, ClangTidyContext &Context,
bool AllowIO = true);
+/// Gets the FixIt attached to \p Diagnostic.
+/// If \p AnyFix is true and there is no FixIt attached to the Message,
+/// returns the first FixIt attached to any notes in the message.
+/// If no FixIt is found, returns nullptr.
+const llvm::StringMap<tooling::Replacements> *
+getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix);
+
/// A diagnostic consumer that turns each \c Diagnostic into a
/// \c SourceManager-independent \c ClangTidyError.
//
@@ -227,7 +234,8 @@
public:
ClangTidyDiagnosticConsumer(ClangTidyContext &Ctx,
DiagnosticsEngine *ExternalDiagEngine = nullptr,
- bool RemoveIncompatibleErrors = true);
+ bool RemoveIncompatibleErrors = true,
+ bool FindAnyFixIt = false);
// FIXME: The concept of converting between FixItHints and Replacements is
// more generic and should be pulled out into a more useful Diagnostics
@@ -257,6 +265,7 @@
ClangTidyContext &Context;
DiagnosticsEngine *ExternalDiagEngine;
bool RemoveIncompatibleErrors;
+ bool FindAnyFixIt;
std::vector<ClangTidyError> Errors;
std::unique_ptr<llvm::Regex> HeaderFilter;
bool LastErrorRelatesToUserCode;
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -247,11 +247,11 @@
ClangTidyDiagnosticConsumer::ClangTidyDiagnosticConsumer(
ClangTidyContext &Ctx, DiagnosticsEngine *ExternalDiagEngine,
- bool RemoveIncompatibleErrors)
+ bool RemoveIncompatibleErrors, bool FindAnyFixit)
: Context(Ctx), ExternalDiagEngine(ExternalDiagEngine),
RemoveIncompatibleErrors(RemoveIncompatibleErrors),
- LastErrorRelatesToUserCode(false), LastErrorPassesLineFilter(false),
- LastErrorWasIgnored(false) {}
+ FindAnyFixIt(FindAnyFixit), LastErrorRelatesToUserCode(false),
+ LastErrorPassesLineFilter(false), LastErrorWasIgnored(false) {}
void ClangTidyDiagnosticConsumer::finalizeLastError() {
if (!Errors.empty()) {
@@ -359,6 +359,19 @@
Context, AllowIO);
}
+const llvm::StringMap<tooling::Replacements> *
+getFixIt(const tooling::Diagnostic &Diagnostic, bool AnyFix) {
+ if (!Diagnostic.Message.Fix.empty())
+ return &Diagnostic.Message.Fix;
+ if (AnyFix) {
+ for (const auto &Note : Diagnostic.Notes) {
+ if (!Note.Fix.empty())
+ return &Note.Fix;
+ }
+ }
+ return nullptr;
+}
+
} // namespace tidy
} // namespace clang
@@ -643,7 +656,7 @@
std::pair<ClangTidyError *, llvm::StringMap<tooling::Replacements> *>>
ErrorFixes;
for (auto &Error : Errors) {
- if (const auto *Fix = tooling::selectFirstFix(Error))
+ if (const auto *Fix = getFixIt(Error, FindAnyFixIt))
ErrorFixes.emplace_back(
&Error, const_cast<llvm::StringMap<tooling::Replacements> *>(Fix));
}
Index: clang-tools-extra/clang-tidy/ClangTidy.h
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.h
+++ clang-tools-extra/clang-tidy/ClangTidy.h
@@ -79,7 +79,7 @@
const tooling::CompilationDatabase &Compilations,
ArrayRef<std::string> InputFiles,
llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> BaseFS,
- bool EnableCheckProfile = false,
+ bool ApplyAnyFix, bool EnableCheckProfile = false,
llvm::StringRef StoreCheckProfile = StringRef());
// FIXME: This interface will need to be significantly extended to be useful.
@@ -89,7 +89,7 @@
/// Errors containing fixes are automatically applied and reformatted. If no
/// clang-format configuration file is found, the given \P FormatStyle is used.
void handleErrors(llvm::ArrayRef<ClangTidyError> Errors,
- ClangTidyContext &Context, bool Fix,
+ ClangTidyContext &Context, bool Fix, bool AnyFix,
unsigned &WarningsAsErrorsCount,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS);
Index: clang-tools-extra/clang-tidy/ClangTidy.cpp
===================================================================
--- clang-tools-extra/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -99,14 +99,15 @@
class ErrorReporter {
public:
- ErrorReporter(ClangTidyContext &Context, bool ApplyFixes,
+ ErrorReporter(ClangTidyContext &Context, bool ApplyFixes, bool ApplyAnyFix,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS)
: Files(FileSystemOptions(), BaseFS), DiagOpts(new DiagnosticOptions()),
DiagPrinter(new TextDiagnosticPrinter(llvm::outs(), &*DiagOpts)),
Diags(IntrusiveRefCntPtr<DiagnosticIDs>(new DiagnosticIDs), &*DiagOpts,
DiagPrinter),
SourceMgr(Diags, Files), Context(Context), ApplyFixes(ApplyFixes),
- TotalFixes(0), AppliedFixes(0), WarningsAsErrors(0) {
+ ApplyAnyFix(ApplyAnyFix), TotalFixes(0), AppliedFixes(0),
+ WarningsAsErrors(0) {
DiagOpts->ShowColors = Context.getOptions().UseColor.getValueOr(
llvm::sys::Process::StandardOutHasColors());
DiagPrinter->BeginSourceFile(LangOpts);
@@ -136,7 +137,8 @@
auto Diag = Diags.Report(Loc, Diags.getCustomDiagID(Level, "%0 [%1]"))
<< Message.Message << Name;
// FIXME: explore options to support interactive fix selection.
- const llvm::StringMap<Replacements> *ChosenFix = selectFirstFix(Error);
+ const llvm::StringMap<Replacements> *ChosenFix =
+ getFixIt(Error, ApplyAnyFix);
if (ApplyFixes && ChosenFix) {
for (const auto &FileAndReplacements : *ChosenFix) {
for (const auto &Repl : FileAndReplacements.second) {
@@ -291,6 +293,7 @@
llvm::StringMap<Replacements> FileReplacements;
ClangTidyContext &Context;
bool ApplyFixes;
+ bool ApplyAnyFix;
unsigned TotalFixes;
unsigned AppliedFixes;
unsigned WarningsAsErrors;
@@ -502,7 +505,8 @@
const CompilationDatabase &Compilations,
ArrayRef<std::string> InputFiles,
llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> BaseFS,
- bool EnableCheckProfile, llvm::StringRef StoreCheckProfile) {
+ bool ApplyAnyFix, bool EnableCheckProfile,
+ llvm::StringRef StoreCheckProfile) {
ClangTool Tool(Compilations, InputFiles,
std::make_shared<PCHContainerOperations>(), BaseFS);
@@ -529,7 +533,7 @@
Context.setEnableProfiling(EnableCheckProfile);
Context.setProfileStoragePrefix(StoreCheckProfile);
- ClangTidyDiagnosticConsumer DiagConsumer(Context);
+ ClangTidyDiagnosticConsumer DiagConsumer(Context, nullptr, true, ApplyAnyFix);
DiagnosticsEngine DE(new DiagnosticIDs(), new DiagnosticOptions(),
&DiagConsumer, /*ShouldOwnClient=*/false);
Context.setDiagnosticsEngine(&DE);
@@ -576,10 +580,10 @@
}
void handleErrors(llvm::ArrayRef<ClangTidyError> Errors,
- ClangTidyContext &Context, bool Fix,
+ ClangTidyContext &Context, bool Fix, bool AnyFix,
unsigned &WarningsAsErrorsCount,
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) {
- ErrorReporter Reporter(Context, Fix, BaseFS);
+ ErrorReporter Reporter(Context, Fix, AnyFix, BaseFS);
llvm::vfs::FileSystem &FileSystem =
Reporter.getSourceManager().getFileManager().getVirtualFileSystem();
auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
