This allows the clang-query tool to use matchers like
"integerLiteral(equals(32))". For this to work, an overloaded function
is added for each possible parameter type.
Details
Details
Diff Detail
Diff Detail
- Repository
- rL LLVM
Event Timeline
Comment Actions
Please be sure to regenerate the AST matcher documentation as well by running clang/docs/tools/dump_ast_matchers.py
| include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 3827 ↗ | (On Diff #98608) | Why not add this support immediately rather than a TODO? |
| include/clang/ASTMatchers/ASTMatchersMacros.h | ||
| 364 ↗ | (On Diff #98608) | Instead of making the user of the macro pass in an overload id, could we make use of the __LINE__ macro to automate it? Given the length of the macro name, I struggle to imagine many people accidentally defining two overloads on the same line (and we can document this macro appropriately, of course). |
| 366 ↗ | (On Diff #98608) | const ParamType &Param per the coding guidelines. |
| 371 ↗ | (On Diff #98608) | Same here. |
Comment Actions
Will do, thanks for the hint.
| include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 3827 ↗ | (On Diff #98608) | The parser would need additional changes for it to be usable in clang-query (see D33093 for boolean support), my initial focus was on supporting IntegerLiterals but bool was added to have an overload. I'll look into adding support for other types. |
| include/clang/ASTMatchers/ASTMatchersMacros.h | ||
| 364 ↗ | (On Diff #98608) | Not sure how __LINE__ would help, uniqueness is not the only requirement, it must also be a fixed name such that Registry can refer to it. (Uniqueness is also needed, otherwise it would be ambiguous). Alternatively, the overload name can be removed, but then the returntype and paramtype for the marshaller should be explicitly specified. This would remove the magic numbers (for overload ID), is this an approach worth looking into? |
| 366 ↗ | (On Diff #98608) | Will fix (this was copied from the above macros). |
| include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 3827 ↗ | (On Diff #98608) | If it turns out to be more work than you want to put in, don't feel obligated to do it. I just thought it might be nice to round it out. |
| include/clang/ASTMatchers/ASTMatchersMacros.h | ||
| 364 ↗ | (On Diff #98608) |
Ah, thank you for the explanation. The fixed name part makes this harder.
I'm not certain what that solution would look like in practice, but if it removes the magic numbers, that sounds promising (assuming the result isn't even more fragile, of course). |
| include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 3846 ↗ | (On Diff #98917) | Is there a reason to not allow the equals matcher to do something like floatingLiteral(equals(1))? Sure, the user could always write 1.0, but it seems somewhat hostile to require it. |
| unittests/ASTMatchers/Dynamic/RegistryTest.cpp | ||
| 534 ↗ | (On Diff #98917) | Can you add tests for floating literals with suffixes (f, l)? |
| 545 ↗ | (On Diff #98917) | Can you add some tests involving the other character literal types (L, u, U, u8)? |
| include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 3846 ↗ | (On Diff #98917) | The ValueMatcher for float does not accept integers at the moment, adding FloatingLiteral now results in a compile error. It can be added though (might do this as well in the next revision, either in the existing patches or a new one). |
| unittests/ASTMatchers/Dynamic/RegistryTest.cpp | ||
| 534 ↗ | (On Diff #98917) | will do |
| 545 ↗ | (On Diff #98917) | will do |
| include/clang/ASTMatchers/ASTMatchers.h | ||
|---|---|---|
| 3846 ↗ | (On Diff #98917) | That's a good reason not to do it right now. Thanks! |
Comment Actions
diff from previous patch:
diff --git a/unittests/ASTMatchers/Dynamic/RegistryTest.cpp b/unittests/ASTMatchers/Dynamic/RegistryTest.cpp index 29fcdec6c1..84e31f721a 100644 --- a/unittests/ASTMatchers/Dynamic/RegistryTest.cpp +++ b/unittests/ASTMatchers/Dynamic/RegistryTest.cpp @@ -530,6 +530,8 @@ TEST_F(RegistryTest, EqualsMatcher) { "floatLiteral", constructMatcher("equals", VariantValue(1.2))) .getTypedMatcher<Stmt>(); EXPECT_TRUE(matches("double x = 1.2;", DoubleStmt)); + EXPECT_TRUE(matches("double x = 1.2f;", DoubleStmt)); + EXPECT_TRUE(matches("double x = 1.2l;", DoubleStmt)); EXPECT_TRUE(matches("double x = 12e-1;", DoubleStmt)); EXPECT_FALSE(matches("double x = 1.23;", DoubleStmt)); @@ -543,6 +545,9 @@ TEST_F(RegistryTest, EqualsMatcher) { "characterLiteral", constructMatcher("equals", VariantValue('x'))) .getTypedMatcher<Stmt>(); EXPECT_TRUE(matches("int x = 'x';", CharStmt)); + EXPECT_TRUE(matches("int x = L'x';", CharStmt)); + EXPECT_TRUE(matches("int x = u'x';", CharStmt)); + EXPECT_TRUE(matches("int x = U'x';", CharStmt)); EXPECT_FALSE(matches("int x = 120;", CharStmt)); }
| unittests/ASTMatchers/Dynamic/RegistryTest.cpp | ||
|---|---|---|
| 545 ↗ | (On Diff #98917) | Done (except for u8 which is only defined for string literals). |