jnykiel updated this revision to Diff 331884.
jnykiel edited the summary of this revision.
jnykiel added a comment.
I've addressed your comments (I agree with them, essentially). After
implementing the insertion of '--' for problematic file names, I had to modify
some interpolation unit tests to get them to consistently pass on both Windows
and Unix-like systems - I made it possible to match against a relative
'non-native' path in a test, as the absolute paths in this unit test fixture
always start with a drive letter on Windows, thus making it impossible to test
the '-' cases there, and always start with a '/' on Unix-like systems, making
the expected string different when run on Windows or Unix-like.
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
@@ -725,14 +725,14 @@
protected:
// Look up the command from a relative path, and return it in string form.
// The input file is not included in the returned command.
- std::string getCommand(llvm::StringRef F) {
+ std::string getCommand(llvm::StringRef F, bool MakeNative = true) {
auto Results =
inferMissingCompileCommands(std::make_unique<MemCDB>(Entries))
- ->getCompileCommands(path(F));
+ ->getCompileCommands(MakeNative ? path(F) : 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))
+ EXPECT_EQ(Results[0].CommandLine.back(), MakeNative ? path(F) : F)
<< "Last arg should be the file";
Results[0].CommandLine.pop_back();
return llvm::join(Results[0].CommandLine, " ");
@@ -812,6 +812,28 @@
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 the input file
+ // -- is removed as it's not necessary - the new input file doesn't start with
+ // a dash
+ EXPECT_EQ(getCommand("dir/bar.cpp"), "clang -D dir/foo.cpp -Wall -std=c++14");
+}
+
+TEST_F(InterpolateTest, InsertDoubleDash) {
+ add("dir/foo.cpp", "-o foo.o -std=c++14 -Wall");
+ EXPECT_EQ(getCommand("-dir/bar.cpp", false),
+ "clang -D dir/foo.cpp -Wall -std=c++14 --");
+}
+
+TEST_F(InterpolateTest, InsertDoubleDashForClangCL) {
+ add("dir/foo.cpp", "clang-cl", "/std:c++14 /W4");
+ EXPECT_EQ(getCommand("/dir/bar.cpp", false),
+ "clang-cl -D dir/foo.cpp /W4 /std:c++14 --");
+}
+
TEST_F(InterpolateTest, Case) {
add("FOO/BAR/BAZ/SHOUT.cc");
add("foo/bar/baz/quiet.cc");
@@ -831,7 +853,7 @@
add("foo.cpp", "clang-cl", "/W4");
// Language flags should be added with CL syntax.
- EXPECT_EQ(getCommand("foo.h"), "clang-cl -D foo.cpp /W4 /TP");
+ EXPECT_EQ(getCommand("foo.h", false), "clang-cl -D foo.cpp /W4 /TP");
}
TEST_F(InterpolateTest, DriverModes) {
@@ -839,8 +861,10 @@
add("bar.cpp", "clang", "--driver-mode=cl");
// --driver-mode overrides should be respected.
- EXPECT_EQ(getCommand("foo.h"), "clang-cl -D foo.cpp --driver-mode=gcc -x c++-header");
- EXPECT_EQ(getCommand("bar.h"), "clang -D bar.cpp --driver-mode=cl /TP");
+ EXPECT_EQ(getCommand("foo.h"),
+ "clang-cl -D foo.cpp --driver-mode=gcc -x c++-header");
+ EXPECT_EQ(getCommand("bar.h", false),
+ "clang -D bar.cpp --driver-mode=cl /TP");
}
TEST(TransferCompileCommandTest, Smoke) {
Index: clang/lib/Tooling/Tooling.cpp
===================================================================
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -440,8 +440,10 @@
return;
// If there's no override in place add our resource dir.
- Args.push_back("-resource-dir=" +
- CompilerInvocation::GetResourcesPath(Argv0, MainAddr));
+ Args = getInsertArgumentAdjuster(
+ ("-resource-dir=" + CompilerInvocation::GetResourcesPath(Argv0, MainAddr))
+ .c_str(),
+ ArgumentInsertPosition::END)(Args, "");
}
int ClangTool::run(ToolAction *Action) {
Index: clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
===================================================================
--- clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
+++ clang/lib/Tooling/InterpolatingCompilationDatabase.cpp
@@ -177,6 +177,10 @@
Opt.matches(OPT__SLASH_Fo))))
continue;
+ // ...including when the inputs are passed after --.
+ if (Opt.matches(OPT__DASH_DASH))
+ break;
+
// Strip -x, but record the overridden language.
if (const auto GivenType = tryParseTypeArg(*Arg)) {
Type = *GivenType;
@@ -235,6 +239,8 @@
llvm::Twine(ClangCLMode ? "/std:" : "-std=") +
LangStandard::getLangStandardForKind(Std).getName()).str());
}
+ if (Filename.startswith("-") || (ClangCLMode && Filename.startswith("/")))
+ 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
@@ -62,7 +62,8 @@
HasSyntaxOnly = true;
}
if (!HasSyntaxOnly)
- AdjustedArgs.push_back("-fsyntax-only");
+ AdjustedArgs = getInsertArgumentAdjuster(
+ "-fsyntax-only", ArgumentInsertPosition::END)(AdjustedArgs, "");
return AdjustedArgs;
};
}
@@ -137,7 +138,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
