jnykiel updated this revision to Diff 331570.
jnykiel retitled this revision from "[Tooling] Handle compilation databases
with clang-cl commands generated by CMake 3.19+" to "[Tooling] Handle
compilation databases containing commands with double dashes".
jnykiel edited the summary of this revision.
jnykiel added a comment.
I have addressed some of your comments. The second revision is good enough to
get the tests to pass - the `injectResourceDir` change was necessary for the
one clang-tidy test that started failing after leaving the `--` in. I don't
feel familiar enough with clangd specifically to attempt comprehensive fixes
there. The clangd issue report specifically mentions `-fsyntax-only` and
`-resource-dir` as well - so while not comprehensive, the fix may be good for
that too.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98824/new/
https://reviews.llvm.org/D98824
Files:
clang/lib/Tooling/ArgumentsAdjusters.cpp
clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
clang/lib/Tooling/Tooling.cpp
clang/unittests/Tooling/CompilationDatabaseTest.cpp
Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===================================================================
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -812,6 +812,16 @@
EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall");
}
+TEST_F(InterpolateTest, StripDoubleDash) {
+ add("dir/foo.cpp", "-o foo.o -std=c++14 -Wall -- dir/foo.cpp");
+ // input file and output option are removed
+ // -Wall flag isn't
+ // -std option gets re-added as the last argument before --
+ // new input file gets added after --
+ EXPECT_EQ(getCommand("dir/bar.cpp"),
+ "clang -D dir/foo.cpp -Wall -std=c++14 --");
+}
+
TEST_F(InterpolateTest, Case) {
add("FOO/BAR/BAZ/SHOUT.cc");
add("foo/bar/baz/quiet.cc");
Index: clang/lib/Tooling/Tooling.cpp
===================================================================
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -435,13 +435,18 @@
static void injectResourceDir(CommandLineArguments &Args, const char *Argv0,
void *MainAddr) {
// Allow users to override the resource dir.
- for (StringRef Arg : Args)
+ for (StringRef Arg : Args) {
+ if (Arg == "--")
+ break;
if (Arg.startswith("-resource-dir"))
return;
+ }
// If there's no override in place add our resource dir.
- Args.push_back("-resource-dir=" +
- CompilerInvocation::GetResourcesPath(Argv0, MainAddr));
+ std::string resourceDir = "-resource-dir=";
+ resourceDir.append(CompilerInvocation::GetResourcesPath(Argv0, MainAddr));
+ getInsertArgumentAdjuster(CommandLineArguments(1, resourceDir),
+ ArgumentInsertPosition::END)(Args, "");
}
int ClangTool::run(ToolAction *Action) {
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -132,6 +132,8 @@
LangStandard::Kind Std = LangStandard::lang_unspecified;
// Whether the command line is for the cl-compatible driver.
bool ClangCLMode;
+ // Whether to insert a double dash before the input file(s).
+ bool InsertDoubleDash = false;
TransferableCommand(CompileCommand C)
: Cmd(std::move(C)), Type(guessType(Cmd.Filename)),
@@ -177,6 +179,13 @@
Opt.matches(OPT__SLASH_Fo))))
continue;
+ // ...including when the inputs are passed after --.
+ // If that is the case, record it.
+ if (Opt.matches(OPT__DASH_DASH)) {
+ InsertDoubleDash = true;
+ break;
+ }
+
// Strip -x, but record the overridden language.
if (const auto GivenType = tryParseTypeArg(*Arg)) {
Type = *GivenType;
@@ -235,6 +244,8 @@
llvm::Twine(ClangCLMode ? "/std:" : "-std=") +
LangStandard::getLangStandardForKind(Std).getName()).str());
}
+ if (InsertDoubleDash)
+ Result.CommandLine.push_back("--");
Result.CommandLine.push_back(std::string(Filename));
return Result;
}
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===================================================================
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -41,8 +41,13 @@
"-save-temps",
"--save-temps",
};
- for (size_t i = 0, e = Args.size(); i < e; ++i) {
+ size_t i = 0;
+ for (size_t e = Args.size(); i < e; ++i) {
StringRef Arg = Args[i];
+
+ if (Arg == "--")
+ break;
+
// Skip output commands.
if (llvm::any_of(OutputCommands, [&Arg](llvm::StringRef OutputCommand) {
return Arg.startswith(OutputCommand);
@@ -63,6 +68,8 @@
}
if (!HasSyntaxOnly)
AdjustedArgs.push_back("-fsyntax-only");
+ for (size_t e = Args.size(); i < e; ++i)
+ AdjustedArgs.push_back(Args[i]);
return AdjustedArgs;
};
}
@@ -137,7 +144,7 @@
CommandLineArguments::iterator I;
if (Pos == ArgumentInsertPosition::END) {
- I = Return.end();
+ I = std::find(Return.begin(), Return.end(), "--");
} else {
I = Return.begin();
++I; // To leave the program name in place
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
