[clangd] Quote/escape argv included in log messages.

https://github.com/clangd/clangd/issues/637

diff --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index 96cbd88..b55d1b0 100644 (file)
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -503,5 +503,32 @@ void ArgStripper::process(std::vector<std::string> &Args) const {
   Args.resize(Write);
 }
 
+
+std::string printArgv(llvm::ArrayRef<llvm::StringRef> Args) {
+  std::string Buf;
+  llvm::raw_string_ostream OS(Buf);
+  bool Sep = false;
+  for (llvm::StringRef Arg : Args) {
+    if (Sep)
+      OS << ' ';
+    Sep = true;
+    if (llvm::all_of(Arg, llvm::isPrint) &&
+        Arg.find_first_of(" \t\n\"\\") == llvm::StringRef::npos) {
+      OS << Arg;
+      continue;
+    }
+    OS << '"';
+    OS.write_escaped(Arg, /*UseHexEscapes=*/true);
+    OS << '"';
+  }
+  return std::move(OS.str());
+}
+
+std::string printArgv(llvm::ArrayRef<std::string> Args) {
+  std::vector<llvm::StringRef> Refs(Args.size());
+  llvm::copy(Args, Refs.begin());
+  return printArgv(Refs);
+}
+
 } // namespace clangd
 } // namespace clang

diff --git a/clang-tools-extra/clangd/CompileCommands.h b/clang-tools-extra/clangd/CompileCommands.h
index 2ba17a0..6e958d2 100644 (file)
--- a/clang-tools-extra/clangd/CompileCommands.h
+++ b/clang-tools-extra/clangd/CompileCommands.h
@@ -96,6 +96,11 @@ private:
   std::deque<std::string> Storage; // Store strings not found in option table.
 };
 
+// Renders an argv list, with arguments separated by spaces.
+// Where needed, arguments are "quoted" and escaped.
+std::string printArgv(llvm::ArrayRef<llvm::StringRef> Args);
+std::string printArgv(llvm::ArrayRef<std::string> Args);
+
 } // namespace clangd
 } // namespace clang
 

diff --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
index 0bb2c46..94faec9 100644 (file)
--- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -222,8 +222,8 @@ extractSystemIncludesAndTarget(llvm::SmallString<128> Driver,
   if (int RC = llvm::sys::ExecuteAndWait(Driver, Args, /*Env=*/llvm::None,
                                          Redirects)) {
     elog("System include extraction: driver execution failed with return code: "
-         "{0}. Args: ['{1}']",
-         llvm::to_string(RC), llvm::join(Args, "', '"));
+         "{0}. Args: [{1}]",
+         llvm::to_string(RC), printArgv(Args));
     return llvm::None;
   }
 

diff --git a/clang-tools-extra/clangd/TUScheduler.cpp b/clang-tools-extra/clangd/TUScheduler.cpp
index 1d0ca1f..1cd6699 100644 (file)
--- a/clang-tools-extra/clangd/TUScheduler.cpp
+++ b/clang-tools-extra/clangd/TUScheduler.cpp
@@ -648,7 +648,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags,
     log("ASTWorker building file {0} version {1} with command {2}\n[{3}]\n{4}",
         FileName, Inputs.Version, Inputs.CompileCommand.Heuristic,
         Inputs.CompileCommand.Directory,
-        llvm::join(Inputs.CompileCommand.CommandLine, " "));
+        printArgv(Inputs.CompileCommand.CommandLine));
 
     StoreDiags CompilerInvocationDiagConsumer;
     std::vector<std::string> CC1Args;
@@ -656,7 +656,7 @@ void ASTWorker::update(ParseInputs Inputs, WantDiagnostics WantDiags,
         Inputs, CompilerInvocationDiagConsumer, &CC1Args);
     // Log cc1 args even (especially!) if creating invocation failed.
     if (!CC1Args.empty())
-      vlog("Driver produced command: cc1 {0}", llvm::join(CC1Args, " "));
+      vlog("Driver produced command: cc1 {0}", printArgv(CC1Args));
     std::vector<Diag> CompilerInvocationDiags =
         CompilerInvocationDiagConsumer.take();
     if (!Invocation) {

diff --git a/clang-tools-extra/clangd/tool/Check.cpp b/clang-tools-extra/clangd/tool/Check.cpp
index e42596b..9e3e439 100644 (file)
--- a/clang-tools-extra/clangd/tool/Check.cpp
+++ b/clang-tools-extra/clangd/tool/Check.cpp
@@ -107,10 +107,10 @@ public:
 
     if (auto TrueCmd = CDB->getCompileCommand(File)) {
       Cmd = std::move(*TrueCmd);
-      log("Compile command from CDB is: {0}", llvm::join(Cmd.CommandLine, " "));
+      log("Compile command from CDB is: {0}", printArgv(Cmd.CommandLine));
     } else {
       Cmd = CDB->getFallbackCommand(File);
-      log("Generic fallback command is: {0}", llvm::join(Cmd.CommandLine, " "));
+      log("Generic fallback command is: {0}", printArgv(Cmd.CommandLine));
     }
 
     return true;
@@ -140,7 +140,7 @@ public:
         buildCompilerInvocation(Inputs, CaptureInvocationDiags, &CC1Args);
     auto InvocationDiags = CaptureInvocationDiags.take();
     ErrCount += showErrors(InvocationDiags);
-    log("internal (cc1) args are: {0}", llvm::join(CC1Args, " "));
+    log("internal (cc1) args are: {0}", printArgv(CC1Args));
     if (!Invocation) {
       elog("Failed to parse command line");
       return false;

diff --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
index d911179..2330426 100644 (file)
--- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -64,7 +64,7 @@ TEST(CommandMangler, Sysroot) {
 
   std::vector<std::string> Cmd = {"clang++", "foo.cc"};
   Mangler.adjust(Cmd);
-  EXPECT_THAT(llvm::join(Cmd, " "),
+  EXPECT_THAT(printArgv(Cmd),
               HasSubstr("-isysroot " + testPath("fake/sysroot")));
 }
 
@@ -214,7 +214,7 @@ static std::string strip(llvm::StringRef Arg, llvm::StringRef Argv) {
   ArgStripper S;
   S.strip(Arg);
   S.process(Args);
-  return llvm::join(Args, " ");
+  return printArgv(Args);
 }
 
 TEST(ArgStripperTest, Spellings) {
@@ -367,6 +367,14 @@ TEST(ArgStripperTest, OrderDependent) {
   EXPECT_THAT(Args, ElementsAre("clang", "foo.cc"));
 }
 
+TEST(PrintArgvTest, All) {
+  std::vector<llvm::StringRef> Args = {
+      "one", "two", "thr ee", "f\"o\"ur", "fi\\ve", "$"
+  };
+  const char *Expected = R"(one two "thr ee" "f\"o\"ur" "fi\\ve" $)";
+  EXPECT_EQ(Expected, printArgv(Args));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

diff --git a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
index 7c62955..2ec6412 100644 (file)
--- a/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
+++ b/clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
@@ -348,7 +348,7 @@ MATCHER_P(hasArg, Flag, "") {
     return false;
   }
   if (!llvm::is_contained(arg->CommandLine, Flag)) {
-    *result_listener << "flags are " << llvm::join(arg->CommandLine, " ");
+    *result_listener << "flags are " << printArgv(arg->CommandLine);
     return false;
   }
   return true;
@@ -457,8 +457,7 @@ MATCHER_P2(hasFlag, Flag, Path, "") {
     return false;
   }
   if (!llvm::is_contained(Cmds.front().CommandLine, Flag)) {
-    *result_listener << "flags are: "
-                     << llvm::join(Cmds.front().CommandLine, " ");
+    *result_listener << "flags are: " << printArgv(Cmds.front().CommandLine);
     return false;
   }
   return true;