[llvm-lib] Add /WX, warn by default on empty inputs, add opt-out

[thakis]

Summary

lib.exe by default exits successfully without writing an output
file when no inputs are passed. llvm-lib has the same behavior,
for compatibility.

This behavior interacts poorly with build systems: If a static
library target had no inputs, llvm-lib would not produce an output
file, causing ninja (or make, or a similar system) to successfully
run that step, but then re-run it on the next build.

After this patch, llvm-lib emits a warning in this case, that with
/WX can be turned into an error. That way, ninja (or make, or...)
will mark the initial build as failed.

People who don't like the warning can use /ignore:emptyoutput to
suppress it.

The warning also points out the existing flag /llvmlibempty which
forces creation of an empty .lib file (this is an extension to lib.exe).

[hans]

Nice!

Were there no other warnings that need to handle /ignore and /WX?

[thakis]

Thanks!

Before this change, every diag that llvm-lib emitted was fatal. So this is the only diag that needs WX / ignore.

lld/COFF/Options.td

 def force : F<"force">,
     HelpText<"Allow undefined and multiply defined symbols">;
 def force_unresolved : F<"force:unresolved">,
     HelpText<"Allow undefined symbols when creating executables">;
 def force_multiple : F<"force:multiple">,
     HelpText<"Allow multiply defined symbols when creating executables">;
 def force_multipleres : F<"force:multipleres">,
     HelpText<"Allow multiply defined resources when creating executables">;
-defm WX : B<"WX", "Treat warnings as errors", "Don't treat warnings as errors">;
+defm WX : B<"WX", "Treat warnings as errors",
+                  "Don't treat warnings as errors (default)">;
 
 defm allowbind : B<"allowbind", "Enable DLL binding (default)",
                    "Disable DLL binding">;
 defm allowisolation : B<"allowisolation", "Enable DLL isolation (default)",
                         "Disable DLL isolation">;
 defm appcontainer : B<"appcontainer",
                       "Image can only be run in an app container",
                       "Image can run outside an app container (default)">;

llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp

static void appendFile(std::vector<NewArchiveMember> &Members,
   // below does, but it's not a lot of work and it's a bit awkward to do
   // in writeArchive() which needs to support many tools, can't assume the
   // input is COFF, and doesn't have a good way to report errors.
   if (Magic == file_magic::coff_object || Magic == file_magic::bitcode) {
     Expected<COFF::MachineTypes> MaybeFileMachine =
         (Magic == file_magic::coff_object) ? getCOFFFileMachine(MB)
                                            : getBitcodeFileMachine(MB);
     if (!MaybeFileMachine) {
-      handleAllErrors(MaybeFileMachine.takeError(), [&](const ErrorInfoBase &EIB) {
-        llvm::errs() << MB.getBufferIdentifier() << ": " << EIB.message()
-                     << "\n";
+      handleAllErrors(MaybeFileMachine.takeError(),
+                      [&](const ErrorInfoBase &EIB) {
+                        llvm::errs() << MB.getBufferIdentifier() << ": "
+                                     << EIB.message() << "\n";
                       });
       exit(1);
     }
     COFF::MachineTypes FileMachine = *MaybeFileMachine;
 
     // FIXME: Once lld-link rejects multiple resource .obj files:
     // Call convertResToCOFF() on .res files and add the resulting
     // COFF file to the .lib output instead of adding the .res file, and remove
     // this check. See PR42180.
llvm::errs() << "ignoring unknown argument: " << Arg->getAsString(Args)
                  << "\n";
 
   // Handle /help
   if (Args.hasArg(OPT_help)) {
     Table.printHelp(outs(), "llvm-lib [options] file...", "LLVM Lib");
     return 0;
   }
 
+  // Parse /ignore:
+  llvm::StringSet<> IgnoredWarnings;
+  for (auto *Arg : Args.filtered(OPT_ignore))
+    IgnoredWarnings.insert(Arg->getValue());
+
   // If no input files and not told otherwise, silently do nothing to match
   // lib.exe
-  if (!Args.hasArgNoClaim(OPT_INPUT) && !Args.hasArg(OPT_llvmlibempty))
+  if (!Args.hasArgNoClaim(OPT_INPUT) && !Args.hasArg(OPT_llvmlibempty)) {
+    if (!IgnoredWarnings.contains("emptyoutput")) {
+      llvm::errs() << "warning: no input files, not writing output file\n";
+      llvm::errs() << "         pass /llvmlibempty to write empty .lib file,\n";
+      llvm::errs() << "         pass /ignore:emptyoutput to suppress warning\n";
+      if (Args.hasFlag(OPT_WX, OPT_WX_no, false)) {
+        llvm::errs() << "treating warning as error due to /WX\n";
+        return 1;
+      }
+    }
     return 0;
+  }
 
   if (Args.hasArg(OPT_lst)) {
     doList(Args);
     return 0;
   }
 
   std::vector<StringRef> SearchPaths = getSearchPaths(&Args, Saver);
 

llvm/lib/ToolDrivers/llvm-lib/Options.td

 include "llvm/Option/OptParser.td"
 
 // lib.exe accepts options starting with either a dash or a slash.
 
 // Flag that takes no arguments.
 class F<string name> : Flag<["/", "-", "/?", "-?"], name>;
 
 // Flag that takes one argument after ":".
 class P<string name, string help> :
       Joined<["/", "-", "/?", "-?"], name#":">, HelpText<help>;
 
+// Boolean flag which can be suffixed by ":no". Using it unsuffixed turns the
+// flag on and using it suffixed by ":no" turns it off.
+multiclass B<string name, string help_on, string help_off> {
+  def "" : F<name>, HelpText<help_on>;
+  def _no : F<name#":no">, HelpText<help_off>;
+}
+
+def ignore : P<"ignore", "Specify warning codes to ignore">;
 def libpath: P<"libpath", "Object file search path">;
 
 // Can't be called "list" since that's a keyword.
 def lst    : F<"list">, HelpText<"List contents of .lib file on stdout">;
 def out    : P<"out", "Path to file to write output">;
 
 def llvmlibthin : F<"llvmlibthin">,
     HelpText<"Make .lib point to .obj files instead of copying their contents">;
 
 def llvmlibempty : F<"llvmlibempty">,
     HelpText<"When given no contents, produce an empty .lib file">;
 
 def machine: P<"machine", "Specify target platform">;
 
+defm WX : B<"WX", "Treat warnings as errors",
+            "Don't treat warnings as errors (default)">;
+
 def help : F<"help">;
 
 // /?? and -?? must be before /? and -? to not confuse lib/Options.
 def help_q : Flag<["/??", "-??", "/?", "-?"], "">, Alias<help>;
 
 //==============================================================================
 // The flags below do nothing. They are defined only for lib.exe compatibility.
 //==============================================================================
 
-class QF<string name> : Joined<["/", "-", "/?", "-?"], name#":">;
-
-def ignore : QF<"ignore">;
 def nologo : F<"nologo">;

llvm/test/tools/llvm-lib/no-inputs.test

 RUN: rm -f %t.lib
-RUN: llvm-lib -out:%t.lib
+RUN: llvm-lib -out:%t.lib 2>&1 | FileCheck --check-prefix=EMPTYWARN %s
 RUN: test ! -e %t.lib
+RUN: not llvm-lib -out:%t.lib /WX 2>&1 | FileCheck --check-prefix=EMPTYWARN %s
+RUN: test ! -e %t.lib
+RUN: llvm-lib -out:%t.lib /WX /WX:no 2>&1 \
+RUN:     | FileCheck --check-prefix=EMPTYWARN %s
+RUN: test ! -e %t.lib
+RUN: llvm-lib /ignore:emptyoutput -out:%t.lib 2>&1 \
+RUN:     | FileCheck --check-prefix=NOEMPTYWARN --allow-empty %s
+RUN: test ! -e %t.lib
+RUN: llvm-lib /ignore:emptyoutput /WX -out:%t.lib 2>&1 \
+RUN:     | FileCheck --check-prefix=NOEMPTYWARN --allow-empty %s
+RUN: test ! -e %t.lib
+
+EMPTYWARN: warning: no input files, not writing output file
+NOEMPTYWARN-NOT: warning: no input files, not writing output file
 
-RUN: llvm-lib /llvmlibempty -out:%t.lib
+RUN: llvm-lib /llvmlibempty -out:%t.lib 2>&1 \
+RUN:     | FileCheck --check-prefix=NOEMPTYWARN --allow-empty %s
 RUN: FileCheck %s < %t.lib
 CHECK: !<arch>