[llvm-install-name-tool] Validate -id value early

[alexander-shaposhnikov]

Summary

The code which validates the value of -id is moved into the function parseInstallNameToolOptions.

Test plan: make check-all

[alexander-shaposhnikov]

Apply clang-format

[rupprecht]

Looks like llvm/test/tools/llvm-objcopy/MachO/install-name-tool-id.test covers this, so LGTM

[jhenderson]

I guess if you wanted to add explicit testing that the behaviour happens regardless of the presence of specific load commands, that would be possible, using a minimal Mach-O file, but I don't mind if you don't want to. LGTM, if you don't.

llvm/tools/llvm-objcopy/CopyConfig.cpp

for (auto *Arg : InputArgs.filtered(INSTALL_NAME_TOOL_rpath)) {
     if (It3 != Config.RPathToAdd.end())
       return createStringError(errc::invalid_argument,
                                "cannot specify both -add_rpath " + *It3 +
                                    " and -rpath " + Old + " " + New);
 
     Config.RPathsToUpdate.insert({Old, New});
   }
 
-  if (auto *Arg = InputArgs.getLastArg(INSTALL_NAME_TOOL_id))
+  if (auto *Arg = InputArgs.getLastArg(INSTALL_NAME_TOOL_id)) {
     Config.SharedLibId = Arg->getValue();
+    if (Config.SharedLibId->empty())
+      return createStringError(errc::invalid_argument,
+                               "cannot specify an empty id");
+  }
 
-  for (auto *Arg : InputArgs.filtered(INSTALL_NAME_TOOL_change)) {
+  for (auto *Arg : InputArgs.filtered(INSTALL_NAME_TOOL_change))
     Config.InstallNamesToUpdate.insert({Arg->getValue(0), Arg->getValue(1)});
-  }
 
   SmallVector<StringRef, 2> Positional;
   for (auto Arg : InputArgs.filtered(INSTALL_NAME_TOOL_UNKNOWN))
     return createStringError(errc::invalid_argument, "unknown argument '%s'",
                              Arg->getAsString(InputArgs).c_str());
   for (auto Arg : InputArgs.filtered(INSTALL_NAME_TOOL_INPUT))
     Positional.push_back(Arg->getValue());
   if (Positional.empty())

llvm/tools/llvm-objcopy/MachO/MachOObjcopy.cpp

if (RPaths.count(New) != 0)
                                "rpath " + New +
                                    " would create a duplicate load command");
   }
 
   // Update load commands.
   for (LoadCommand &LC : Obj.LoadCommands) {
     switch (LC.MachOLoadCommand.load_command_data.cmd) {
     case MachO::LC_ID_DYLIB:
-      if (Config.SharedLibId) {
-        StringRef Id = Config.SharedLibId.getValue();
-        if (Id.empty())
-          return createStringError(errc::invalid_argument,
-                                   "cannot specify an empty id");
-        updateLoadCommandPayloadString<MachO::dylib_command>(LC, Id);
-      }
+      if (Config.SharedLibId)
+        updateLoadCommandPayloadString<MachO::dylib_command>(
+            LC, *Config.SharedLibId);
       break;
 
     case MachO::LC_RPATH: {
       StringRef RPath = getPayloadString(LC);
       StringRef NewRPath = Config.RPathsToUpdate.lookup(RPath);
       if (!NewRPath.empty())
         updateLoadCommandPayloadString<MachO::rpath_command>(LC, NewRPath);
       break;