[dsymutil] Disallow --reproducer=Use

[keith]

Summary

This should be implied by --use-reproducer instead as a path is required
for this mode

[JDevlieghere]

I'm wondering if we shouldn't just hide the Use mode from the user and make that an implementation detail. The help string for the --reproducer option already says:

Specify the reproducer generation mode.

Limiting the option to /generation/ modes (GenerateOnExit, GenerateOnCrash and Off) makes sense to me conceptually.

The help string for --use-reproducer is also wrong:

Use the object files from the given reproducer path. Alias for --reproducer=Use.

The flag is not an alias but rather implies --reproducer=Use which could be an implementation detail.

[keith]

Disallow Use reproducer mode

[keith]

Yea I wondered the same thing but went with the initial approach since it was in the help doc. Changed to just disallow that value instead

[JDevlieghere]

Thanks, this LGTM!

llvm/tools/dsymutil/Options.td

def threads: Separate<["--", "-"], "num-threads">,
   Group<grp_general>;
 def: Separate<["-"], "j">,
   MetaVarName<"<threads>">,
   HelpText<"Alias for --num-threads">,
   Group<grp_general>;
 
 def reproducer: Separate<["--", "-"], "reproducer">,
   MetaVarName<"<mode>">,
-  HelpText<"Specify the reproducer generation mode. Valid options are 'GenerateOnExit', 'GenerateOnCrash', 'Use', 'Off'.">,
+  HelpText<"Specify the reproducer generation mode. Valid options are 'GenerateOnExit', 'GenerateOnCrash', 'Off'.">,
   Group<grp_general>;
 def: Joined<["--", "-"], "reproducer=">, Alias<reproducer>;
 
 def gen_reproducer: F<"gen-reproducer">,
   HelpText<"Generate a reproducer consisting of the input object files. Alias for --reproducer=GenerateOnExit.">,
   Group<grp_general>;
 
 def use_reproducer: Separate<["--", "-"], "use-reproducer">,
   MetaVarName<"<path>">,
-  HelpText<"Use the object files from the given reproducer path. Alias for --reproducer=Use.">,
+  HelpText<"Use the object files from the given reproducer path.">,
   Group<grp_general>;
 def: Joined<["--", "-"], "use-reproducer=">, Alias<use_reproducer>;
 
 def remarks_prepend_path: Separate<["--", "-"], "remarks-prepend-path">,
   MetaVarName<"<path>">,
   HelpText<"Specify a directory to prepend to the paths of the external remark files.">,
   Group<grp_general>;
 def: Joined<["--", "-"], "remarks-prepend-path=">, Alias<remarks_prepend_path>;
 
 def remarks_output_format: Separate<["--", "-"], "remarks-output-format">,
   MetaVarName<"<format>">,
   HelpText<"Specify the format to be used when serializing the linked remarks.">,
   Group<grp_general>;
 def: Joined<["--", "-"], "remarks-output-format=">, Alias<remarks_output_format>;

llvm/tools/dsymutil/dsymutil.cpp

static Expected<ReproducerMode> getReproducerMode(opt::InputArgList &Args) {
   if (Args.hasArg(OPT_gen_reproducer))
     return ReproducerMode::GenerateOnExit;
   if (opt::Arg *Reproducer = Args.getLastArg(OPT_reproducer)) {
     StringRef S = Reproducer->getValue();
     if (S == "GenerateOnExit")
       return ReproducerMode::GenerateOnExit;
     if (S == "GenerateOnCrash")
       return ReproducerMode::GenerateOnCrash;
-    if (S == "Use")
-      return ReproducerMode::Use;
     if (S == "Off")
       return ReproducerMode::Off;
     return make_error<StringError>(
         "invalid reproducer mode: '" + S +
             "'. Supported values are 'GenerateOnExit', 'GenerateOnCrash', "
-            "'Use', 'Off'.",
+            "'Off'.",
         inconvertibleErrorCode());
   }
   return ReproducerMode::GenerateOnCrash;
 }
 
 static Expected<DWARFVerify> getVerifyKind(opt::InputArgList &Args) {
   if (Args.hasArg(OPT_verify))
     return DWARFVerify::Output;
int main(int argc, char **argv) {
 
   if (Args.hasArg(OPT_version)) {
     cl::PrintVersionMessage();
     return EXIT_SUCCESS;
   }
 
   auto OptionsOrErr = getOptions(Args);
   if (!OptionsOrErr) {
-    WithColor::error() << toString(OptionsOrErr.takeError());
+    WithColor::error() << toString(OptionsOrErr.takeError()) << '\n';
     return EXIT_FAILURE;
   }
 
   auto &Options = *OptionsOrErr;
 
   InitializeAllTargetInfos();
   InitializeAllTargetMCs();
   InitializeAllTargets();
   InitializeAllAsmPrinters();
 
   auto Repro = Reproducer::createReproducer(Options.ReproMode,
                                             Options.ReproducerPath, argc, argv);
   if (!Repro) {
-    WithColor::error() << toString(Repro.takeError());
+    WithColor::error() << toString(Repro.takeError()) << '\n';
     return EXIT_FAILURE;
   }
 
   Options.LinkOpts.VFS = (*Repro)->getVFS();
 
   for (const auto &Arch : Options.Archs)
     if (Arch != "*" && Arch != "all" &&
         !object::MachOObjectFile::isValidArch(Arch)) {