[clang-tools-extra] b662651 - [clangd] Use command line adjusters for inserting compile flags

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2021-06-17T09:24:53+02:00
New Revision: b662651586bedd5350914f64463fe415105785c8

URL: https://github.com/llvm/llvm-project/commit/b662651586bedd5350914f64463fe415105785c8
DIFF: https://github.com/llvm/llvm-project/commit/b662651586bedd5350914f64463fe415105785c8.diff

LOG: [clangd] Use command line adjusters for inserting compile flags

This fixes issues with `--` in the compile flags.

Fixes https://github.com/clangd/clangd/issues/632.

Differential Revision: https://reviews.llvm.org/D99523

Added: 
    

Modified: 
    clang-tools-extra/clangd/CompileCommands.cpp
    clang-tools-extra/clangd/ConfigCompile.cpp
    clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
    clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index 7966b7dfa8a3d..633d13b8b9f0d 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -20,6 +20,8 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include <string>
+#include <vector>
 
 namespace clang {
 namespace clangd {
@@ -209,14 +211,20 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
   Cmd = tooling::getStripPluginsAdjuster()(Cmd, "");
   Cmd = tooling::getClangSyntaxOnlyAdjuster()(Cmd, "");
 
+  std::vector<std::string> ToAppend;
   if (ResourceDir && !Has("-resource-dir"))
-    Cmd.push_back(("-resource-dir=" + *ResourceDir));
+    ToAppend.push_back(("-resource-dir=" + *ResourceDir));
 
   // Don't set `-isysroot` if it is already set or if `--sysroot` is set.
   // `--sysroot` is a superset of the `-isysroot` argument.
   if (Sysroot && !Has("-isysroot") && !Has("--sysroot")) {
-    Cmd.push_back("-isysroot");
-    Cmd.push_back(*Sysroot);
+    ToAppend.push_back("-isysroot");
+    ToAppend.push_back(*Sysroot);
+  }
+
+  if (!ToAppend.empty()) {
+    Cmd = tooling::getInsertArgumentAdjuster(
+        std::move(ToAppend), tooling::ArgumentInsertPosition::END)(Cmd, "");
   }
 
   if (!Cmd.empty()) {
@@ -504,7 +512,6 @@ 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);

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 16d66f6154f25..438dd74d866c6 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -47,6 +47,7 @@
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/SourceMgr.h"
+#include <algorithm>
 #include <string>
 
 namespace clang {
@@ -270,7 +271,9 @@ struct FragmentCompiler {
         Add.push_back(std::move(*A));
       Out.Apply.push_back([Add(std::move(Add))](const Params &, Config &C) {
         C.CompileFlags.Edits.push_back([Add](std::vector<std::string> &Args) {
-          Args.insert(Args.end(), Add.begin(), Add.end());
+          // The point to insert at. Just append when `--` isn't present.
+          auto It = llvm::find(Args, "--");
+          Args.insert(It, Add.begin(), Add.end());
         });
       });
     }

diff  --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
index 33a4afb6374c4..7fb2ae7664fa7 100644
--- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -41,13 +41,14 @@ TEST(CommandMangler, Everything) {
   Mangler.ClangPath = testPath("fake/clang");
   Mangler.ResourceDir = testPath("fake/resources");
   Mangler.Sysroot = testPath("fake/sysroot");
-  std::vector<std::string> Cmd = {"clang++", "-Xclang", "-load", "-Xclang",
-                                  "plugin",  "-MF",     "dep",   "foo.cc"};
+  std::vector<std::string> Cmd = {"clang++", "-Xclang", "-load",
+                                  "-Xclang", "plugin",  "-MF",
+                                  "dep",     "--",      "foo.cc"};
   Mangler.adjust(Cmd);
-  EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "foo.cc",
-                               "-fsyntax-only",
+  EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"), "-fsyntax-only",
                                "-resource-dir=" + testPath("fake/resources"),
-                               "-isysroot", testPath("fake/sysroot")));
+                               "-isysroot", testPath("fake/sysroot"), "--",
+                               "foo.cc"));
 }
 
 TEST(CommandMangler, ResourceDir) {
@@ -378,4 +379,3 @@ TEST(PrintArgvTest, All) {
 } // namespace
 } // namespace clangd
 } // namespace clang
-

diff  --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
index 5e7fce8016f12..381180381f36f 100644
--- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -123,12 +123,12 @@ TEST_F(ConfigCompileTests, Condition) {
 TEST_F(ConfigCompileTests, CompileCommands) {
   Frag.CompileFlags.Add.emplace_back("-foo");
   Frag.CompileFlags.Remove.emplace_back("--include-directory=");
-  std::vector<std::string> Argv = {"clang", "-I", "bar/", "a.cc"};
+  std::vector<std::string> Argv = {"clang", "-I", "bar/", "--", "a.cc"};
   EXPECT_TRUE(compileAndApply());
   EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(2));
   for (auto &Edit : Conf.CompileFlags.Edits)
     Edit(Argv);
-  EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo"));
+  EXPECT_THAT(Argv, ElementsAre("clang", "-foo", "--", "a.cc"));
 }
 
 TEST_F(ConfigCompileTests, CompilationDatabase) {