-
Notifications
You must be signed in to change notification settings - Fork 10.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Tooling] Fix FixedCompilationDatabase with header compile flags #73913
Conversation
@llvm/pr-subscribers-clang Author: Sam McCall (sam-mccall) ChangesSummary: Today doesn't work with header-parsing actions because these are "precompile" Reviewers: Subscribers: Full diff: https://github.com/llvm/llvm-project/pull/73913.diff 2 Files Affected:
diff --git a/clang/lib/Tooling/CompilationDatabase.cpp b/clang/lib/Tooling/CompilationDatabase.cpp
index fdf6015508d94ba..2b6e6a09f1335c2 100644
--- a/clang/lib/Tooling/CompilationDatabase.cpp
+++ b/clang/lib/Tooling/CompilationDatabase.cpp
@@ -156,6 +156,7 @@ struct CompileJobAnalyzer {
bool CollectChildren = Collect;
switch (A->getKind()) {
case driver::Action::CompileJobClass:
+ case driver::Action::PrecompileJobClass:
CollectChildren = true;
break;
@@ -293,7 +294,8 @@ static bool stripPositionalArgs(std::vector<const char *> Args,
// -flto* flags make the BackendJobClass, which still needs analyzer.
if (Cmd.getSource().getKind() == driver::Action::AssembleJobClass ||
Cmd.getSource().getKind() == driver::Action::BackendJobClass ||
- Cmd.getSource().getKind() == driver::Action::CompileJobClass) {
+ Cmd.getSource().getKind() == driver::Action::CompileJobClass ||
+ Cmd.getSource().getKind() == driver::Action::PrecompileJobClass) {
CompileAnalyzer.run(&Cmd.getSource());
}
}
diff --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
index 5173d472486bfc2..fffef88a91f7c4e 100644
--- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -656,6 +656,25 @@ TEST(ParseFixedCompilationDatabase, HandlesPositionalArgs) {
EXPECT_EQ(2, Argc);
}
+TEST(ParseFixedCompilationDatabase, HandlesPositionalArgsHeader) {
+ const char *Argv[] = {"1", "2", "--", "-xc++-header",
+ "-c", "somefile.h", "-DDEF3"};
+ int Argc = std::size(Argv);
+ std::string ErrorMsg;
+ std::unique_ptr<FixedCompilationDatabase> Database =
+ FixedCompilationDatabase::loadFromCommandLine(Argc, Argv, ErrorMsg);
+ ASSERT_TRUE((bool)Database);
+ ASSERT_TRUE(ErrorMsg.empty());
+ std::vector<CompileCommand> Result =
+ Database->getCompileCommands("source");
+ ASSERT_EQ(1ul, Result.size());
+ ASSERT_EQ(".", Result[0].Directory);
+ ASSERT_THAT(Result[0].CommandLine,
+ ElementsAre(EndsWith("clang-tool"), "-xc++-header", "-c",
+ "-DDEF3", "source"));
+ EXPECT_EQ(2, Argc);
+}
+
TEST(ParseFixedCompilationDatabase, HandlesPositionalArgsSyntaxOnly) {
// Adjust the given command line arguments to ensure that any positional
// arguments in them are stripped.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
Summary: The logic to strip positional args feels very fragile, but it's terribly useful when you want to use a tool on a file and have the exact argv. Today doesn't work with header-parsing actions because these are "precompile" rather than "compile", from tooling's perspective it's all the same. Reviewers: kadircet Subscribers:
Summary:
The logic to strip positional args feels very fragile, but it's terribly useful
when you want to use a tool on a file and have the exact argv.
Today doesn't work with header-parsing actions because these are "precompile"
rather than "compile", from tooling's perspective it's all the same.
Reviewers:
kadircet
Subscribers: