-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[clang-tidy] New option to remove arguments from the command line #164344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0e58783
d9a64e6
f2c15c6
452eecd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -581,6 +581,24 @@ runClangTidy(clang::tidy::ClangTidyContext &Context, | |
return AdjustedArgs; | ||
}; | ||
|
||
// Remove unwanted arguments passed to the compiler | ||
ArgumentsAdjuster CompilationArgsToRemove = | ||
[&Context](const CommandLineArguments &Args, StringRef Filename) { | ||
ClangTidyOptions Opts = Context.getOptionsForFile(Filename); | ||
CommandLineArguments AdjustedArgs = Args; | ||
|
||
if (Opts.RemovedArgs) { | ||
for (StringRef ArgToIgnore : *Opts.RemovedArgs) { | ||
AdjustedArgs.erase(std::remove(AdjustedArgs.begin(), | ||
AdjustedArgs.end(), ArgToIgnore), | ||
AdjustedArgs.end()); | ||
} | ||
} | ||
|
||
return AdjustedArgs; | ||
}; | ||
|
||
Tool.appendArgumentsAdjuster(CompilationArgsToRemove); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: for consistency with the one for |
||
Tool.appendArgumentsAdjuster(PerFileExtraArgumentsInserter); | ||
Tool.appendArgumentsAdjuster(getStripPluginsAdjuster()); | ||
Context.setEnableProfiling(EnableCheckProfile); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -227,6 +227,7 @@ template <> struct MappingTraits<ClangTidyOptions> { | |
Options.ExcludeHeaderFilterRegex); | ||
IO.mapOptional("FormatStyle", Options.FormatStyle); | ||
IO.mapOptional("User", Options.User); | ||
IO.mapOptional("RemovedArgs", Options.RemovedArgs); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't alphabetically formatted, so could we place |
||
IO.mapOptional("CheckOptions", Options.CheckOptions); | ||
IO.mapOptional("ExtraArgs", Options.ExtraArgs); | ||
IO.mapOptional("ExtraArgsBefore", Options.ExtraArgsBefore); | ||
|
@@ -252,6 +253,7 @@ ClangTidyOptions ClangTidyOptions::getDefaults() { | |
Options.SystemHeaders = false; | ||
Options.FormatStyle = "none"; | ||
Options.User = std::nullopt; | ||
Options.RemovedArgs = std::nullopt; | ||
for (const ClangTidyModuleRegistry::entry &Module : | ||
ClangTidyModuleRegistry::entries()) | ||
Options.mergeWith(Module.instantiate()->getModuleOptions(), 0); | ||
|
@@ -292,6 +294,7 @@ ClangTidyOptions &ClangTidyOptions::mergeWith(const ClangTidyOptions &Other, | |
overrideValue(SystemHeaders, Other.SystemHeaders); | ||
overrideValue(FormatStyle, Other.FormatStyle); | ||
overrideValue(User, Other.User); | ||
mergeVectors(RemovedArgs, Other.RemovedArgs); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto consistency |
||
overrideValue(UseColor, Other.UseColor); | ||
mergeVectors(ExtraArgs, Other.ExtraArgs); | ||
mergeVectors(ExtraArgsBefore, Other.ExtraArgsBefore); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,6 +112,10 @@ struct ClangTidyOptions { | |
/// comments in the relevant check. | ||
std::optional<std::string> User; | ||
|
||
/// \brief Remove command line arguments sent to the compiler matching this | ||
/// regex. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's no longer a regex |
||
std::optional<std::vector<std::string>> RemovedArgs; | ||
|
||
/// Helper structure for storing option value with priority of the value. | ||
struct ClangTidyValue { | ||
ClangTidyValue() = default; | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -167,6 +167,9 @@ Improvements to clang-tidy | |||||
scripts by adding the `-hide-progress` option to suppress progress and | ||||||
informational messages. | ||||||
|
||||||
- Improved :program:`clang-tidy` by adding the option `RemovedArgs` to remove | ||||||
arguments sent to the compiler when invoking clang-tidy. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
- Deprecated the :program:`clang-tidy` ``zircon`` module. All checks have been | ||||||
moved to the ``fuchsia`` module instead. The ``zircon`` module will be removed | ||||||
in the 24th release. | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -329,6 +329,10 @@ An overview of all the command-line options: | |
example, to place the correct user name in | ||
TODO() comments in the relevant check. | ||
WarningsAsErrors - Same as '--warnings-as-errors'. | ||
RemovedArgs - List of arguments to remove from the command | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would It be hard to also add cli-option
|
||
line sent to the compiler. Please note that | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps good to document that this removes the args from the "original" command line, before considering |
||
removing arguments from the command line | ||
might lead to false positive or negatives. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Warning people about false positives and negatives makes sense, but I don't think we should be telling people to expect crashes; a crash is always a bug, it's not something users should ever expect, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm yes, crash is a strong word. I meant more "hard compiler errors" which cannot simply be suppressed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I may be missing something, but how can removing compiler options lead to FP/FN? I agree with "hard compiler errors" thought. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As an example, removing the |
||
The effective configuration can be inspected using --dump-config: | ||
|
@@ -338,6 +342,7 @@ An overview of all the command-line options: | |
WarningsAsErrors: '' | ||
HeaderFileExtensions: ['', 'h','hh','hpp','hxx'] | ||
ImplementationFileExtensions: ['c','cc','cpp','cxx'] | ||
RemovedArgs: ['-Werror'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 I think RemoveArgs sounds better There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I meant to remove this line from the code. |
||
HeaderFilterRegex: '' | ||
FormatStyle: none | ||
InheritParentConfig: true | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename to |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
// RUN: not clang-tidy %s -- -fnot-an-option | FileCheck %s -check-prefix=INVALID-A | ||
// RUN: clang-tidy %s --config="{RemovedArgs: ['-fnot-an-option']}" -- -fnot-an-option | ||
// RUN: clang-tidy %s --config="{RemovedArgs: ['-fnot-another-option', '-fnot-an-option']}" -- -fnot-an-option -fnot-another-option | ||
|
||
// INVALID-A: error: unknown argument: '-fnot-an-option' [clang-diagnostic-error] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing newline at the end of the file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ArgToRemove