[ASTMatchers] Add clang-query support for equals matcher

[Lekensteyn]

Summary

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.

[aaron.ballman]

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

Why not add this support immediately rather than a TODO?

include/clang/ASTMatchers/ASTMatchersMacros.h

364

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

const ParamType &Param per the coding guidelines.

371

Same here.

[Lekensteyn]

In D33094#752153, @aaron.ballman wrote:

Please be sure to regenerate the AST matcher documentation as well by running clang/docs/tools/dump_ast_matchers.py

Will do, thanks for the hint.

include/clang/ASTMatchers/ASTMatchers.h

3827

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

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

Will fix (this was copied from the above macros).

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

3827

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

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).

Ah, thank you for the explanation. The fixed name part makes this harder.

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?

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).

[Lekensteyn]

v1 -> v2:

• Add CharacterLiteral and FloatingLiterals support
• updated documentation comment to include examples for all four supported matcher types
• updated docs with dump_ast_matchers.py

Depends on:

• D33093 for boolean support
• D33135 for float support

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

3838

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

Can you add tests for floating literals with suffixes (f, l)?

545

Can you add some tests involving the other character literal types (L, u, U, u8)?

[Lekensteyn]

include/clang/ASTMatchers/ASTMatchers.h

3838

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

will do

545

will do

[aaron.ballman]

include/clang/ASTMatchers/ASTMatchers.h

3838

That's a good reason not to do it right now. Thanks!

[Lekensteyn]

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

Done (except for u8 which is only defined for string literals).

[aaron.ballman]

LGTM!

include/clang/ASTMatchers/ASTMatchers.h

 
 /// \brief Matches literals that are equal to the given value.
 ///
 /// Example matches true (matcher = cxxBoolLiteral(equals(true)))
 /// \code
 ///   true
 /// \endcode
 ///
-/// Usable as: Matcher<CharacterLiteral>, Matcher<CXXBoolLiteral>,
+/// Usable as: Matcher<CharacterLiteral>, Matcher<CXXBoolLiteralExpr>,
 ///            Matcher<FloatingLiteral>, Matcher<IntegerLiteral>
 template <typename ValueT>
 internal::PolymorphicMatcherWithParam1<internal::ValueEqualsMatcher, ValueT>
 equals(const ValueT &Value) {
   return internal::PolymorphicMatcherWithParam1<
     internal::ValueEqualsMatcher,
     ValueT>(Value);
 }
 
+/// \brief Instantiations for the \c equals matcher.
+/// TODO add support for FloatingLiteral and CharacterLiteral
+/// @{
+AST_CONCRETE_EQUALS_MATCHER(CXXBoolLiteralExpr, bool, 0);
+AST_CONCRETE_EQUALS_MATCHER(IntegerLiteral, unsigned, 1);
+/// @}
+
 /// \brief Matches the operator Name of operator expressions (binary or
 /// unary).
 ///
 /// Example matches a || b (matcher = binaryOperator(hasOperatorName("||")))
 /// \code
 ///   !(a || b)
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P(hasOperatorName,
                           AST_POLYMORPHIC_SUPPORTED_TYPES(BinaryOperator,
                                                           UnaryOperator),
                           std::string, Name) {
   return Name == Node.getOpcodeStr(Node.getOpcode());
 }
 

include/clang/ASTMatchers/ASTMatchersMacros.h

#define AST_POLYMORPHIC_MATCHER_P2_OVERLOAD(DefineMatcher, ReturnTypesF,       \
   template <typename NodeType, typename ParamT1, typename ParamT2>             \
   bool internal::matcher_##DefineMatcher##OverloadId##Matcher<                 \
       NodeType, ParamT1, ParamT2>::                                            \
       matches(const NodeType &Node,                                            \
               ::clang::ast_matchers::internal::ASTMatchFinder *Finder,         \
               ::clang::ast_matchers::internal::BoundNodesTreeBuilder *Builder) \
           const
 
+/// \brief Defines functions and a type for the \c equals matcher, suitable for
+/// comparing a \c ReturnType node against the a \c ParamType value.
+#define AST_CONCRETE_EQUALS_MATCHER(ReturnType, ParamType, OverloadId)         \
+  inline ::clang::ast_matchers::internal::Matcher<ReturnType> equals(          \
+      ParamType const &Param) {                                                \
+    return ::clang::ast_matchers::internal::makeMatcher(                       \
+        new internal::ValueEqualsMatcher<ReturnType, ParamType>(Param));       \
+  }                                                                            \
+  typedef ::clang::ast_matchers::internal::Matcher<ReturnType>(                \
+      &equals_Type##OverloadId)(ParamType const &Param)
+
 /// \brief Creates a variadic matcher for both a specific \c Type as well as
 /// the corresponding \c TypeLoc.
 #define AST_TYPE_MATCHER(NodeType, MatcherName)                                \
   const ::clang::ast_matchers::internal::VariadicDynCastAllOfMatcher<          \
       Type, NodeType> MatcherName
 // FIXME: add a matcher for TypeLoc derived classes using its custom casting
 // API (no longer dyn_cast) if/when we need such matching
 

lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_OVERLOADED_2(hasType);
   REGISTER_OVERLOADED_2(isDerivedFrom);
   REGISTER_OVERLOADED_2(isSameOrDerivedFrom);
   REGISTER_OVERLOADED_2(loc);
   REGISTER_OVERLOADED_2(pointsTo);
   REGISTER_OVERLOADED_2(references);
   REGISTER_OVERLOADED_2(thisPointerType);
 
+  // TODO add support for floats and char literals
+  REGISTER_OVERLOADED_2(equals);
+
   REGISTER_MATCHER(accessSpecDecl);
   REGISTER_MATCHER(addrLabelExpr);
   REGISTER_MATCHER(alignOfExpr);
   REGISTER_MATCHER(allOf);
   REGISTER_MATCHER(anyOf);
   REGISTER_MATCHER(anything);
   REGISTER_MATCHER(argumentCountIs);
   REGISTER_MATCHER(arraySubscriptExpr);

unittests/ASTMatchers/Dynamic/RegistryTest.cpp

 }
 
 TEST_F(RegistryTest, ParenExpr) {
   Matcher<Stmt> Value = constructMatcher("parenExpr").getTypedMatcher<Stmt>();
   EXPECT_TRUE(matches("int i = (1);", Value));
   EXPECT_FALSE(matches("int i = 1;", Value));
 }
 
+TEST_F(RegistryTest, EqualsMatcher) {
+  Matcher<Stmt> IntegerStmt = constructMatcher(
+      "integerLiteral", constructMatcher("equals", VariantValue(42)))
+      .getTypedMatcher<Stmt>();
+  EXPECT_TRUE(matches("int x = 42;", IntegerStmt));
+  EXPECT_FALSE(matches("int x = 1;", IntegerStmt));
+
+  Matcher<Stmt> BooleanStmt = constructMatcher(
+      "cxxBoolLiteral", constructMatcher("equals", VariantValue(true)))
+      .getTypedMatcher<Stmt>();
+  EXPECT_TRUE(matches("bool x = true;", BooleanStmt));
+  EXPECT_FALSE(matches("bool x = false;", BooleanStmt));
+  EXPECT_FALSE(matches("bool x = 0;", BooleanStmt));
+}
+
 } // end anonymous namespace
 } // end namespace dynamic
 } // end namespace ast_matchers
 } // end namespace clang