hamzasood updated this revision to Diff 162872.
hamzasood added a comment.
- Re-uploaded with full context.
Yeah, I encountered these issues while using clangd on Windows. I haven't run
into any clangd issues after applying this patch, but admittedly my usage is
very basic (pretty much just code completion). It may well be subtly broken in
other places.
https://reviews.llvm.org/D51321
Files:
lib/Tooling/InterpolatingCompilationDatabase.cpp
unittests/Tooling/CompilationDatabaseTest.cpp
Index: unittests/Tooling/CompilationDatabaseTest.cpp
===================================================================
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -14,6 +14,8 @@
#include "clang/Tooling/JSONCompilationDatabase.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/Support/Path.h"
+
+#define GTEST_HAS_COMBINE 1
#include "gtest/gtest.h"
namespace clang {
@@ -644,15 +646,50 @@
}
};
-class InterpolateTest : public ::testing::Test {
+enum class DriverExe { Clang, ClangCL };
+enum class DriverMode { None, GCC, CL };
+
+class InterpolateTest
+ : public ::testing::TestWithParam<std::tuple<DriverExe, DriverMode>> {
+
protected:
+ bool isTestingCL() const {
+ switch (std::get<1>(GetParam())) {
+ case DriverMode::None: return std::get<0>(GetParam()) == DriverExe::ClangCL;
+ case DriverMode::GCC: return false;
+ case DriverMode::CL: return true;
+ default: llvm_unreachable("Invalid DriverMode");
+ }
+ }
+ StringRef getClangExe() const {
+ switch (std::get<0>(GetParam())) {
+ case DriverExe::Clang: return "clang";
+ case DriverExe::ClangCL: return "clang-cl";
+ default: llvm_unreachable("Invalid DriverExe");
+ }
+ }
+ StringRef getDriverModeArg() const {
+ switch (std::get<1>(GetParam())) {
+ case DriverMode::None: return StringRef();
+ case DriverMode::GCC: return "--driver-mode=gcc";
+ case DriverMode::CL: return "--driver-mode=cl";
+ default: llvm_unreachable("Invalid DriverMode");
+ }
+ }
+
// Adds an entry to the underlying compilation database.
// A flag is injected: -D <File>, so the command used can be identified.
void add(llvm::StringRef File, llvm::StringRef Flags = "") {
- llvm::SmallVector<StringRef, 8> Argv = {"clang", File, "-D", File};
+ llvm::SmallVector<StringRef, 8> Argv;
+ Argv.push_back(getClangExe());
+ if (!getDriverModeArg().empty())
+ Argv.push_back(getDriverModeArg());
+ Argv.append({"-D", File});
llvm::SplitString(Flags, Argv);
+
llvm::SmallString<32> Dir;
llvm::sys::path::system_temp_directory(false, Dir);
+
Entries[path(File)].push_back(
{Dir, path(File), {Argv.begin(), Argv.end()}, "foo.o"});
}
@@ -675,67 +712,101 @@
->getCompileCommands(path(F));
if (Results.empty())
return "none";
- // drop the input file argument, so tests don't have to deal with path().
- EXPECT_EQ(Results[0].CommandLine.back(), path(F))
- << "Last arg should be the file";
- Results[0].CommandLine.pop_back();
- return llvm::join(Results[0].CommandLine, " ");
+
+ ArrayRef<std::string> CmdLine = Results[0].CommandLine;
+
+ // Drop the clang executable path.
+ EXPECT_EQ(CmdLine.front(), getClangExe()) << "First arg should be clang";
+ CmdLine = CmdLine.drop_front();
+
+ // Drop the --driver-mode flag.
+ if (!getDriverModeArg().empty()) {
+ EXPECT_EQ(CmdLine.front(), getDriverModeArg())
+ << "Expected driver mode arg";
+ CmdLine = CmdLine.drop_front();
+ }
+
+ // Drop the input file.
+ EXPECT_EQ(CmdLine.back(), path(F)) << "Last arg should be the file";
+ CmdLine = CmdLine.drop_back();
+
+ return llvm::join(CmdLine, " ");
}
MemCDB::EntryMap Entries;
};
-TEST_F(InterpolateTest, Nearby) {
+TEST_P(InterpolateTest, Nearby) {
add("dir/foo.cpp");
add("dir/bar.cpp");
add("an/other/foo.cpp");
// great: dir and name both match (prefix or full, case insensitive)
- EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
- EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+ EXPECT_EQ(getCommand("dir/f.cpp"), "-D dir/foo.cpp");
+ EXPECT_EQ(getCommand("dir/FOO.cpp"), "-D dir/foo.cpp");
// no name match. prefer matching dir, break ties by alpha
- EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+ EXPECT_EQ(getCommand("dir/a.cpp"), "-D dir/bar.cpp");
// an exact name match beats one segment of directory match
EXPECT_EQ(getCommand("some/other/bar.h"),
- "clang -D dir/bar.cpp -x c++-header");
+ isTestingCL()
+ ? "-D dir/bar.cpp /TP"
+ : "-D dir/bar.cpp -x c++-header");
// two segments of directory match beat a prefix name match
- EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+ EXPECT_EQ(getCommand("an/other/b.cpp"), "-D an/other/foo.cpp");
// if nothing matches at all, we still get the closest alpha match
EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
- "clang -D an/other/foo.cpp");
+ "-D an/other/foo.cpp");
}
-TEST_F(InterpolateTest, Language) {
- add("dir/foo.cpp", "-std=c++17");
- add("dir/baz.cee", "-x c");
+TEST_P(InterpolateTest, Language) {
+ add("dir/foo.cpp", isTestingCL() ? "/std:c++17" : "-std=c++17");
+ add("dir/baz.cee", isTestingCL() ? "/TC" : "-x c");
// .h is ambiguous, so we add explicit language flags
EXPECT_EQ(getCommand("foo.h"),
- "clang -D dir/foo.cpp -x c++-header -std=c++17");
+ isTestingCL()
+ ? "-D dir/foo.cpp /TP /std:c++17"
+ : "-D dir/foo.cpp -x c++-header -std=c++17");
// and don't add -x if the inferred language is correct.
- EXPECT_EQ(getCommand("foo.hpp"), "clang -D dir/foo.cpp -std=c++17");
+ EXPECT_EQ(getCommand("foo.hpp"),
+ isTestingCL()
+ ? "-D dir/foo.cpp /std:c++17"
+ : "-D dir/foo.cpp -std=c++17");
// respect -x if it's already there.
- EXPECT_EQ(getCommand("baz.h"), "clang -D dir/baz.cee -x c-header");
+ EXPECT_EQ(getCommand("baz.h"),
+ isTestingCL()
+ ? "-D dir/baz.cee /TC"
+ : "-D dir/baz.cee -x c-header");
// prefer a worse match with the right language
- EXPECT_EQ(getCommand("foo.c"), "clang -D dir/baz.cee");
+ EXPECT_EQ(getCommand("foo.c"), "-D dir/baz.cee");
Entries.erase(path(StringRef("dir/baz.cee")));
// Now we transfer across languages, so drop -std too.
- EXPECT_EQ(getCommand("foo.c"), "clang -D dir/foo.cpp");
+ EXPECT_EQ(getCommand("foo.c"), "-D dir/foo.cpp");
}
-TEST_F(InterpolateTest, Strip) {
- add("dir/foo.cpp", "-o foo.o -Wall");
- // the -o option and the input file are removed, but -Wall is preserved.
- EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
+TEST_P(InterpolateTest, Strip) {
+ // The input and output are removed, but the warning level is preserved.
+ if (isTestingCL()) {
+ add("dir/foo.cpp", "/Fofoo.o /W3");
+ EXPECT_EQ(getCommand("dir/bar.cpp"), "-D dir/foo.cpp /W3");
+ } else {
+ add("dir/foo.cpp", "-o foo.o -Wall");
+ EXPECT_EQ(getCommand("dir/bar.cpp"), "-D dir/foo.cpp -Wall");
+ }
}
-TEST_F(InterpolateTest, Case) {
+TEST_P(InterpolateTest, Case) {
add("FOO/BAR/BAZ/SHOUT.cc");
add("foo/bar/baz/quiet.cc");
// Case mismatches are completely ignored, so we choose the name match.
- EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "clang -D FOO/BAR/BAZ/SHOUT.cc");
+ EXPECT_EQ(getCommand("foo/bar/baz/shout.C"), "-D FOO/BAR/BAZ/SHOUT.cc");
}
+INSTANTIATE_TEST_CASE_P(RegularAndCL, InterpolateTest,
+ ::testing::Combine(
+ ::testing::Values(DriverExe::Clang, DriverExe::ClangCL),
+ ::testing::Values(DriverMode::None, DriverMode::GCC, DriverMode::CL)));
+
TEST(CompileCommandTest, EqualityOperator) {
CompileCommand CCRef("/foo/bar", "hello.c", {"a", "b"}, "hello.o");
CompileCommand CCTest = CCRef;
Index: lib/Tooling/InterpolatingCompilationDatabase.cpp
===================================================================
--- lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -48,6 +48,7 @@
#include "clang/Frontend/LangStandard.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringSwitch.h"
#include "llvm/Option/ArgList.h"
@@ -119,55 +120,96 @@
}
}
+// Determine whether the given file path is the clang-cl executable.
+static bool checkIsCLMode(const std::vector<std::string> &CmdLine,
+ const llvm::opt::OptTable &OptTable) {
+ const std::string DriverModeName =
+ OptTable.getOption(driver::options::OPT_driver_mode).getPrefixedName();
+
+ // First check for a --driver-mode override.
+ for (StringRef S : llvm::reverse(CmdLine)) {
+ if (S.startswith(DriverModeName))
+ return S.drop_front(DriverModeName.length()) == "cl";
+ }
+
+ // Otherwise just check the clang file name.
+ return llvm::sys::path::stem(CmdLine.front()).endswith_lower("clang-cl");
+}
+
// A CompileCommand that can be applied to another file.
struct TransferableCommand {
// Flags that should not apply to all files are stripped from CommandLine.
CompileCommand Cmd;
// Language detected from -x or the filename.
types::ID Type = types::TY_INVALID;
// Standard specified by -std.
LangStandard::Kind Std = LangStandard::lang_unspecified;
+ // Whether the command is for the CL driver.
+ bool IsCLMode = false;
TransferableCommand(CompileCommand C)
: Cmd(std::move(C)), Type(guessType(Cmd.Filename)) {
- std::vector<std::string> NewArgs = {Cmd.CommandLine.front()};
+ // Stash the old arguments.
+ std::vector<std::string> OldArgs = std::move(Cmd.CommandLine);
+ Cmd.CommandLine.clear();
+
+ // Transform the command line to an llvm ArgList.
+ // Make sure that OldArgs lives for at least as long as this variable as the
+ // arg list contains pointers to the OldArgs strings.
+ llvm::opt::InputArgList ArgList;
+ {
+ // Unfortunately InputArgList requires an array of c-strings whereas we
+ // have an array of std::string; we'll need an intermediate vector.
+ SmallVector<const char *, 8> TmpArgv;
+ TmpArgv.reserve(OldArgs.size());
+ std::transform(OldArgs.begin(), OldArgs.end(),
+ std::back_inserter(TmpArgv),
+ [](const std::string &S) { return S.c_str(); });
+ ArgList = llvm::opt::InputArgList(TmpArgv.begin(), TmpArgv.end());
+ }
+
// Parse the old args in order to strip out and record unwanted flags.
auto OptTable = clang::driver::createDriverOptTable();
- std::vector<const char *> Argv;
- for (unsigned I = 1; I < Cmd.CommandLine.size(); ++I)
- Argv.push_back(Cmd.CommandLine[I].c_str());
- unsigned MissingI, MissingC;
- auto ArgList = OptTable->ParseArgs(Argv, MissingI, MissingC);
- for (const auto *Arg : ArgList) {
- const auto &option = Arg->getOption();
+ IsCLMode = checkIsCLMode(OldArgs, *OptTable);
+ Cmd.CommandLine.emplace_back(OldArgs.front());
+ for (unsigned Start = 1, End = Start; End < OldArgs.size(); Start = End) {
+ std::unique_ptr<llvm::opt::Arg> Arg;
+ {
+ unsigned Included, Excluded;
+ if (IsCLMode) {
+ Included = driver::options::CoreOption | driver::options::CLOption;
+ Excluded = 0;
+ } else {
+ Included = 0;
+ Excluded = driver::options::CLOption;
+ }
+ Arg = std::unique_ptr<llvm::opt::Arg>(
+ OptTable->ParseOneArg(ArgList, End, Included, Excluded));
+ }
+
+ if (!Arg)
+ continue;
+
// Strip input and output files.
- if (option.matches(clang::driver::options::OPT_INPUT) ||
- option.matches(clang::driver::options::OPT_o)) {
+ if (isUntypedInputOrOutput(*Arg))
continue;
- }
- // Strip -x, but record the overridden language.
- if (option.matches(clang::driver::options::OPT_x)) {
- for (const char *Value : Arg->getValues())
- Type = types::lookupTypeForTypeSpecifier(Value);
+
+ // Strip type specifiers, but record the overridden language.
+ if (const auto GivenType = isTypeSpecArg(*Arg)) {
+ Type = *GivenType;
continue;
}
- // Strip --std, but record the value.
- if (option.matches(clang::driver::options::OPT_std_EQ)) {
- for (const char *Value : Arg->getValues()) {
- Std = llvm::StringSwitch<LangStandard::Kind>(Value)
-#define LANGSTANDARD(id, name, lang, desc, features) \
- .Case(name, LangStandard::lang_##id)
-#define LANGSTANDARD_ALIAS(id, alias) .Case(alias, LangStandard::lang_##id)
-#include "clang/Frontend/LangStandards.def"
- .Default(Std);
- }
+
+ // Strip std, but record the value.
+ if (const auto GivenStd = isStdArg(*Arg)) {
+ if (*GivenStd != LangStandard::lang_unspecified)
+ Std = *GivenStd;
continue;
}
- llvm::opt::ArgStringList ArgStrs;
- Arg->render(ArgList, ArgStrs);
- NewArgs.insert(NewArgs.end(), ArgStrs.begin(), ArgStrs.end());
+
+ Cmd.CommandLine.insert(Cmd.CommandLine.end(),
+ &OldArgs[Start], &OldArgs[End]);
}
- Cmd.CommandLine = std::move(NewArgs);
if (Std != LangStandard::lang_unspecified) // -std take precedence over -x
Type = toType(LangStandard::getLangStandardForKind(Std).getLanguage());
@@ -180,26 +222,81 @@
Result.Filename = Filename;
bool TypeCertain;
auto TargetType = guessType(Filename, &TypeCertain);
- // If the filename doesn't determine the language (.h), transfer with -x.
+ // If the filename doesn't determine the language (.h), use an appropriate
+ // argument to transfer it.
if (!TypeCertain) {
TargetType = types::onlyPrecompileType(TargetType) // header?
? types::lookupHeaderTypeForSourceType(Type)
: Type;
- Result.CommandLine.push_back("-x");
- Result.CommandLine.push_back(types::getTypeName(TargetType));
+ if (IsCLMode) {
+ const StringRef Flag = toCLFlag(TargetType);
+ if (!Flag.empty())
+ Result.CommandLine.push_back(Flag);
+ } else {
+ Result.CommandLine.push_back("-x");
+ Result.CommandLine.push_back(types::getTypeName(TargetType));
+ }
}
// --std flag may only be transferred if the language is the same.
// We may consider "translating" these, e.g. c++11 -> c11.
if (Std != LangStandard::lang_unspecified && foldType(TargetType) == Type) {
Result.CommandLine.push_back(
- "-std=" +
+ (IsCLMode ? "/std:" : "-std=") +
std::string(LangStandard::getLangStandardForKind(Std).getName()));
}
Result.CommandLine.push_back(Filename);
return Result;
}
private:
+ bool isUntypedInputOrOutput(const llvm::opt::Arg &Arg) const {
+ const llvm::opt::Option &Opt = Arg.getOption();
+
+ if (Opt.matches(driver::options::OPT_INPUT))
+ return true;
+
+ if (IsCLMode)
+ return Opt.matches(driver::options::OPT__SLASH_Fa) ||
+ Opt.matches(driver::options::OPT__SLASH_Fe) ||
+ Opt.matches(driver::options::OPT__SLASH_Fi) ||
+ Opt.matches(driver::options::OPT__SLASH_Fo);
+ else
+ return Opt.matches(driver::options::OPT_o);
+ }
+
+ Optional<types::ID> isTypeSpecArg(const llvm::opt::Arg &Arg) const {
+ const llvm::opt::Option &Opt = Arg.getOption();
+ if (IsCLMode) {
+ if (Opt.matches(driver::options::OPT__SLASH_TC) ||
+ Opt.matches(driver::options::OPT__SLASH_Tc))
+ return types::TY_C;
+ if (Opt.matches(driver::options::OPT__SLASH_TP) ||
+ Opt.matches(driver::options::OPT__SLASH_Tp))
+ return types::TY_CXX;
+ } else {
+ if (Opt.matches(driver::options::OPT_x))
+ return types::lookupTypeForTypeSpecifier(Arg.getValue());
+ }
+ return None;
+ }
+
+ Optional<LangStandard::Kind> isStdArg(const llvm::opt::Arg &Arg) const {
+ const llvm::opt::Option &Opt = Arg.getOption();
+ if (Opt.matches(IsCLMode ? driver::options::OPT__SLASH_std
+ : driver::options::OPT_std_EQ)) {
+ return llvm::StringSwitch<LangStandard::Kind>(Arg.getValue())
+#define LANGSTANDARD(id, name, lang, desc, features) \
+ .Case(name, LangStandard::lang_##id)
+#define LANGSTANDARD_ALIAS(id, alias) \
+ .Case(alias, LangStandard::lang_##id)
+#include "clang/Frontend/LangStandards.def"
+#undef LANGSTANDARD_ALIAS
+#undef LANGSTANDARD
+ .Default(LangStandard::lang_unspecified);
+ }
+ return None;
+ }
+
// Map the language from the --std flag to that of the -x flag.
static types::ID toType(InputKind::Language Lang) {
switch (Lang) {
@@ -215,6 +312,20 @@
return types::TY_INVALID;
}
}
+
+ static StringRef toCLFlag(types::ID Type) {
+ types::isCXX(Type);
+ switch (Type) {
+ case types::TY_C:
+ case types::TY_CHeader:
+ return "/TC";
+ case types::TY_CXX:
+ case types::TY_CXXHeader:
+ return "/TP";
+ default:
+ return StringRef();
+ }
+ }
};
// CommandIndex does the real work: given a filename, it produces the best
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
