[Driver] Use enumeration for quoting mode. NFC

[smeenai]

Summary

Boolean parameters are generally hard to understand, especially when we
don't consistently have a comment for them. Change to an enumeration.

While I believe this change is worthwhile by itself, its main purpose is
to serve as cleanup for a follow-up which will add a third mode to this
enumeration, to allow verbatim printing of arguments (without any
quoting or escaping).

[phosek]

LGTM

[rnk]

lgtm+2

clang/include/clang/Driver/Job.h

class Command {
 
   /// Encodes an array of C strings into a single string separated by whitespace.
   /// This function will also put in quotes arguments that have whitespaces and
   /// will escape the regular backslashes (used in Windows paths) and quotes.
   /// The results are the contents of a response file, written into a raw_ostream.
   void writeResponseFile(raw_ostream &OS) const;
 
 public:
+  enum class PrintingMode {
+    AlwaysQuote,
+    QuoteIfNeeded,
+  };
+
   Command(const Action &Source, const Tool &Creator, const char *Executable,
           const llvm::opt::ArgStringList &Arguments,
           ArrayRef<InputInfo> Inputs);
   // FIXME: This really shouldn't be copyable, but is currently copied in some
   // error handling in Driver::generateCompilationDiagnostics.
   Command(const Command &) = default;
   virtual ~Command() = default;
 
-  virtual void Print(llvm::raw_ostream &OS, const char *Terminator, bool Quote,
+  virtual void Print(llvm::raw_ostream &OS, const char *Terminator,
+                     PrintingMode Mode,
                      CrashReportInfo *CrashInfo = nullptr) const;
 
   virtual int Execute(ArrayRef<Optional<StringRef>> Redirects,
                       std::string *ErrMsg, bool *ExecutionFailed) const;
 
   /// getSource - Return the Action which caused the creation of this job.
   const Action &getSource() const { return Source; }
 
public:
   ///         from the parent process will be used.
   void setEnvironment(llvm::ArrayRef<const char *> NewEnvironment);
 
   const char *getExecutable() const { return Executable; }
 
   const llvm::opt::ArgStringList &getArguments() const { return Arguments; }
 
   /// Print a command argument, and optionally quote it.
-  static void printArg(llvm::raw_ostream &OS, StringRef Arg, bool Quote);
+  static void printArg(llvm::raw_ostream &OS, StringRef Arg, PrintingMode Mode);
 
   /// Set whether to print the input filenames when executing.
   void setPrintInputFilenames(bool P) { PrintInputFilenames = P; }
 };
 
 /// Like Command, but with a fallback which is executed in case
 /// the primary command crashes.
 class FallbackCommand : public Command {
 public:
   FallbackCommand(const Action &Source_, const Tool &Creator_,
                   const char *Executable_,
                   const llvm::opt::ArgStringList &Arguments_,
                   ArrayRef<InputInfo> Inputs,
                   std::unique_ptr<Command> Fallback_);
 
-  void Print(llvm::raw_ostream &OS, const char *Terminator, bool Quote,
+  void Print(llvm::raw_ostream &OS, const char *Terminator, PrintingMode Mode,
              CrashReportInfo *CrashInfo = nullptr) const override;
 
   int Execute(ArrayRef<Optional<StringRef>> Redirects, std::string *ErrMsg,
               bool *ExecutionFailed) const override;
 
 private:
   std::unique_ptr<Command> Fallback;
 };
 
 /// Like Command, but always pretends that the wrapped command succeeded.
 class ForceSuccessCommand : public Command {
 public:
   ForceSuccessCommand(const Action &Source_, const Tool &Creator_,
                       const char *Executable_,
                       const llvm::opt::ArgStringList &Arguments_,
                       ArrayRef<InputInfo> Inputs);
 
-  void Print(llvm::raw_ostream &OS, const char *Terminator, bool Quote,
+  void Print(llvm::raw_ostream &OS, const char *Terminator, PrintingMode Mode,
              CrashReportInfo *CrashInfo = nullptr) const override;
 
   int Execute(ArrayRef<Optional<StringRef>> Redirects, std::string *ErrMsg,
               bool *ExecutionFailed) const override;
 };
 
 /// JobList - A sequence of jobs to perform.
 class JobList {
 public:
   using list_type = SmallVector<std::unique_ptr<Command>, 4>;
   using size_type = list_type::size_type;
   using iterator = llvm::pointee_iterator<list_type::iterator>;
   using const_iterator = llvm::pointee_iterator<list_type::const_iterator>;
 
 private:
   list_type Jobs;
 
 public:
   void Print(llvm::raw_ostream &OS, const char *Terminator,
-             bool Quote, CrashReportInfo *CrashInfo = nullptr) const;
+             Command::PrintingMode Mode,
+             CrashReportInfo *CrashInfo = nullptr) const;
 
   /// Add a job to the list (taking ownership).
   void addJob(std::unique_ptr<Command> J) { Jobs.push_back(std::move(J)); }
 
   /// Clear the job list.
   void clear();
 
   const list_type &getJobs() const { return Jobs; }

clang/lib/Driver/Compilation.cpp

if (getDriver().CCPrintOptions && getDriver().CCPrintOptionsFilename) {
         return 1;
       }
       OS = OwnedStream.get();
     }
 
     if (getDriver().CCPrintOptions)
       *OS << "[Logging clang options]";
 
-    C.Print(*OS, "\n", /*Quote=*/getDriver().CCPrintOptions);
+    C.Print(*OS, "\n",
+            getDriver().CCPrintOptions ? Command::PrintingMode::AlwaysQuote
+                                       : Command::PrintingMode::QuoteIfNeeded);
   }
 
   std::string Error;
   bool ExecutionFailed;
   int Res = C.Execute(Redirects, &Error, &ExecutionFailed);
   if (!Error.empty()) {
     assert(Res && "Error string set with 0 result code!");
     getDriver().Diag(diag::err_drv_command_failure) << Error;

clang/lib/Driver/Driver.cpp

 static void printArgList(raw_ostream &OS, const llvm::opt::ArgList &Args) {
   llvm::opt::ArgStringList ASL;
   for (const auto *A : Args)
     A->render(Args, ASL);
 
   for (auto I = ASL.begin(), E = ASL.end(); I != E; ++I) {
     if (I != ASL.begin())
       OS << ' ';
-    Command::printArg(OS, *I, true);
+    Command::printArg(OS, *I, Command::PrintingMode::AlwaysQuote);
   }
   OS << '\n';
 }
 
 bool Driver::getCrashDiagnosticFile(StringRef ReproCrashFilename,
                                     SmallString<128> &CrashDiagDir) {
   using namespace llvm::sys;
   assert(llvm::Triple(llvm::sys::getProcessTriple()).isOSDarwin() &&
void Driver::generateCompilationDiagnostics(
   if (EC) {
     Diag(clang::diag::note_drv_command_failed_diag_msg)
         << "Error generating run script: " << Script << " " << EC.message();
   } else {
     ScriptOS << "# Crash reproducer for " << getClangFullVersion() << "\n"
              << "# Driver args: ";
     printArgList(ScriptOS, C.getInputArgs());
     ScriptOS << "# Original command: ";
-    Cmd.Print(ScriptOS, "\n", /*Quote=*/true);
-    Cmd.Print(ScriptOS, "\n", /*Quote=*/true, &CrashInfo);
+    Cmd.Print(ScriptOS, "\n", Command::PrintingMode::AlwaysQuote);
+    Cmd.Print(ScriptOS, "\n", Command::PrintingMode::AlwaysQuote, &CrashInfo);
     if (!AdditionalInformation.empty())
       ScriptOS << "\n# Additional information: " << AdditionalInformation
                << "\n";
     if (Report)
       Report->TemporaryFiles.push_back(Script.str());
     Diag(clang::diag::note_drv_command_failed_diag_msg) << Script;
   }
 
void Driver::setUpResponseFiles(Compilation &C, Command &Cmd) {
   Cmd.setResponseFile(C.addTempFile(C.getArgs().MakeArgString(TmpName)));
 }
 
 int Driver::ExecuteCompilation(
     Compilation &C,
     SmallVectorImpl<std::pair<int, const Command *>> &FailingCommands) {
   // Just print if -### was present.
   if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH)) {
-    C.getJobs().Print(llvm::errs(), "\n", true);
+    C.getJobs().Print(llvm::errs(), "\n", Command::PrintingMode::AlwaysQuote);
     return 0;
   }
 
   // If there were errors building the compilation, quit now.
   if (Diags.hasErrorOccurred())
     return 1;
 
   // Set up response file names for each command, if necessary

clang/lib/Driver/Job.cpp

if (IsInclude)
     return !HaveCrashVFS;
   if (FlagRef.startswith("-fmodules-cache-path="))
     return true;
 
   SkipNum = 0;
   return false;
 }
 
-void Command::printArg(raw_ostream &OS, StringRef Arg, bool Quote) {
+void Command::printArg(raw_ostream &OS, StringRef Arg, PrintingMode Mode) {
   const bool Escape = Arg.find_first_of(" \"\\$") != StringRef::npos;
 
-  if (!Quote && !Escape) {
+  if (Mode == PrintingMode::QuoteIfNeeded && !Escape) {
     OS << Arg;
     return;
   }
 
   // Quote and escape. This isn't really complete, but good enough.
   OS << '"';
   for (const auto c : Arg) {
     if (c == '"' || c == '\\' || c == '$')
rewriteIncludes(const llvm::ArrayRef<const char *> &Args, size_t Idx,
   assert(NumArgs == 2 && "Not expecting more than two arguments");
   StringRef Inc(Args[Idx + NumArgs - 1]);
   if (!getAbsPath(Inc, NewInc))
     return;
   IncFlags.push_back(SmallString<128>(Args[Idx]));
   IncFlags.push_back(std::move(NewInc));
 }
 
-void Command::Print(raw_ostream &OS, const char *Terminator, bool Quote,
+void Command::Print(raw_ostream &OS, const char *Terminator, PrintingMode Mode,
                     CrashReportInfo *CrashInfo) const {
   // Always quote the exe.
   OS << ' ';
-  printArg(OS, Executable, /*Quote=*/true);
+  printArg(OS, Executable, PrintingMode::AlwaysQuote);
 
   ArrayRef<const char *> Args = Arguments;
   SmallVector<const char *, 128> ArgsRespFile;
   if (ResponseFile != nullptr) {
     buildArgvForResponseFile(ArgsRespFile);
     Args = ArrayRef<const char *>(ArgsRespFile).slice(1); // no executable name
   }
 
if (CrashInfo) {
 
       // Relative includes need to be expanded to absolute paths.
       if (HaveCrashVFS && IsInclude) {
         SmallVector<SmallString<128>, 2> NewIncFlags;
         rewriteIncludes(Args, i, NumArgs, NewIncFlags);
         if (!NewIncFlags.empty()) {
           for (auto &F : NewIncFlags) {
             OS << ' ';
-            printArg(OS, F.c_str(), Quote);
+            printArg(OS, F.c_str(), Mode);
           }
           i += NumArgs - 1;
           continue;
         }
       }
 
       auto Found = llvm::find_if(InputFilenames,
                                  [&Arg](StringRef IF) { return IF == Arg; });
       if (Found != InputFilenames.end() &&
           (i == 0 || StringRef(Args[i - 1]) != "-main-file-name")) {
         // Replace the input file name with the crashinfo's file name.
         OS << ' ';
         StringRef ShortName = llvm::sys::path::filename(CrashInfo->Filename);
-        printArg(OS, ShortName.str(), Quote);
+        printArg(OS, ShortName.str(), Mode);
         continue;
       }
     }
 
     OS << ' ';
-    printArg(OS, Arg, Quote);
+    printArg(OS, Arg, Mode);
   }
 
   if (CrashInfo && HaveCrashVFS) {
     OS << ' ';
-    printArg(OS, "-ivfsoverlay", Quote);
+    printArg(OS, "-ivfsoverlay", Mode);
     OS << ' ';
-    printArg(OS, CrashInfo->VFSPath.str(), Quote);
+    printArg(OS, CrashInfo->VFSPath.str(), Mode);
 
     // The leftover modules from the crash are stored in
     //  <name>.cache/vfs/modules
     // Leave it untouched for pcm inspection and provide a clean/empty dir
     // path to contain the future generated module cache:
     //  <name>.cache/vfs/repro-modules
     SmallString<128> RelModCacheDir = llvm::sys::path::parent_path(
         llvm::sys::path::parent_path(CrashInfo->VFSPath));
     llvm::sys::path::append(RelModCacheDir, "repro-modules");
 
     std::string ModCachePath = "-fmodules-cache-path=";
     ModCachePath.append(RelModCacheDir.c_str());
 
     OS << ' ';
-    printArg(OS, ModCachePath, Quote);
+    printArg(OS, ModCachePath, Mode);
   }
 
   if (ResponseFile != nullptr) {
     OS << "\n Arguments passed via response file:\n";
     writeResponseFile(OS);
     // Avoiding duplicated newline terminator, since FileLists are
     // newline-separated.
     if (Creator.getResponseFilesSupport() != Tool::RF_FileList)
FallbackCommand::FallbackCommand(const Action &Source_, const Tool &Creator_,
                                  const char *Executable_,
                                  const llvm::opt::ArgStringList &Arguments_,
                                  ArrayRef<InputInfo> Inputs,
                                  std::unique_ptr<Command> Fallback_)
     : Command(Source_, Creator_, Executable_, Arguments_, Inputs),
       Fallback(std::move(Fallback_)) {}
 
 void FallbackCommand::Print(raw_ostream &OS, const char *Terminator,
-                            bool Quote, CrashReportInfo *CrashInfo) const {
-  Command::Print(OS, "", Quote, CrashInfo);
+                            PrintingMode Mode,
+                            CrashReportInfo *CrashInfo) const {
+  Command::Print(OS, "", Mode, CrashInfo);
   OS << " ||";
-  Fallback->Print(OS, Terminator, Quote, CrashInfo);
+  Fallback->Print(OS, Terminator, Mode, CrashInfo);
 }
 
 static bool ShouldFallback(int ExitCode) {
   // FIXME: We really just want to fall back for internal errors, such
   // as when some symbol cannot be mangled, when we should be able to
   // parse something but can't, etc.
   return ExitCode != 0;
 }

 }
 
 ForceSuccessCommand::ForceSuccessCommand(
     const Action &Source_, const Tool &Creator_, const char *Executable_,
     const llvm::opt::ArgStringList &Arguments_, ArrayRef<InputInfo> Inputs)
     : Command(Source_, Creator_, Executable_, Arguments_, Inputs) {}
 
 void ForceSuccessCommand::Print(raw_ostream &OS, const char *Terminator,
-                            bool Quote, CrashReportInfo *CrashInfo) const {
-  Command::Print(OS, "", Quote, CrashInfo);
+                                PrintingMode Mode,
+                                CrashReportInfo *CrashInfo) const {
+  Command::Print(OS, "", Mode, CrashInfo);
   OS << " || (exit 0)" << Terminator;
 }
 
 int ForceSuccessCommand::Execute(ArrayRef<llvm::Optional<StringRef>> Redirects,
                                  std::string *ErrMsg,
                                  bool *ExecutionFailed) const {
   int Status = Command::Execute(Redirects, ErrMsg, ExecutionFailed);
   (void)Status;
   if (ExecutionFailed)
     *ExecutionFailed = false;
   return 0;
 }
 
-void JobList::Print(raw_ostream &OS, const char *Terminator, bool Quote,
+void JobList::Print(raw_ostream &OS, const char *Terminator,
+                    Command::PrintingMode Mode,
                     CrashReportInfo *CrashInfo) const {
   for (const auto &Job : *this)
-    Job.Print(OS, Terminator, Quote, CrashInfo);
+    Job.Print(OS, Terminator, Mode, CrashInfo);
 }
 
 void JobList::clear() { Jobs.clear(); }

clang/lib/Frontend/CreateInvocationFromCommandLine.cpp

std::unique_ptr<CompilerInvocation> clang::createInvocationFromCommandLine(
   TheDriver.setCheckInputsExist(false);
 
   std::unique_ptr<driver::Compilation> C(TheDriver.BuildCompilation(Args));
   if (!C)
     return nullptr;
 
   // Just print the cc1 options if -### was present.
   if (C->getArgs().hasArg(driver::options::OPT__HASH_HASH_HASH)) {
-    C->getJobs().Print(llvm::errs(), "\n", true);
+    C->getJobs().Print(llvm::errs(), "\n",
+                       driver::Command::PrintingMode::AlwaysQuote);
     return nullptr;
   }
 
   // We expect to get back exactly one command job, if we didn't something
   // failed. Offload compilation is an exception as it creates multiple jobs. If
   // that's the case, we proceed with the first job. If caller needs a
   // particular job, it should be controlled via options (e.g.
   // --cuda-{host|device}-only for CUDA) passed to the driver.
for (auto &A : C->getActions()){
         break;
       }
     }
   }
   if (Jobs.size() == 0 || !isa<driver::Command>(*Jobs.begin()) ||
       (Jobs.size() > 1 && !OffloadCompilation)) {
     SmallString<256> Msg;
     llvm::raw_svector_ostream OS(Msg);
-    Jobs.Print(OS, "; ", true);
+    Jobs.Print(OS, "; ", driver::Command::PrintingMode::AlwaysQuote);
     Diags->Report(diag::err_fe_expected_compiler_job) << OS.str();
     return nullptr;
   }
 
   const driver::Command &Cmd = cast<driver::Command>(*Jobs.begin());
   if (StringRef(Cmd.getCreator().getName()) != "clang") {
     Diags->Report(diag::err_fe_expected_clang_command);
     return nullptr;

clang/lib/Tooling/Tooling.cpp

 static const llvm::opt::ArgStringList *getCC1Arguments(
     DiagnosticsEngine *Diagnostics, driver::Compilation *Compilation) {
   // We expect to get back exactly one Command job, if we didn't something
   // failed. Extract that job from the Compilation.
   const driver::JobList &Jobs = Compilation->getJobs();
   if (Jobs.size() != 1 || !isa<driver::Command>(*Jobs.begin())) {
     SmallString<256> error_msg;
     llvm::raw_svector_ostream error_stream(error_msg);
-    Jobs.Print(error_stream, "; ", true);
+    Jobs.Print(error_stream, "; ", driver::Command::PrintingMode::AlwaysQuote);
     Diagnostics->Report(diag::err_fe_expected_compiler_job)
         << error_stream.str();
     return nullptr;
   }
 
   // The one job we find should be to invoke clang again.
   const auto &Cmd = cast<driver::Command>(*Jobs.begin());
   if (StringRef(Cmd.getCreator().getName()) != "clang") {

 
 bool ToolInvocation::runInvocation(
     const char *BinaryName, driver::Compilation *Compilation,
     std::shared_ptr<CompilerInvocation> Invocation,
     std::shared_ptr<PCHContainerOperations> PCHContainerOps) {
   // Show the invocation, with -v.
   if (Invocation->getHeaderSearchOpts().Verbose) {
     llvm::errs() << "clang Invocation:\n";
-    Compilation->getJobs().Print(llvm::errs(), "\n", true);
+    Compilation->getJobs().Print(llvm::errs(), "\n",
+                                 driver::Command::PrintingMode::AlwaysQuote);
     llvm::errs() << "\n";
   }
 
   return Action->runInvocation(std::move(Invocation), Files,
                                std::move(PCHContainerOps), DiagConsumer);
 }
 
 bool FrontendActionFactory::runInvocation(