Skip to content

Commit

Permalink
Revert "Revert "[clangd] Adjust compile flags to contain only the req…
Browse files Browse the repository at this point in the history
…uested file as input""

This reverts commit 04e21fb.
  • Loading branch information
kadircet committed Jul 27, 2021
1 parent ab714ba commit 259e365
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 27 deletions.
26 changes: 17 additions & 9 deletions clang-tools-extra/clangd/CompileCommands.cpp
Expand Up @@ -196,7 +196,8 @@ CommandMangler CommandMangler::detect() {

CommandMangler CommandMangler::forTests() { return CommandMangler(); }

void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
void CommandMangler::adjust(std::vector<std::string> &Cmd,
llvm::StringRef File) const {
trace::Span S("AdjustCompileFlags");
auto &OptTable = clang::driver::getDriverOptTable();
// OriginalArgs needs to outlive ArgList.
Expand All @@ -211,8 +212,10 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
// ParseArgs propagates missig arg/opt counts on error, but preserves
// everything it could parse in ArgList. So we just ignore those counts.
unsigned IgnoredCount;
// Drop the executable name, as ParseArgs doesn't expect it. This means
// indices are actually of by one between ArgList and OriginalArgs.
auto ArgList = OptTable.ParseArgs(
OriginalArgs, IgnoredCount, IgnoredCount,
llvm::makeArrayRef(OriginalArgs).drop_front(), IgnoredCount, IgnoredCount,
/*FlagsToInclude=*/
IsCLMode ? (driver::options::CLOption | driver::options::CoreOption)
: /*everything*/ 0,
Expand All @@ -223,12 +226,17 @@ void CommandMangler::adjust(std::vector<std::string> &Cmd) const {
// modifications done in the following steps apply in more cases (like setting
// -x, which only affects inputs that come after it).
if (!ArgList.hasArgNoClaim(driver::options::OPT__DASH_DASH)) {
// In theory there might be more than one input, but clangd can't deal with
// them anyway.
if (auto *Input = ArgList.getLastArg(driver::options::OPT_INPUT)) {
Cmd.insert(Cmd.end(), {"--", Input->getAsString(ArgList)});
Cmd.erase(Cmd.begin() + Input->getIndex());
}
// Drop all the inputs and only add one for the current file.
llvm::SmallVector<unsigned, 1> IndicesToDrop;
for (auto *Input : ArgList.filtered(driver::options::OPT_INPUT))
IndicesToDrop.push_back(Input->getIndex());
llvm::sort(IndicesToDrop);
llvm::for_each(llvm::reverse(IndicesToDrop),
// +1 to account for the executable name in Cmd[0] that
// doesn't exist in ArgList.
[&Cmd](unsigned Idx) { Cmd.erase(Cmd.begin() + Idx + 1); });
Cmd.push_back("--");
Cmd.push_back(File.str());
}

for (auto &Edit : Config::current().CompileFlags.Edits)
Expand Down Expand Up @@ -278,7 +286,7 @@ CommandMangler::operator clang::tooling::ArgumentsAdjuster() && {
return [Mangler = std::make_shared<CommandMangler>(std::move(*this))](
const std::vector<std::string> &Args, llvm::StringRef File) {
auto Result = Args;
Mangler->adjust(Result);
Mangler->adjust(Result, File);
return Result;
};
}
Expand Down
3 changes: 2 additions & 1 deletion clang-tools-extra/clangd/CompileCommands.h
Expand Up @@ -12,6 +12,7 @@
#include "clang/Tooling/ArgumentsAdjusters.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include <deque>
#include <string>
#include <vector>
Expand Down Expand Up @@ -46,7 +47,7 @@ struct CommandMangler {
// - on mac, find clang and isysroot by querying the `xcrun` launcher
static CommandMangler detect();

void adjust(std::vector<std::string> &Cmd) const;
void adjust(std::vector<std::string> &Cmd, llvm::StringRef File) const;
explicit operator clang::tooling::ArgumentsAdjuster() &&;

private:
Expand Down
4 changes: 2 additions & 2 deletions clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
Expand Up @@ -763,7 +763,7 @@ OverlayCDB::getCompileCommand(PathRef File) const {
if (!Cmd)
return llvm::None;
if (ArgsAdjuster)
Cmd->CommandLine = ArgsAdjuster(Cmd->CommandLine, Cmd->Filename);
Cmd->CommandLine = ArgsAdjuster(Cmd->CommandLine, File);
return Cmd;
}

Expand All @@ -773,7 +773,7 @@ tooling::CompileCommand OverlayCDB::getFallbackCommand(PathRef File) const {
Cmd.CommandLine.insert(Cmd.CommandLine.end(), FallbackFlags.begin(),
FallbackFlags.end());
if (ArgsAdjuster)
Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, Cmd.Filename);
Cmd.CommandLine = ArgsAdjuster(Cmd.CommandLine, File);
return Cmd;
}

Expand Down
Expand Up @@ -48,7 +48,7 @@
#
# ERR: ASTWorker building file {{.*}}foo.c version 0 with command
# ERR: [{{.*}}clangd-test2]
# ERR: clang -c -Wall -Werror {{.*}} -- foo.c
# ERR: clang -c -Wall -Werror {{.*}} -- {{.*}}foo.c
---
{"jsonrpc":"2.0","id":5,"method":"shutdown"}
---
Expand Down
36 changes: 23 additions & 13 deletions clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
Expand Up @@ -44,7 +44,7 @@ TEST(CommandMangler, Everything) {
Mangler.ResourceDir = testPath("fake/resources");
Mangler.Sysroot = testPath("fake/sysroot");
std::vector<std::string> Cmd = {"clang++", "--", "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
EXPECT_THAT(Cmd, ElementsAre(testPath("fake/clang++"),
"-resource-dir=" + testPath("fake/resources"),
"-isysroot", testPath("fake/sysroot"), "--",
Expand All @@ -55,7 +55,7 @@ TEST(CommandMangler, ResourceDir) {
auto Mangler = CommandMangler::forTests();
Mangler.ResourceDir = testPath("fake/resources");
std::vector<std::string> Cmd = {"clang++", "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
EXPECT_THAT(Cmd, Contains("-resource-dir=" + testPath("fake/resources")));
}

Expand All @@ -64,7 +64,7 @@ TEST(CommandMangler, Sysroot) {
Mangler.Sysroot = testPath("fake/sysroot");

std::vector<std::string> Cmd = {"clang++", "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
EXPECT_THAT(llvm::join(Cmd, " "),
HasSubstr("-isysroot " + testPath("fake/sysroot")));
}
Expand All @@ -74,19 +74,19 @@ TEST(CommandMangler, ClangPath) {
Mangler.ClangPath = testPath("fake/clang");

std::vector<std::string> Cmd = {"clang++", "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
EXPECT_EQ(testPath("fake/clang++"), Cmd.front());

Cmd = {"unknown-binary", "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
EXPECT_EQ(testPath("fake/unknown-binary"), Cmd.front());

Cmd = {testPath("path/clang++"), "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
EXPECT_EQ(testPath("path/clang++"), Cmd.front());

Cmd = {"foo/unknown-binary", "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
EXPECT_EQ("foo/unknown-binary", Cmd.front());
}

Expand Down Expand Up @@ -129,7 +129,7 @@ TEST(CommandMangler, ClangPathResolve) {
auto Mangler = CommandMangler::forTests();
Mangler.ClangPath = testPath("fake/clang");
std::vector<std::string> Cmd = {(TempDir + "/bin/foo").str(), "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
// Directory based on resolved symlink, basename preserved.
EXPECT_EQ((TempDir + "/lib/foo").str(), Cmd.front());

Expand All @@ -146,13 +146,13 @@ TEST(CommandMangler, ClangPathResolve) {
Mangler.ClangPath = testPath("fake/clang");
// Driver found on PATH.
Cmd = {"foo", "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
// Found the symlink and resolved the path as above.
EXPECT_EQ((TempDir + "/lib/foo").str(), Cmd.front());

// Symlink not resolved with -no-canonical-prefixes.
Cmd = {"foo", "-no-canonical-prefixes", "foo.cc"};
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
EXPECT_EQ((TempDir + "/bin/foo").str(), Cmd.front());
}
#endif
Expand All @@ -171,7 +171,7 @@ TEST(CommandMangler, ConfigEdits) {
Argv = tooling::getInsertArgumentAdjuster("--hello")(Argv, "");
});
WithContextValue WithConfig(Config::Key, std::move(Cfg));
Mangler.adjust(Cmd);
Mangler.adjust(Cmd, "foo.cc");
}
// Edits are applied in given order and before other mangling and they always
// go before filename.
Expand Down Expand Up @@ -350,7 +350,7 @@ TEST(CommandMangler, InputsAfterDashDash) {
const auto Mangler = CommandMangler::forTests();
{
std::vector<std::string> Args = {"clang", "/Users/foo.cc"};
Mangler.adjust(Args);
Mangler.adjust(Args, "/Users/foo.cc");
EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2),
ElementsAre("--", "/Users/foo.cc"));
EXPECT_THAT(llvm::makeArrayRef(Args).drop_back(2),
Expand All @@ -361,11 +361,21 @@ TEST(CommandMangler, InputsAfterDashDash) {
{
std::vector<std::string> Args = {"clang", "--driver-mode=cl", "bar.cc",
"/Users/foo.cc"};
Mangler.adjust(Args);
Mangler.adjust(Args, "bar.cc");
EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2),
ElementsAre("--", "bar.cc"));
EXPECT_THAT(llvm::makeArrayRef(Args).drop_back(2), Not(Contains("bar.cc")));
}
// All inputs but the main file is dropped.
{
std::vector<std::string> Args = {"clang", "foo.cc", "bar.cc"};
Mangler.adjust(Args, "baz.cc");
EXPECT_THAT(llvm::makeArrayRef(Args).take_back(2),
ElementsAre("--", "baz.cc"));
EXPECT_THAT(
llvm::makeArrayRef(Args).drop_back(2),
testing::AllOf(Not(Contains("foo.cc")), Not(Contains("bar.cc"))));
}
}
} // namespace
} // namespace clangd
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/unittests/TestTU.cpp
Expand Up @@ -59,7 +59,7 @@ ParseInputs TestTU::inputs(MockFS &FS) const {
Argv.push_back(FullFilename);

auto Mangler = CommandMangler::forTests();
Mangler.adjust(Inputs.CompileCommand.CommandLine);
Mangler.adjust(Inputs.CompileCommand.CommandLine, FullFilename);
Inputs.CompileCommand.Filename = FullFilename;
Inputs.CompileCommand.Directory = testRoot();
Inputs.Contents = Code;
Expand Down

0 comments on commit 259e365

Please sign in to comment.