[clang-tools-extra] 0683a1e - [clangd] Adjust compile flags so they work when applied to other file(type)s.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-01-04T16:10:27+01:00
New Revision: 0683a1e588ade04bba2572e5ab6cf1361ed392d4

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

LOG: [clangd] Adjust compile flags so they work when applied to other file(type)s.

It's reasonable to want to use the command from one file to compile another.
In particular, the command from a translation unit to parse a related header:
  {"file": "foo.h", "command": "clang foo.cpp"}

This is largely what InterpolatingCompilationDatabase tries to do.
To do this correctly can require nontrivial changes to the argv, because the
file extension affects semantics.  e.g. here we must add "-x c++header".

When external tools compile commands for different files, we should apply the
same adjustments. This is better than telling people to "fix their tools":
 - simple e.g. python scripts shouldn't have to interpret clang argv
 - this is a good way to represent the intent "parse header X in the context of
   file Y", which can work even if X is not self-contained. clangd does not
   support this today, but some other tools do, and we may one day.

This issue is discussed in https://github.com/clangd/clangd/issues/519

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

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp b/clang-tools-extra/clangd/CompileCommands.cpp
index d707bf69eded..5c98e40a87fd 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -241,16 +241,38 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd,
   if (ArchOptCount < 2)
     IndicesToDrop.clear();
 
+  // In some cases people may try to reuse the command from another file, e.g.
+  //   { File: "foo.h", CommandLine: "clang foo.cpp" }.
+  // We assume the intent is to parse foo.h the same way as foo.cpp, or as if
+  // it were being included from foo.cpp.
+  //
+  // We're going to rewrite the command to refer to foo.h, and this may change
+  // its semantics (e.g. by parsing the file as C). If we do this, we should
+  // use transferCompileCommand to adjust the argv.
+  // In practice only the extension of the file matters, so do this only when
+  // it 
diff ers.
+  llvm::StringRef FileExtension = llvm::sys::path::extension(File);
+  llvm::Optional<std::string> TransferFrom;
+  auto SawInput = [&](llvm::StringRef Input) {
+    if (llvm::sys::path::extension(Input) != FileExtension)
+      TransferFrom.emplace(Input);
+  };
+
   // Strip all the inputs and `--`. We'll put the input for the requested file
   // explicitly at the end of the flags. This ensures modifications done in the
   // following steps apply in more cases (like setting -x, which only affects
   // inputs that come after it).
-  for (auto *Input : ArgList.filtered(driver::options::OPT_INPUT))
+  for (auto *Input : ArgList.filtered(driver::options::OPT_INPUT)) {
+    SawInput(Input->getValue(0));
     IndicesToDrop.push_back(Input->getIndex());
+  }
   // Anything after `--` is also treated as input, drop them as well.
   if (auto *DashDash =
           ArgList.getLastArgNoClaim(driver::options::OPT__DASH_DASH)) {
-    Cmd.resize(DashDash->getIndex() + 1); // +1 to account for Cmd[0].
+    auto DashDashIndex = DashDash->getIndex() + 1; // +1 accounts for Cmd[0]
+    for (unsigned I = DashDashIndex; I < Cmd.size(); ++I)
+      SawInput(Cmd[I]);
+    Cmd.resize(DashDashIndex);
   }
   llvm::sort(IndicesToDrop);
   llvm::for_each(llvm::reverse(IndicesToDrop),
@@ -262,6 +284,24 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd,
   Cmd.push_back("--");
   Cmd.push_back(File.str());
 
+  if (TransferFrom) {
+    tooling::CompileCommand TransferCmd;
+    TransferCmd.Filename = std::move(*TransferFrom);
+    TransferCmd.CommandLine = std::move(Cmd);
+    TransferCmd = transferCompileCommand(std::move(TransferCmd), File);
+    Cmd = std::move(TransferCmd.CommandLine);
+
+    // Restore the canonical "driver --opts -- filename" form we expect.
+    // FIXME: This is ugly and coupled. Make transferCompileCommand ensure it?
+    assert(!Cmd.empty() && Cmd.back() == File);
+    Cmd.pop_back();
+    if (!Cmd.empty() && Cmd.back() == "--")
+      Cmd.pop_back();
+    assert(!llvm::is_contained(Cmd, "--"));
+    Cmd.push_back("--");
+    Cmd.push_back(File.str());
+  }
+
   for (auto &Edit : Config::current().CompileFlags.Edits)
     Edit(Cmd);
 

diff  --git a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
index 4cb6ef9a1661..3afcf59ac077 100644
--- a/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -53,6 +53,18 @@ TEST(CommandMangler, Everything) {
                                "foo.cc"));
 }
 
+TEST(CommandMangler, FilenameMismatch) {
+  auto Mangler = CommandMangler::forTests();
+  Mangler.ClangPath = testPath("clang");
+  // Our compile flags refer to foo.cc...
+  std::vector<std::string> Cmd = {"clang", "foo.cc"};
+  // but we're applying it to foo.h...
+  Mangler.adjust(Cmd, "foo.h");
+  // so transferCompileCommand should add -x c++-header to preserve semantics.
+  EXPECT_THAT(
+      Cmd, ElementsAre(testPath("clang"), "-x", "c++-header", "--", "foo.h"));
+}
+
 TEST(CommandMangler, ResourceDir) {
   auto Mangler = CommandMangler::forTests();
   Mangler.ResourceDir = testPath("fake/resources");