[Clang][Driver] New parameter allow-unrecognized-arguments #162201
Conversation
This parameter is used to suppress the ``Unknown argument '...'`` error that clang will emit whenever it encounters an unknown argument. This is probably an error to make sure the user fixes it's mistake by either removing the argument or renaming it, but there are some cases where it's not possible to fix the issue. For instance, CMake now injects gcc-specific arguments in the clang-tidy command that breaks static-analysis (https://gitlab.kitware.com/cmake/cmake/-/issues/26283) This will also allow users to run clang-tidy / clangd on a gcc-based project without the need to maintain two separate build commands to run llvm-based tools. By enabling this parameter, the user is able to downgrade the error to a warning (unknown-argument) that he can further silence using the ``-Qunused-arguments`` flag if needed. Fixes: llvm#108455
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-driver Author: Félix-Antoine Constantin (felix642) ChangesThis is another attempt at fixing #108455. Please see the initial discussion in the following PR: #111453 This parameter is used to suppress the This is probably an error to make sure the user fixes it's mistake by either removing the argument or renaming it, but there are some cases where it's not possible to fix the issue. This will also allow users to run clang-tidy / clangd on a gcc-based project without the need to maintain two separate build commands to run llvm-based tools. By enabling this parameter, the user is able to downgrade the error to a warning (unknown-argument) that he can further silence using the Fixes: #108455 Full diff: https://github.com/llvm/llvm-project/pull/162201.diff 4 Files Affected:
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2ef609831637e..de0d32f68253a 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1011,6 +1011,9 @@ def : Flag<["-"], "Xcompiler">, IgnoredGCCCompat;
def Z_Flag : Flag<["-"], "Z">, Group<Link_Group>;
def all__load : Flag<["-"], "all_load">;
def allowable__client : Separate<["-"], "allowable_client">;
+def allow_unrecognized_arguments : Flag<["--"], "allow-unrecognized-arguments">,
+ Visibility<[ClangOption, CLOption]>,
+ HelpText<"Ignore unrecognized command-line arguments instead of reporting an error.">;
def ansi : Flag<["-", "--"], "ansi">, Group<CompileOnly_Group>;
def arch__errors__fatal : Flag<["-"], "arch_errors_fatal">;
def arch : Separate<["-"], "arch">, Flags<[NoXarchOption,TargetSpecific]>;
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 85a1335785542..4f2ac9ba8271c 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -303,29 +303,31 @@ InputArgList Driver::ParseArgStrings(ArrayRef<const char *> ArgStrings,
}
}
- for (const Arg *A : Args.filtered(options::OPT_UNKNOWN)) {
- unsigned DiagID;
- auto ArgString = A->getAsString(Args);
- std::string Nearest;
- if (getOpts().findNearest(ArgString, Nearest, VisibilityMask) > 1) {
- if (!IsCLMode() &&
- getOpts().findExact(ArgString, Nearest,
- llvm::opt::Visibility(options::CC1Option))) {
- DiagID = diag::err_drv_unknown_argument_with_suggestion;
- Diags.Report(DiagID) << ArgString << "-Xclang " + Nearest;
+ if (!Args.hasArg(options::OPT_allow_unrecognized_arguments)) {
+ for (const Arg *A : Args.filtered(options::OPT_UNKNOWN)) {
+ unsigned DiagID;
+ auto ArgString = A->getAsString(Args);
+ std::string Nearest;
+ if (getOpts().findNearest(ArgString, Nearest, VisibilityMask) > 1) {
+ if (!IsCLMode() &&
+ getOpts().findExact(ArgString, Nearest,
+ llvm::opt::Visibility(options::CC1Option))) {
+ DiagID = diag::err_drv_unknown_argument_with_suggestion;
+ Diags.Report(DiagID) << ArgString << "-Xclang " + Nearest;
+ } else {
+ DiagID = IsCLMode() ? diag::warn_drv_unknown_argument_clang_cl
+ : diag::err_drv_unknown_argument;
+ Diags.Report(DiagID) << ArgString;
+ }
} else {
- DiagID = IsCLMode() ? diag::warn_drv_unknown_argument_clang_cl
- : diag::err_drv_unknown_argument;
- Diags.Report(DiagID) << ArgString;
+ DiagID = IsCLMode()
+ ? diag::warn_drv_unknown_argument_clang_cl_with_suggestion
+ : diag::err_drv_unknown_argument_with_suggestion;
+ Diags.Report(DiagID) << ArgString << Nearest;
}
- } else {
- DiagID = IsCLMode()
- ? diag::warn_drv_unknown_argument_clang_cl_with_suggestion
- : diag::err_drv_unknown_argument_with_suggestion;
- Diags.Report(DiagID) << ArgString << Nearest;
+ ContainsError |= Diags.getDiagnosticLevel(DiagID, SourceLocation()) >
+ DiagnosticsEngine::Warning;
}
- ContainsError |= Diags.getDiagnosticLevel(DiagID, SourceLocation()) >
- DiagnosticsEngine::Warning;
}
for (const Arg *A : Args.filtered(options::OPT_o)) {
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 422375240bab6..0ba29db471617 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -4991,15 +4991,17 @@ bool CompilerInvocation::CreateFromArgsImpl(
Diags.Report(diag::err_drv_missing_argument)
<< Args.getArgString(MissingArgIndex) << MissingArgCount;
- // Issue errors on unknown arguments.
- for (const auto *A : Args.filtered(OPT_UNKNOWN)) {
- auto ArgString = A->getAsString(Args);
- std::string Nearest;
- if (Opts.findNearest(ArgString, Nearest, VisibilityMask) > 1)
- Diags.Report(diag::err_drv_unknown_argument) << ArgString;
- else
- Diags.Report(diag::err_drv_unknown_argument_with_suggestion)
- << ArgString << Nearest;
+ if (!Args.hasArg(options::OPT_allow_unrecognized_arguments)) {
+ // Issue errors on unknown arguments.
+ for (const auto *A : Args.filtered(OPT_UNKNOWN)) {
+ auto ArgString = A->getAsString(Args);
+ std::string Nearest;
+ if (Opts.findNearest(ArgString, Nearest, VisibilityMask) > 1)
+ Diags.Report(diag::err_drv_unknown_argument) << ArgString;
+ else
+ Diags.Report(diag::err_drv_unknown_argument_with_suggestion)
+ << ArgString << Nearest;
+ }
}
ParseFileSystemArgs(Res.getFileSystemOpts(), Args, Diags);
diff --git a/clang/test/Driver/unsupported-option.c b/clang/test/Driver/unsupported-option.c
index 7234e52571582..465f1879b31c8 100644
--- a/clang/test/Driver/unsupported-option.c
+++ b/clang/test/Driver/unsupported-option.c
@@ -32,3 +32,7 @@
// RUN: not %clang -c -Qunused-arguments --target=aarch64-- -mfpu=crypto-neon-fp-armv8 %s 2>&1 \
// RUN: | FileCheck %s --check-prefix=QUNUSED_ARGUMENTS
// QUNUSED_ARGUMENTS: error: unsupported option '-mfpu=' for target 'aarch64--'
+
+// RUN: %clang %s -invalid --allow-unrecognized-arguments -### 2>&1 | \
+// RUN: FileCheck %s --check-prefix=UNKNOWN_ARGUMENT
+// UNKNOWN_ARGUMENT: warning: argument unused during compilation: '-invalid'
|
|
@nicovank @carlosgalvezp @AaronBallman @HerrCai0907 @5chmidti @MaskRay |
Thanks for investigating fixes here!
Personally, I'm not a big fan of having a command line option to control diagnostics of command line options. I think command line options are generally processed in a left-to-right order where the last flag "wins", which is kind of awkward with this design. e.g., I think this probably should remain an error. Passing unknown inputs to the compiler and expecting to get a known output is a pretty big ask IMO. That said, maybe others have a different opinion.
IMO, that's a CMake bug; adding an option we have to support forever to work around that behavior is not really ideal. |
|
Thank you for the feedback @AaronBallman. Just to clarify:
Undoubtedly, this was only an example on the reasoning behind this change. But I do think it is annoying from a user's perspective to be unable to use clang-tidy / clangd in those situations. They have to either wait for a fix from (in that case cmake) or us to properly fix the issue. This is not the first time this kind of issue happened and this is definitely not going to be the last one. The main use case I see for this option would be to give the user the ability to run clang-tidy, clangd (or any clang-based tool as a matter of fact) on a gcc project and to get some decents results without having to maintain two separate build commands or to write some convoluted script to transform the gcc-based command line into something that clang will not complain about. If we give the user the ability to run clang-based tools without any changes on they're project would, in my opinion, greatly simplify the integration of those tools in existing projects. It follows the logic of "Make it work and then make it right" and the "Unrecognized argument" is probably one of the only thing keeping them from doing that.
That being said, I'm wondering if it would be better to instead modify the This would limit the scope of this option and would give us the ability to enable it only for the use-cases that we think are needed. |
|
You can use the If we decide to add this functionality, we could consider implementing --start-ignore-unknown-arguments / --end-ignore-unknown-arguments flags. This would be similar to the existing --start-no-unused-arguments / --end-no-unused-arguments flags, which are used to suppress warnings. @mstorsjo Alternative: |
|
Referring to command-line flags which clearly specify the standards documents they're referring to in the flag itself as "gcc-specific arguments" seems disingenuous and unprofessional. The reference to Kitware's proposal is the first result from google. The kitware issue clearly describes how to turn these off in the cmake file:
As I understand it, modules are a pretty fundamental change to the way C++ dependencies work. The
I also see that I also see that my locally installed copy of The linked issue regarding cmake describes these three arguments as problematic: The proposed methodology to directly ignore these inputs seems tantamount to a rejection of the proposals for interoperable module support across build systems.
Given that modules are a standard API, and that clang already supports precisely the functionality needed, it's unclear why this is being described as "gcc-based". Is there context that's not being stated here?
Given that my laptop's clang++ version supports very similar command-line arguments already, I'm curious why clang-tidy wouldn't be able to make use of that support from clang. Is there some deeper issue that stops clang-tidy from being able to support C++20 modules, or from using the code in clang that processes module dependencies? Much like the make jobserver protocol that ninja and cargo now implement, gcc describes a very explicit effort of at least 5 years to get external buy-in for the exact functionality that is being described as "gcc-based" here (https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Module-Mapper.html):
Removing arguments which define dependency relationships from the command line, based upon internal filtering logic that's defined in the specific version of clang-tidy's compiled C++ code and not in the build system, strikes me as a decision which could incur later user requests for an additional flag to cancel out the first flag filtering functionality, if the user ever figures out their dependency issue was because of this filtering logic in the first place. In particular, many build systems such as bazel will cache process executions based upon the command-line arguments, especially if those arguments specify input files for the compiler to read. It would seem more appropriate to make such incompatibilities visible to the build system by writing diagnostics about unsupported flags to a separate output file, instead of subtly changing the semantics of command-line parsing in a data-dependent manner. |
|
FWIW,
This sounds like it could be a solution!
I don't think this would work, because it would need to be added to the GCC compile flags, and then those flags would be unknown to GCC? Regarding modules, I think it's out of scope for this ticket and deserves an RFC/ticket of its own. One example use case I've personally come across is as follows:
The problem I see with simply removing flags is that the code may not be analysed correctly, which may lead to other errors, which people will then ask us to patch, and so on. |
This is another attempt at fixing #108455. Please see the initial discussion in the following PR: #111453
This parameter is used to suppress the
Unknown argument '...'error that clang will emit whenever it encounters an unknown argument.This is probably an error to make sure the user fixes it's mistake by either removing the argument or renaming it, but there are some cases where it's not possible to fix the issue.
For instance, CMake now injects gcc-specific arguments in the clang-tidy command that breaks static-analysis
(https://gitlab.kitware.com/cmake/cmake/-/issues/26283)
This will also allow users to run clang-tidy / clangd on a gcc-based project without the need to maintain two separate build commands to run llvm-based tools.
By enabling this parameter, the user is able to downgrade the error to a warning (unknown-argument) that he can further silence using the
-Qunused-argumentsflag if needed.Fixes: #108455