Skip to content

[clang-query] Load queries and matchers from file during REPL cycle #90603

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

Merged
merged 5 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions clang-tools-extra/clang-query/Query.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//

#include "Query.h"
#include "QueryParser.h"
#include "QuerySession.h"
#include "clang/AST/ASTDumper.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
Expand Down Expand Up @@ -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";
Comment on lines +288 to +291
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than printing directly to errs(), I think you should construct a TextDiagnostic object and use that to emit the diagnostic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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
18 changes: 17 additions & 1 deletion clang-tools-extra/clang-query/Query.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ enum QueryKind {
QK_SetTraversalKind,
QK_EnableOutputKind,
QK_DisableOutputKind,
QK_Quit
QK_Quit,
QK_File
};

class QuerySession;
Expand Down Expand Up @@ -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

Expand Down
10 changes: 8 additions & 2 deletions clang-tools-extra/clang-query/QueryParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,8 @@ enum ParsedQueryKind {
PQK_Unlet,
PQK_Quit,
PQK_Enable,
PQK_Disable
PQK_Disable,
PQK_File
};

enum ParsedQueryVariable {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down
18 changes: 2 additions & 16 deletions clang-tools-extra/clang-query/tool/ClangQuery.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 3 additions & 1 deletion clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ Improvements to clang-doc
Improvements to clang-query
---------------------------

The improvements are...
- Added the `file` command to dynamically load a list of commands and matchers
from an external file, allowing the cost of reading the compilation database
and building the AST to be imposed just once for faster prototyping.

Improvements to clang-rename
----------------------------
Expand Down
1 change: 1 addition & 0 deletions clang-tools-extra/test/clang-query/Inputs/empty.script
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# This file intentionally has no queries
1 change: 1 addition & 0 deletions clang-tools-extra/test/clang-query/Inputs/file.script
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
f DIRECTORY/runtime_file.script
5 changes: 5 additions & 0 deletions clang-tools-extra/test/clang-query/Inputs/runtime_file.script
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
set bind-root false

l func functionDecl(hasName("bar"))
m func.bind("f")
m varDecl().bind("v")
2 changes: 2 additions & 0 deletions clang-tools-extra/test/clang-query/errors.c
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// RUN: not clang-query -c foo -c bar %s -- | FileCheck %s
// RUN: not clang-query -f %S/Inputs/foo.script %s -- | FileCheck %s
// RUN: not clang-query -f %S/Inputs/nonexistent.script %s -- 2>&1 | FileCheck --check-prefix=CHECK-NONEXISTENT %s
// RUN: not clang-query -c 'file %S/Inputs/nonexistent.script' %s -- 2>&1 | FileCheck --check-prefix=CHECK-NONEXISTENT-FILEQUERY %s
// RUN: not clang-query -c foo -f foo %s -- 2>&1 | FileCheck --check-prefix=CHECK-BOTH %s

// CHECK: unknown command: foo
// CHECK-NOT: unknown command: bar

// CHECK-NONEXISTENT: cannot open {{.*}}nonexistent.script
// CHECK-NONEXISTENT-FILEQUERY: cannot open {{.*}}nonexistent.script
// CHECK-BOTH: cannot specify both -c and -f
2 changes: 2 additions & 0 deletions clang-tools-extra/test/clang-query/file-empty.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// RUN: clang-query -c 'file %S/Inputs/empty.script' %s --
// COM: no output expected; nothing to CHECK
14 changes: 14 additions & 0 deletions clang-tools-extra/test/clang-query/file-query.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// 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: "f" binds here
void bar(void) {}

// CHECK: file-query.c:14:1: note: "v" binds here
int baz{1};
4 changes: 3 additions & 1 deletion clang-tools-extra/unittests/clang-query/QueryParserTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
Expand Down
Loading