[llvm-ar] Update error messages and tests as per latest preferred style

[sameerarora101]

Summary

It updates two error messages under performOperation
in the file llvm-ar.cpp. Furthermore, it also updates
tests that print out these error msgs:
llvm/test/Object/ar-create.test and
llvm/test/tools/llvm-ar/print.test.
Depends on D80838

[smeenai]

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

663

I'm guessing this was done by the linter as part of its formatting? It's a valid change, but it shouldn't be part of this diff.

Depending on how people who contribute to llvm-ar (e.g. @MaskRay) feel, we can either commit the formatting changes separately or just undo them (and ignore any lint warnings about them).

1031

Ditto here.

[sameerarora101]

Add lower case i to the test message in print.test

[jhenderson]

Seems reasonable to me, but could you clarify why you are making the change? What is the latest "preferred style" you mean here?

[sameerarora101]

@jhenderson There were two types of error messages being thrown before: error loading ... and unable to load ... So the idea is to make the error messages consistent (instead of having 2 different formats). Furthermore, since unable to load ... was updated more recently and is the preferred style going forward (as per @smeenai ), we should change the former to match the new message.

[sameerarora101]

add colon to test titles.

[smeenai]

As we discussed in https://bugs.llvm.org/show_bug.cgi?id=40244, some of the error messages here were still inconsistent, and this makes them consistent (with the style that was decided on in D67038).

[jhenderson]

LGTM, with the formatting changes undone, and the test changes rebased once the new tests from D80838 are moved out.

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

663

My personal take: if you are making changes in the immediate area (e.g. within a couple of lines), then it's okay to make the formatting change, either as part of, or in addition to the current patch, but that doesn't apply here, so I think this and the other formatting change below should be reverted.

[sameerarora101]

Rebasing on new D80838.

[MaskRay]

Thanks!

[jhenderson]

Looks good again, once the test is fixed in the parent revision.

[sameerarora101]

Rebase again on D80838.

[sameerarora101]

Rebase again on D80838: splitting the directory and permission cases into separate files.

[sameerarora101]

Rebasing on D80838: updating comment "with chmod".

[sameerarora101]

NFC: adding branch (git).

llvm/test/Object/ar-create.test

 Test which operations create an archive and which don't.
 
 RUN: touch %t
 RUN: rm -f %t.foo.a
 RUN: not llvm-ar p %t.foo.a %t 2>&1 | FileCheck %s
 RUN: not llvm-ar d %t.foo.a %t 2>&1 | FileCheck %s
 RUN: not llvm-ar m %t.foo.a %t 2>&1 | FileCheck %s
 RUN: not llvm-ar t %t.foo.a %t 2>&1 | FileCheck %s
 RUN: not llvm-ar x %t.foo.a %t 2>&1 | FileCheck %s
 
 RUN: llvm-ar q %t.foo.a %t 2>&1 | FileCheck --check-prefix=CREATE %s
 RUN: rm -f %t.foo.a
 RUN: llvm-ar r %t.foo.a %t 2>&1 | FileCheck --check-prefix=CREATE %s
 RUN: rm -f %t.foo.a
 
-CHECK: llvm-ar{{(.exe|.EXE)?}}: error: error loading '{{[^']+}}.foo.a':
+CHECK: llvm-ar{{(.exe|.EXE)?}}: error: unable to load '{{[^']+}}.foo.a':
 CREATE: creating {{.*}}.foo.a

llvm/test/tools/llvm-ar/error-opening-directory.test

 # RUN: rm -rf %t && mkdir -p %t
 
 ## Passing in a directory:
 # RUN: mkdir -p %t/tmpDir
 # RUN: not llvm-ar p %t/tmpDir 2>&1 | \
 # RUN:    FileCheck %s --check-prefix=IS-DIR -DARCHIVE=%t/tmpDir
 
-# IS-DIR: error: error opening '[[ARCHIVE]]': {{[iI]}}s a directory
+# IS-DIR: error: unable to open '[[ARCHIVE]]': {{[iI]}}s a directory

llvm/test/tools/llvm-ar/error-opening-permission.test

 ## Unsupported on windows as marking files "unreadable"
 ## is non-trivial on windows.
 # UNSUPPORTED: system-windows
 
 # RUN: rm -rf %t && mkdir -p %t
 # RUN: echo file1 > %t/1.txt
 
 ## No Permission:
 # RUN: llvm-ar rc %t/permission.b %t/1.txt
 # RUN: chmod 100 %t/permission.b
 # RUN: not llvm-ar p %t/permission.b 2>&1 | \
 # RUN:   FileCheck %s --check-prefix=NO-PERMISSION -DARCHIVE=%t/permission.b
 
-# NO-PERMISSION: error: error opening '[[ARCHIVE]]': {{[pP]}}ermission denied
+# NO-PERMISSION: error: unable to open '[[ARCHIVE]]': {{[pP]}}ermission denied

llvm/test/tools/llvm-ar/print.test

 # RUN:   | FileCheck %s --check-prefix=NO-ARCHIVE
 # 
 # NO-ARCHIVE: error: an archive name must be specified
 
 ## Archive does not exist:
 # RUN: not llvm-ar p %t/missing.a 2>&1 \
 # RUN:   | FileCheck %s --check-prefix=MISSING-ARCHIVE -DARCHIVE=%t/missing.a
 
-# MISSING-ARCHIVE: error: error loading '[[ARCHIVE]]': {{[nN]}}o such file or directory
+# MISSING-ARCHIVE: error: unable to load '[[ARCHIVE]]': {{[nN]}}o such file or directory
 
 ## Member does not exist:
 # RUN: not llvm-ar p %t/archive.a %t-missing.txt 2>&1 \
 # RUN:   | FileCheck %s --check-prefix=MISSING-FILE -DFILE=%t-missing.txt
 
 # MISSING-FILE: error: '[[FILE]]' was not found
 
 ## Print thin archive:
 # RUN: llvm-ar Trc %t/thin-archive.a %t/1.txt %t/2.txt %t/3.txt
 # RUN: llvm-ar p %t/archive.a %t/2.txt \
 # RUN:   | FileCheck %s --check-prefix=SINGLE --match-full-lines --implicit-check-not {{.}}

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

 
 static std::string normalizePath(StringRef Path) {
   return CompareFullPath ? sys::path::convert_to_slash(Path)
                          : std::string(sys::path::filename(Path));
 }
 
 static bool comparePaths(StringRef Path1, StringRef Path2) {
 // When on Windows this function calls CompareStringOrdinal
-// as Windows file paths are case-insensitive. 
+// as Windows file paths are case-insensitive.
 // CompareStringOrdinal compares two Unicode strings for
 // binary equivalence and allows for case insensitivity.
 #ifdef _WIN32
   SmallVector<wchar_t, 128> WPath1, WPath2;
   failIfError(sys::windows::UTF8ToUTF16(normalizePath(Path1), WPath1));
   failIfError(sys::windows::UTF8ToUTF16(normalizePath(Path2), WPath2));
 
   return CompareStringOrdinal(WPath1.data(), WPath1.size(), WPath2.data(),
static void addChildMember(std::vector<NewArchiveMember> &Members,
   failIfError(NMOrErr.takeError());
   // If the child member we're trying to add is thin, use the path relative to
   // the archive it's in, so the file resolves correctly.
   if (Thin && FlattenArchive) {
     StringSaver Saver(Alloc);
     Expected<std::string> FileNameOrErr(M.getName());
     failIfError(FileNameOrErr.takeError());
     if (sys::path::is_absolute(*FileNameOrErr)) {
       NMOrErr->MemberName = Saver.save(sys::path::convert_to_slash(*FileNameOrErr));
     } else {
       FileNameOrErr = M.getFullName();
       failIfError(FileNameOrErr.takeError());
       Expected<std::string> PathOrErr =
           computeArchiveRelativePath(ArchiveName, *FileNameOrErr);
       NMOrErr->MemberName = Saver.save(
           PathOrErr ? *PathOrErr : sys::path::convert_to_slash(*FileNameOrErr));
     }

 
 static int performOperation(ArchiveOperation Operation,
                             std::vector<NewArchiveMember> *NewMembers) {
   // Create or open the archive object.
   ErrorOr<std::unique_ptr<MemoryBuffer>> Buf =
       MemoryBuffer::getFile(ArchiveName, -1, false);
   std::error_code EC = Buf.getError();
   if (EC && EC != errc::no_such_file_or_directory)
-    fail("error opening '" + ArchiveName + "': " + EC.message());
+    fail("unable to open '" + ArchiveName + "': " + EC.message());
 
   if (!EC) {
     Error Err = Error::success();
     object::Archive Archive(Buf.get()->getMemBufferRef(), Err);
     failIfError(std::move(Err), "unable to load '" + ArchiveName + "'");
     if (Archive.isThin())
       CompareFullPath = true;
     performOperation(Operation, &Archive, std::move(Buf.get()), NewMembers);
     return 0;
   }
 
   assert(EC == errc::no_such_file_or_directory);
 
   if (!shouldCreateArchive(Operation)) {
-    failIfError(EC, Twine("error loading '") + ArchiveName + "'");
+    failIfError(EC, Twine("unable to load '") + ArchiveName + "'");
   } else {
     if (!Create) {
       // Produce a warning if we should and we're creating the archive
       WithColor::warning(errs(), ToolName)
           << "creating " << ArchiveName << "\n";
     }
   }
 
   performOperation(Operation, nullptr, nullptr, NewMembers);
   return 0;
 }
 
 static void runMRIScript() {
   enum class MRICommand { AddLib, AddMod, Create, CreateThin, Delete, Save, End, Invalid };
 
   ErrorOr<std::unique_ptr<MemoryBuffer>> Buf = MemoryBuffer::getSTDIN();
   failIfError(Buf.getError());
   const MemoryBuffer &Ref = *Buf.get();
   bool Saved = false;
   std::vector<NewArchiveMember> NewMembers;
   ParsingMRIScript = true;
 
case MRICommand::Save:
       Saved = true;
       break;
     case MRICommand::End:
       break;
     case MRICommand::Invalid:
       fail("unknown command: " + CommandStr);
     }
   }
-  
   ParsingMRIScript = false;
-  
   // Nothing to do if not saved.
   if (Saved)
     performOperation(ReplaceOrInsert, &NewMembers);
   exit(0);
 }
 
 static bool handleGenericOption(StringRef arg) {
   if (arg == "-help" || arg == "--help" || arg == "-h") {