llvm-ranlib/nm: Don't print usage message except for usage errors

[sbc100]

Summary

Also, fix a bug in ranlib where it didn't correctly detect being run
without any argument and would try to operate on the empty string.

[dschuff]

llvm/test/tools/llvm-ar/invalid-object-file.test

10

I'm not sure what this sentence is trying to say. maybe remove "such as"?

[sbc100]

• feedback

llvm/test/tools/llvm-ar/invalid-object-file.test

 ## Test that we get a proper error message with a file that is not a recognized object file.
 
 # RUN: rm -f %t.txt
 # RUN: touch %t.txt
 # RUN: not llvm-ar sc %t.txt 2>&1 > /dev/null | FileCheck %s -DFILE=%t.txt
 
 # CHECK:     error: unable to load '[[FILE]]': file too small to be an archive
 # CHECK-NOT: {{.}}
+
+## Also test that errors like this (e.g. invlid input files) don't generate a usage message
+# CHECK-NOT: USAGE: llvm-ar

llvm/test/tools/llvm-ranlib/bad-usage.test

+## Test that useage message is shown when no input file is given
+## And that we return non-zero.
+
+# RUN: not llvm-ranlib 2>&1 | FileCheck %s
+
+# CHECK: error: an archive name must be specified
+# CHECK: USAGE: llvm-ranlib

llvm/test/tools/llvm-ranlib/help-message.test

+## Show that the help message for llvm-ranlib can be printed with either the
+## long flag -help.
+
+# RUN: llvm-ranlib -h | FileCheck %s
+# RUN: llvm-ranlib -help | FileCheck %s
+# RUN: llvm-ranlib --help | FileCheck %s
+
+# CHECK: USAGE: llvm-ranlib

llvm/tools/llvm-ar/llvm-ar.cpp

 using namespace llvm;
 
 // The name this program was invoked as.
 static StringRef ToolName;
 
 // The basename of this program.
 static StringRef Stem;
 
-const char RanlibHelp[] = R"(
-OVERVIEW: LLVM Ranlib (llvm-ranlib)
+const char RanlibHelp[] = R"(OVERVIEW: LLVM Ranlib (llvm-ranlib)
 
   This program generates an index to speed access to archives
 
 USAGE: llvm-ranlib <archive-file>
 
 OPTIONS:
-  -help                             - Display available options
-  -version                          - Display the version of this program
+  -h --help                         - Display available options
+  --version                         - Display the version of this program
 )";
 
-const char ArHelp[] = R"(
-OVERVIEW: LLVM Archiver
+const char ArHelp[] = R"(OVERVIEW: LLVM Archiver
 
 USAGE: llvm-ar [options] [-]<operation>[modifiers] [relpos] [count] <archive> [files]
        llvm-ar -M [<mri-script]
 
 OPTIONS:
   --format              - archive format to create
     =default            -   default
     =gnu                -   gnu
if (Stem.contains_lower("ranlib"))
     outs() << RanlibHelp;
   else if (Stem.contains_lower("ar"))
     outs() << ArHelp;
 }
 
 static unsigned MRILineNumber;
 static bool ParsingMRIScript;
 
+// Show the error plus the usage message, and exit.
+LLVM_ATTRIBUTE_NORETURN static void badUsage(Twine Error) {
+  WithColor::error(errs(), ToolName) << Error << "\n";
+  printHelpMessage();
+  exit(1);
+}
+
 // Show the error message and exit.
 LLVM_ATTRIBUTE_NORETURN static void fail(Twine Error) {
   if (ParsingMRIScript) {
     WithColor::error(errs(), ToolName)
         << "script line " << MRILineNumber << ": " << Error << "\n";
   } else {
     WithColor::error(errs(), ToolName) << Error << "\n";
-    printHelpMessage();
   }
-
   exit(1);
 }
 
 static void failIfError(std::error_code EC, Twine Context = "") {
   if (!EC)
     return;
 
   std::string ContextStr = Context.str();
static void getRelPos() {
   RelPos = PositionalArgs[0];
   PositionalArgs.erase(PositionalArgs.begin());
 }
 
 // Extract the parameter from the command line for the [count] argument
 // associated with the N modifier
 static void getCountParam() {
   if (PositionalArgs.empty())
-    fail("expected [count] for 'N' modifier");
+    badUsage("expected [count] for 'N' modifier");
   auto CountParamArg = StringRef(PositionalArgs[0]);
   if (CountParamArg.getAsInteger(10, CountParam))
-    fail("value for [count] must be numeric, got: " + CountParamArg);
+    badUsage("value for [count] must be numeric, got: " + CountParamArg);
   if (CountParam < 1)
-    fail("value for [count] must be positive, got: " + CountParamArg);
+    badUsage("value for [count] must be positive, got: " + CountParamArg);
   PositionalArgs.erase(PositionalArgs.begin());
 }
 
 // Get the archive file name from the command line
 static void getArchive() {
   if (PositionalArgs.empty())
-    fail("an archive name must be specified");
+    badUsage("an archive name must be specified");
   ArchiveName = PositionalArgs[0];
   PositionalArgs.erase(PositionalArgs.begin());
 }
 
 static object::Archive &readLibrary(const Twine &Library) {
   auto BufOrErr = MemoryBuffer::getFile(Library, -1, false);
   failIfError(BufOrErr.getError(), "could not open library " + Library);
   ArchiveBuffers.push_back(std::move(*BufOrErr));
   auto LibOrErr =
       object::Archive::create(ArchiveBuffers.back()->getMemBufferRef());
   failIfError(errorToErrorCode(LibOrErr.takeError()),
               "could not parse library");
   Archives.push_back(std::move(*LibOrErr));
   return *Archives.back();
 }
 
 static void runMRIScript();
 
 // Parse the command line options as presented and return the operation
 // specified. Process all modifiers and check to make sure that constraints on
 // modifier/operation pairs have not been violated.
 static ArchiveOperation parseCommandLine() {
   if (MRI) {
     if (!PositionalArgs.empty() || !Options.empty())
-      fail("cannot mix -M and other options");
+      badUsage("cannot mix -M and other options");
     runMRIScript();
   }
 
   // Keep track of number of operations. We can only specify one
   // per execution.
   unsigned NumOperations = 0;
 
   // Keep track of the number of positional modifiers (a,b,i). Only
case 'L':
       break;
     case 'V':
       cl::PrintVersionMessage();
       exit(0);
     case 'h':
       printHelpMessage();
       exit(0);
     default:
-      fail(std::string("unknown option ") + Options[i]);
+      badUsage(std::string("unknown option ") + Options[i]);
     }
   }
 
   // At this point, the next thing on the command line must be
   // the archive name.
   getArchive();
 
   // Everything on the command line at this point is a member.
   Members.assign(PositionalArgs.begin(), PositionalArgs.end());
 
   if (NumOperations == 0 && MaybeJustCreateSymTab) {
     NumOperations = 1;
     Operation = CreateSymTab;
     if (!Members.empty())
-      fail("the 's' operation takes only an archive as argument");
+      badUsage("the 's' operation takes only an archive as argument");
   }
 
   // Perform various checks on the operation/modifier specification
   // to make sure we are dealing with a legal request.
   if (NumOperations == 0)
-    fail("you must specify at least one of the operations");
+    badUsage("you must specify at least one of the operations");
   if (NumOperations > 1)
-    fail("only one operation may be specified");
+    badUsage("only one operation may be specified");
   if (NumPositional > 1)
-    fail("you may only specify one of 'a', 'b', and 'i' modifiers");
+    badUsage("you may only specify one of 'a', 'b', and 'i' modifiers");
   if (AddAfter || AddBefore)
     if (Operation != Move && Operation != ReplaceOrInsert)
-      fail("the 'a', 'b' and 'i' modifiers can only be specified with "
+      badUsage("the 'a', 'b' and 'i' modifiers can only be specified with "
                "the 'm' or 'r' operations");
   if (CountParam)
     if (Operation != Extract && Operation != Delete)
-      fail("the 'N' modifier can only be specified with the 'x' or 'd' "
+      badUsage("the 'N' modifier can only be specified with the 'x' or 'd' "
                "operations");
   if (OriginalDates && Operation != Extract)
-    fail("the 'o' modifier is only applicable to the 'x' operation");
+    badUsage("the 'o' modifier is only applicable to the 'x' operation");
   if (OnlyUpdate && Operation != ReplaceOrInsert)
-    fail("the 'u' modifier is only applicable to the 'r' operation");
+    badUsage("the 'u' modifier is only applicable to the 'r' operation");
   if (AddLibrary && Operation != QuickAppend)
-    fail("the 'L' modifier is only applicable to the 'q' operation");
+    badUsage("the 'L' modifier is only applicable to the 'q' operation");
 
   // Return the parsed operation to the caller
   return Operation;
 }
 
 // Implements the 'p' operation. This function traverses the archive
 // looking for members that match the path list.
 static void doPrint(StringRef Name, const object::Archive::Child &C) {
static void runMRIScript() {
   
   // Nothing to do if not saved.
   if (Saved)
     performOperation(ReplaceOrInsert, &NewMembers);
   exit(0);
 }
 
 static bool handleGenericOption(StringRef arg) {
-  if (arg == "-help" || arg == "--help") {
+  if (arg == "-help" || arg == "--help" || arg == "-h") {
     printHelpMessage();
     return true;
   }
   if (arg == "-version" || arg == "--version") {
     cl::PrintVersionMessage();
     return true;
   }
   return false;
if (handleGenericOption(argv[i])) {
       return 0;
     } else {
       if (ArchiveSpecified)
         fail("exactly one archive should be specified");
       ArchiveSpecified = true;
       ArchiveName = argv[i];
     }
   }
+  if (!ArchiveSpecified) {
+    badUsage("an archive name must be specified");
+  }
   return performOperation(CreateSymTab, nullptr);
 }
 
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
   ToolName = argv[0];
 
   llvm::InitializeAllTargetInfos();