[clang-query] Load queries and matchers from file during REPL cycle #90603
Conversation
The clang-query tool has the ability to execute or pre-load queries from a file when the tool is started, but doesn't have the ability to do the same from the interactive REPL prompt. Because the prompt also doesn't seem to allow multi-line matchers, this can make prototyping and iterating on more complicated matchers difficult. Supporting a dynamic load at REPL time allows the cost of reading the compilation database and building the AST to be imposed just once, and allows faster prototyping.
|
@llvm/pr-subscribers-clang-tools-extra Author: Chris Warner (cwarner-8702) ChangesThe clang-query tool has the ability to execute or pre-load queries from a file when the tool is launched, but doesn't have the ability to do the same from the interactive REPL prompt. Because the prompt also doesn't seem to allow multi-line matchers, this can make prototyping and iterating on more complicated matchers difficult. Supporting a dynamic load at REPL time allows the cost of reading the compilation database and building the AST to be imposed just once, and allows faster prototyping. cc: @AaronBallman Full diff: https://github.com/llvm/llvm-project/pull/90603.diff 8 Files Affected:
diff --git a/clang-tools-extra/clang-query/Query.cpp b/clang-tools-extra/clang-query/Query.cpp
index c436d6fa949868..9d5807a52fa8ed 100644
--- a/clang-tools-extra/clang-query/Query.cpp
+++ b/clang-tools-extra/clang-query/Query.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "Query.h"
+#include "QueryParser.h"
#include "QuerySession.h"
#include "clang/AST/ASTDumper.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -281,5 +282,26 @@ const QueryKind SetQueryKind<bool>::value;
const QueryKind SetQueryKind<OutputKind>::value;
#endif
+bool FileQuery::run(llvm::raw_ostream &OS, QuerySession &QS) const {
+ auto Buffer = llvm::MemoryBuffer::getFile(StringRef{File}.trim());
+ if (!Buffer) {
+ if (Prefix.has_value())
+ llvm::errs() << *Prefix << ": ";
+ llvm::errs() << "cannot open " << File << ": "
+ << Buffer.getError().message() << "\n";
+ return false;
+ }
+
+ StringRef FileContentRef(Buffer.get()->getBuffer());
+
+ while (!FileContentRef.empty()) {
+ QueryRef Q = QueryParser::parse(FileContentRef, QS);
+ if (!Q->run(llvm::outs(), QS))
+ return false;
+ FileContentRef = Q->RemainingContent;
+ }
+ return true;
+}
+
} // namespace query
} // namespace clang
diff --git a/clang-tools-extra/clang-query/Query.h b/clang-tools-extra/clang-query/Query.h
index 7aefa6bb5ee0dd..7242479633c24f 100644
--- a/clang-tools-extra/clang-query/Query.h
+++ b/clang-tools-extra/clang-query/Query.h
@@ -30,7 +30,8 @@ enum QueryKind {
QK_SetTraversalKind,
QK_EnableOutputKind,
QK_DisableOutputKind,
- QK_Quit
+ QK_Quit,
+ QK_File
};
class QuerySession;
@@ -188,6 +189,21 @@ struct DisableOutputQuery : SetNonExclusiveOutputQuery {
}
};
+struct FileQuery : Query {
+ FileQuery(StringRef File, StringRef Prefix = StringRef())
+ : Query(QK_File), File(File),
+ Prefix(!Prefix.empty() ? std::optional<std::string>(Prefix)
+ : std::nullopt) {}
+
+ bool run(llvm::raw_ostream &OS, QuerySession &QS) const override;
+
+ static bool classof(const Query *Q) { return Q->Kind == QK_File; }
+
+private:
+ std::string File;
+ std::optional<std::string> Prefix;
+};
+
} // namespace query
} // namespace clang
diff --git a/clang-tools-extra/clang-query/QueryParser.cpp b/clang-tools-extra/clang-query/QueryParser.cpp
index 162acc1a598dd5..85a442bdd7deda 100644
--- a/clang-tools-extra/clang-query/QueryParser.cpp
+++ b/clang-tools-extra/clang-query/QueryParser.cpp
@@ -183,7 +183,8 @@ enum ParsedQueryKind {
PQK_Unlet,
PQK_Quit,
PQK_Enable,
- PQK_Disable
+ PQK_Disable,
+ PQK_File
};
enum ParsedQueryVariable {
@@ -222,12 +223,14 @@ QueryRef QueryParser::doParse() {
.Case("let", PQK_Let)
.Case("m", PQK_Match, /*IsCompletion=*/false)
.Case("match", PQK_Match)
- .Case("q", PQK_Quit, /*IsCompletion=*/false)
+ .Case("q", PQK_Quit, /*IsCompletion=*/false)
.Case("quit", PQK_Quit)
.Case("set", PQK_Set)
.Case("enable", PQK_Enable)
.Case("disable", PQK_Disable)
.Case("unlet", PQK_Unlet)
+ .Case("f", PQK_File, /*IsCompletion=*/false)
+ .Case("file", PQK_File)
.Default(PQK_Invalid);
switch (QKind) {
@@ -351,6 +354,9 @@ QueryRef QueryParser::doParse() {
return endQuery(new LetQuery(Name, VariantValue()));
}
+ case PQK_File:
+ return new FileQuery(Line);
+
case PQK_Invalid:
return new InvalidQuery("unknown command: " + CommandStr);
}
diff --git a/clang-tools-extra/clang-query/tool/ClangQuery.cpp b/clang-tools-extra/clang-query/tool/ClangQuery.cpp
index da7ac270144809..a2de7a2dced86e 100644
--- a/clang-tools-extra/clang-query/tool/ClangQuery.cpp
+++ b/clang-tools-extra/clang-query/tool/ClangQuery.cpp
@@ -74,22 +74,8 @@ static cl::opt<std::string> PreloadFile(
bool runCommandsInFile(const char *ExeName, std::string const &FileName,
QuerySession &QS) {
- auto Buffer = llvm::MemoryBuffer::getFile(FileName);
- if (!Buffer) {
- llvm::errs() << ExeName << ": cannot open " << FileName << ": "
- << Buffer.getError().message() << "\n";
- return true;
- }
-
- StringRef FileContentRef(Buffer.get()->getBuffer());
-
- while (!FileContentRef.empty()) {
- QueryRef Q = QueryParser::parse(FileContentRef, QS);
- if (!Q->run(llvm::outs(), QS))
- return true;
- FileContentRef = Q->RemainingContent;
- }
- return false;
+ FileQuery Query(FileName, ExeName);
+ return !Query.run(llvm::errs(), QS);
}
int main(int argc, const char **argv) {
diff --git a/clang-tools-extra/test/clang-query/Inputs/file.script b/clang-tools-extra/test/clang-query/Inputs/file.script
new file mode 100644
index 00000000000000..b58e7bbc24bfb9
--- /dev/null
+++ b/clang-tools-extra/test/clang-query/Inputs/file.script
@@ -0,0 +1 @@
+f DIRECTORY/runtime_file.script
diff --git a/clang-tools-extra/test/clang-query/Inputs/runtime_file.script b/clang-tools-extra/test/clang-query/Inputs/runtime_file.script
new file mode 100644
index 00000000000000..5272580f4c965e
--- /dev/null
+++ b/clang-tools-extra/test/clang-query/Inputs/runtime_file.script
@@ -0,0 +1 @@
+m functionDecl()
diff --git a/clang-tools-extra/test/clang-query/file-query.c b/clang-tools-extra/test/clang-query/file-query.c
new file mode 100644
index 00000000000000..6bd3fd204cb8a7
--- /dev/null
+++ b/clang-tools-extra/test/clang-query/file-query.c
@@ -0,0 +1,11 @@
+// RUN: rm -rf %/t
+// RUN: mkdir %/t
+// RUN: cp %/S/Inputs/file.script %/t/file.script
+// RUN: cp %/S/Inputs/runtime_file.script %/t/runtime_file.script
+// Need to embed the correct temp path in the actual JSON-RPC requests.
+// RUN: sed -e "s|DIRECTORY|%/t|" %/t/file.script > %/t/file.script.temp
+
+// RUN: clang-query -c 'file %/t/file.script.temp' %s -- | FileCheck %s
+
+// CHECK: file-query.c:11:1: note: "root" binds here
+void bar(void) {}
diff --git a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
index 06b0d7b365904e..b561e2bb983321 100644
--- a/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
+++ b/clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
@@ -197,7 +197,7 @@ TEST_F(QueryParserTest, Comment) {
TEST_F(QueryParserTest, Complete) {
std::vector<llvm::LineEditor::Completion> Comps =
QueryParser::complete("", 0, QS);
- ASSERT_EQ(8u, Comps.size());
+ ASSERT_EQ(9u, Comps.size());
EXPECT_EQ("help ", Comps[0].TypedText);
EXPECT_EQ("help", Comps[0].DisplayText);
EXPECT_EQ("let ", Comps[1].TypedText);
@@ -214,6 +214,8 @@ TEST_F(QueryParserTest, Complete) {
EXPECT_EQ("disable", Comps[6].DisplayText);
EXPECT_EQ("unlet ", Comps[7].TypedText);
EXPECT_EQ("unlet", Comps[7].DisplayText);
+ EXPECT_EQ("file ", Comps[8].TypedText);
+ EXPECT_EQ("file", Comps[8].DisplayText);
Comps = QueryParser::complete("set o", 5, QS);
ASSERT_EQ(1u, Comps.size());
|
| if (Prefix.has_value()) | ||
| llvm::errs() << *Prefix << ": "; | ||
| llvm::errs() << "cannot open " << File << ": " | ||
| << Buffer.getError().message() << "\n"; |
There was a problem hiding this comment.
Rather than printing directly to errs(), I think you should construct a TextDiagnostic object and use that to emit the diagnostic.
There was a problem hiding this comment.
Got any tips on using TextDiagnostic? I am struggling to figure out what to pass as the LangOptions and DiagnosticOptions considering the error isn't coming from code per-se (getting the ASTContext from one of the ASTUnit objects and the options from it seems too hacky)
There was a problem hiding this comment.
I was thinking of pulling the information from the ASTUnit as that's how MatchQuery works, but then I realized this suggestion isn't fantastic because there's no source location to emit the diagnostic for. That got me looking at InvalidQuery and I see we emit directly to the passed raw_ostream, which is llvm::outs().
Curiously, runCommandsInFile() emits to llvm::errs() instead, so we sometimes emit to the output stream and sometimes to the error stream. I'd say this case can continue to output to the error stream and maybe someday we should make this a bit more consistent across the tool.
| if (Prefix.has_value()) | ||
| llvm::errs() << *Prefix << ": "; | ||
| llvm::errs() << "cannot open " << File << ": " | ||
| << Buffer.getError().message() << "\n"; |
There was a problem hiding this comment.
I was thinking of pulling the information from the ASTUnit as that's how MatchQuery works, but then I realized this suggestion isn't fantastic because there's no source location to emit the diagnostic for. That got me looking at InvalidQuery and I see we emit directly to the passed raw_ostream, which is llvm::outs().
Curiously, runCommandsInFile() emits to llvm::errs() instead, so we sometimes emit to the output stream and sometimes to the error stream. I'd say this case can continue to output to the error stream and maybe someday we should make this a bit more consistent across the tool.
| void bar(void) {} | ||
|
|
||
| // CHECK: file-query.c:14:1: note: "v" binds here | ||
| int baz{1}; |
There was a problem hiding this comment.
Can you add a newline to the end of the file?
The clang-query tool has the ability to execute or pre-load queries from a file when the tool is launched, but doesn't have the ability to do the same from the interactive REPL prompt. Because the prompt also doesn't seem to allow multi-line matchers, this can make prototyping and iterating on more complicated matchers difficult.
Supporting a dynamic load at REPL time allows the cost of reading the compilation database and building the AST to be imposed just once, and allows faster prototyping.
cc: @AaronBallman