[clang] Add --start-no-unused-arguments/--end-no-unused-arguments to silence some unused argument warnings

[mstorsjo]

Summary

When passing a set of flags to configure defaults for a specific
target (similar to the cmake settings CLANG_DEFAULT_RTLIB,
CLANG_DEFAULT_UNWINDLIB, CLANG_DEFAULT_CXX_STDLIB and
CLANG_DEFAULT_LINKER, but without hardcoding it in the binary),
some of the flags may cause warnings (e.g. -stdlib= when compiling C
code). Allow requesting selectively ignoring unused arguments among
some of the arguments on the command line, without needing to resort
to -Qunused-arguments or -Wno-unused-command-line-argument.

Fix up the existing diagnostics.c testcase. It was added in
response to PR12181 to fix handling of
-Werror=unused-command-line-argument, but the command line option
in the test (-fzyzzybalubah) now triggers "error: unknown argument"
instead of the intended warning. Change it into a linker input
(-lfoo) which triggers the intended diagnostic. Extend the
existing test case to check more cases and make sure that it keeps
testing the intended case.

Add testing of the new option to this existing test.

[MaskRay]

I know some folks who dislike aggressive unused command line option diagnostics but also worry about -Qunused-arguments -Wno-unused-command-line-argument affecting the entire command line. start/end looks good to me.

clang/docs/ClangCommandLineReference.rst

1342

The sentence will be scrubbed.

Use path/to/clang-tblgen -gen-opt-docs -I llvm/include -I clang/include/clang/Driver clang/include/clang/Driver/ClangOptionDocs.td > clang/docs/ClangCommandLineReference.rst to regenerate this rst.

clang/include/clang/Driver/Options.td

3918

Drop NoXarchOption. The option can be used with Darwin -Xarch (test/Driver/Xarch.c) though it is unnecessary to test it.

clang/test/Driver/diagnostics.c

21

Looks like no command has an output. In case there is one, make sure -o /dev/null

[MaskRay]

Add arguments for silencing unused argument warnings for some but not all arguments

A more searchable subject (mentioning the new option in the subject is useful for archaeology) may be Add --start-no-unused-arguments/--end-no-unused-arguments to silence some unused argument warnings

[mstorsjo]

In D116503#3221083, @MaskRay wrote:

A more searchable subject (mentioning the new option in the subject is useful for archaeology) may be Add --start-no-unused-arguments/--end-no-unused-arguments to silence some unused argument warnings

Thanks, that's indeed a better summary (although the subject line ends up even longer than now). Will change it to that form.

clang/docs/ClangCommandLineReference.rst

1342

Ok, will do. There's lots of other changes in the file though, from other options that haven't been added here, but I'm just including the changes for the options I'm adding in this commit.

clang/include/clang/Driver/Options.td

3918

Ok, will do.

clang/test/Driver/diagnostics.c

21

As all commands use -fsyntax-only, I presume there should be no output. So do I read your comment correctly that we should have -o /dev/null specifically _if_ we'd have a test that lack -fsyntax-only, or do you want me to add it just in case? (The preexisting test doesn't have that and just use -fsyntax-only.)

[mstorsjo]

Regenerated ClangCommandLineReference.rst (with the relevant changes), removed NoXarchOption.

[MaskRay]

I forwarded this to some groups who want this functionality. Jannik2099 from Gentoo likes the change.

But make sure to wait a bit to see what others think.

clang/test/Driver/diagnostics.c

21

-fsyntax-only doesn't need -o /dev/null.

Some downstream users (including Google) may run the test in the source directory and make the source directory in a read-only filesystem, so printing files other than somewhere in %t* will lead to an error.

[MaskRay]

clang/test/Driver/diagnostics.c

34

You may change some x86_64-apple-darwin10 to other triples, probably including clang-cl (to test CoreOption).

The canonical spelling for -target is ---target=. -target (space separated driver option) is just abused so much that it is unrealistic to remove support now.

[MaskRay]

clang/test/Driver/diagnostics.c

2

If you are going to update comments, remove this PR12181. It says an issue about -Werror=unused-command-line-argument but your comment below has covered the information.

[phosek]

LGTM this is something we have a use case for this in Fuchsia as well.

[mstorsjo]

Add testcases for using the new option with clang-cl, use --target= everywhere.

[mstorsjo]

In D116503#3223094, @MaskRay wrote:

But make sure to wait a bit to see what others think.

Do you think it's ok to go ahead and land this change now, or should I wait a bit more? (I'd like to have it landed with enough margin before the 14.x branch so that it has time to settle before that.)

[MaskRay]

In D116503#3231138, @mstorsjo wrote:

In D116503#3223094, @MaskRay wrote:

But make sure to wait a bit to see what others think.

Do you think it's ok to go ahead and land this change now, or should I wait a bit more? (I'd like to have it landed with enough margin before the 14.x branch so that it has time to settle before that.)

The wait time has been sufficiently long to me:) Go head.

clang/docs/ClangCommandLineReference.rst

 Enable the specified warning
 
 .. option:: -Wdeprecated, -Wno-deprecated
 
 Enable warnings for deprecated constructs and define \_\_DEPRECATED
 
 .. option:: -Wnonportable-cfstrings<arg>, -Wno-nonportable-cfstrings<arg>
 
+.. option:: --start-no-unused-arguments, --end-no-unused-arguments
+
+Don't warn for unused arguments between these arguments.
+
 Target-independent compilation options
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 .. option:: -Wframe-larger-than=<arg>
 
 .. option:: -fPIC, -fno-PIC
 
 .. option:: -fPIE, -fno-PIE
 

clang/include/clang/Driver/Options.td

def emit_ast : Flag<["-"], "emit-ast">,
   HelpText<"Emit Clang AST files for source inputs">;
 def emit_llvm : Flag<["-"], "emit-llvm">, Flags<[CC1Option]>, Group<Action_Group>,
   HelpText<"Use the LLVM representation for assembler and object files">;
 def emit_interface_stubs : Flag<["-"], "emit-interface-stubs">, Flags<[CC1Option]>, Group<Action_Group>,
   HelpText<"Generate Interface Stub Files.">;
 def emit_merged_ifs : Flag<["-"], "emit-merged-ifs">,
   Flags<[CC1Option]>, Group<Action_Group>,
   HelpText<"Generate Interface Stub Files, emit merged text not binary.">;
+def end_no_unused_arguments : Flag<["--"], "end-no-unused-arguments">, Flags<[NoXarchOption, CoreOption]>,
+  HelpText<"Start emitting warnings for unused driver arguments">;
 def interface_stub_version_EQ : JoinedOrSeparate<["-"], "interface-stub-version=">, Flags<[CC1Option]>;
 def exported__symbols__list : Separate<["-"], "exported_symbols_list">;
 def e : JoinedOrSeparate<["-"], "e">, Flags<[LinkerInput]>, Group<Link_Group>;
 def fmax_tokens_EQ : Joined<["-"], "fmax-tokens=">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Max total number of preprocessed tokens for -Wmax-tokens.">,
   MarshallingInfoInt<LangOpts<"MaxTokens">>;
 def fPIC : Flag<["-"], "fPIC">, Group<f_Group>;
 def fno_PIC : Flag<["-"], "fno-PIC">, Group<f_Group>;

 def segs__read__only__addr : Separate<["-"], "segs_read_only_addr">;
 def segs__read__write__addr : Separate<["-"], "segs_read_write_addr">;
 def segs__read__ : Joined<["-"], "segs_read_">;
 def shared_libgcc : Flag<["-"], "shared-libgcc">;
 def shared : Flag<["-", "--"], "shared">, Group<Link_Group>;
 def single__module : Flag<["-"], "single_module">;
 def specs_EQ : Joined<["-", "--"], "specs=">, Group<Link_Group>;
 def specs : Separate<["-", "--"], "specs">, Flags<[Unsupported]>;
+def start_no_unused_arguments : Flag<["--"], "start-no-unused-arguments">, Flags<[NoXarchOption, CoreOption]>,
+  HelpText<"Don't emit warnings about unused arguments for the following arguments">;
 def static_libgcc : Flag<["-"], "static-libgcc">;
 def static_libstdcxx : Flag<["-"], "static-libstdc++">;
 def static : Flag<["-", "--"], "static">, Group<Link_Group>, Flags<[NoArgumentUnused]>;
 def std_default_EQ : Joined<["-"], "std-default=">;
 def std_EQ : Joined<["-", "--"], "std=">, Flags<[CC1Option,FlangOption,FC1Option]>,
   Group<CompileOnly_Group>, HelpText<"Language standard to compile for">,
   ValuesCode<[{
     const char *Values =

clang/lib/Driver/Driver.cpp

 
 DerivedArgList *Driver::TranslateInputArgs(const InputArgList &Args) const {
   const llvm::opt::OptTable &Opts = getOpts();
   DerivedArgList *DAL = new DerivedArgList(Args);
 
   bool HasNostdlib = Args.hasArg(options::OPT_nostdlib);
   bool HasNostdlibxx = Args.hasArg(options::OPT_nostdlibxx);
   bool HasNodefaultlib = Args.hasArg(options::OPT_nodefaultlibs);
+  bool IgnoreUnused = false;
   for (Arg *A : Args) {
+    if (IgnoreUnused)
+      A->claim();
+
+    if (A->getOption().matches(options::OPT_start_no_unused_arguments)) {
+      IgnoreUnused = true;
+      continue;
+    }
+    if (A->getOption().matches(options::OPT_end_no_unused_arguments)) {
+      IgnoreUnused = false;
+      continue;
+    }
+
     // Unfortunately, we have to parse some forwarding options (-Xassembler,
     // -Xlinker, -Xpreprocessor) because we either integrate their functionality
     // (assembler and preprocessor), or bypass a previous driver ('collect2').
 
     // Rewrite linker options, to replace --no-demangle with a custom internal
     // option.
     if ((A->getOption().matches(options::OPT_Wl_COMMA) ||
          A->getOption().matches(options::OPT_Xlinker)) &&

clang/test/Driver/diagnostics.c

 // Parse diagnostic arguments in the driver
 // PR12181
 
+// Exactly which arguments are warned about and which aren't differ based
+// on what target is selected. -stdlib= and -fuse-ld= emit diagnostics when
+// compiling C code, for e.g. *-linux-gnu. Linker inputs, like -lfoo, emit
+// diagnostics when only compiling for all targets.
+
+// This is normally a non-fatal warning:
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo %s 2>&1 | FileCheck %s
+
+// Either with a specific -Werror=unused.. or a blanket -Werror, this
+// causes the command to fail.
+// RUN: not %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo \
+// RUN:   -Werror=unused-command-line-argument %s 2>&1 | FileCheck %s
+
 // RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah \
-// RUN:   -Werror=unused-command-line-argument %s
+// RUN:   -fsyntax-only -lfoo -Werror %s 2>&1 | FileCheck %s
 
+// With a specific -Wno-..., no diagnostic should be printed.
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Wno-unused-command-line-argument %s 2>&1 | count 0
+
+// With -Qunused-arguments, no diagnostic should be printed.
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -fsyntax-only -lfoo -Werror \
+// RUN:   -Qunused-arguments %s 2>&1 | count 0
+
+// With the argument enclosed in --{start,end}-no-unused-arguments,
+// there's no diagnostic.
+// RUN: %clang -target x86_64-apple-darwin10 -fsyntax-only \
+// RUN:   --start-no-unused-arguments -lfoo --end-no-unused-arguments \
+// RUN:   -Werror %s 2>&1 | count 0
+
+// With --{start,end}-no-unused-argument around a different argument, it
+// still warns about the unused argument.
 // RUN: not %clang -target x86_64-apple-darwin10 \
-// RUN:   -fsyntax-only -fzyzzybalubah -Werror %s
+// RUN:   --start-no-unused-arguments -fsyntax-only --end-no-unused-arguments \
+// RUN:   -lfoo -Werror %s 2>&1 | FileCheck %s
+
+// CHECK: -lfoo: 'linker' input unused