[ASTMatchers] Add `isConsteval` matcher

[Izaron]

Summary

Support C++20 consteval functions and C++2b if consteval for AST
Matchers.

[Izaron]

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

1807

Actually if consteval is a C++23 feature, but there is no isCXX23orLater() method. The is consteval construction seems to be backported to C++20 in clang, as it compiles and just emits a warning

warning: consteval if is a C++2b extension [-Wc++2b-extensions]

[aaron.ballman]

Adding a few more reviewers in case there are differing opinions.

clang/include/clang/ASTMatchers/ASTMatchers.h

5173–5174

5179

It'd be good to show an example of:

if ! consteval {}

if ! consteval {} else {}

as well so users know what to expect.

Should there be a matcher so users can distinguish between if consteval and if ! consteval?

Test cases for these sort of things would also be appreciated.

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

1794–1796

1807–1809

Yes, we support if consteval as an extension in older language modes, so I think this is reasonable.

[Izaron]

Fixed commit

[Izaron]

Fixed commit (with clang-format)

[Izaron]

clang/include/clang/ASTMatchers/ASTMatchers.h

5179

Thanks!

Should there be a matcher so users can distinguish between if consteval and if ! consteval?

This is an interesting question. I wonder how intensively if ! consteval is ever going to be used in real life to make a unique matcher for it? I can't make up a hypothetical checker that would need it.

Let's see what other fellow reviewers think about it =)

[cor3ntin]

clang/include/clang/ASTMatchers/ASTMatchers.h

5179

That's a good question.
Maybe we can add it later if there is a need?
I suspect that if we need 2, we need 3, hasConsteval, hasNegatedConsteval, hasNonNegatedConsteval

Until a need present itself, maybe one is enough

[aaron.ballman]

clang/include/clang/ASTMatchers/ASTMatchers.h

5179

This is an interesting question. I wonder how intensively if ! consteval is ever going to be used in real life to make a unique matcher for it? I can't make up a hypothetical checker that would need it.

That's not the salient question. I'm more worried that the current interface is confusing and not easy to extend. There's no motivation in the review for why this support is needed, and we tend to err on the side of being conservative with matcher interfaces (we only add them when they're necessary and generally useful, otherwise users can use local matchers).

I think the matcher isn't well designed for IfStmt and we should drop support for that initially, and only focus on support for FunctionDecl where the behavior is more clear. Then we can decide what the correct interface is for IfStmt once we have a better picture of how the matcher is expected to be used and a more general design for it.

[cor3ntin]

clang/include/clang/ASTMatchers/ASTMatchers.h

5179

That's not the salient question. I'm more worried that the current interface is confusing and not easy to extend. There's no motivation in the review for why this support is needed, and we tend to err on the side of being conservative with matcher interfaces (we only add them when they're necessary and generally useful, otherwise users can use local matchers).

Arguably, there is one for if constexpr and both of them should be symmetric. Introducing asymmetry for no good reason seems wrong. I assume that there was a motivation initially?

I think the matcher isn't well designed for IfStmt and we should drop support for that initially, and only focus on support for FunctionDecl where the behavior is more clear. Then we can decide what the correct interface is for IfStmt once we have a better picture of how the matcher is expected to be used and a more general design for it.

This was initially my gut feeling, however there is no separate matcher for if constexpr(false) and if constexpr (true), so I don't think that if ! consteval needs to be treated differently here.
Either way, i don't think adding one hurts

[aaron.ballman]

clang/include/clang/ASTMatchers/ASTMatchers.h

5179

You can match over the expression in if constexpr to determine whether it's true or false (at least in theory, we probably need some better matchers around constant expression value comparisons), but you cannot match over the syntax of if ! consteval vs if consteval, so I see them as being different cases in that regard.

[aaron.ballman]

LGTM with a minor adjustment to the documentation.

clang/include/clang/ASTMatchers/ASTMatchers.h

5173

Needs to be re-wrapped to 80-col, mostly wanting to call out the if ! consteval bit more explicitly.

5179

I spoke with @cor3ntin off-list a bit about this and am now more comfortable. What was bothering me was that:

if consteval {
}

if ! consteval {
}

are both matched by ifStmt(isConsteval()) and matching on the second case looks downright odd to me. However, that is the interface of our AST. We can add isNonNegatedConsteval() and isNegatedConsteval() as matchers when we need to distinguish between the cases. Thanks for bearing with me while I figured that out. :-)

[Izaron]

Fixed: make docs more explicit

[Izaron]

Thanks! Let's see if docs are more explicit and nicer now.

[aaron.ballman]

LGTM! Do you need someone to commit on your behalf? If so, what name and email address would you like used for patch attribution?

[Izaron]

In D117613#3258854, @aaron.ballman wrote:

LGTM! Do you need someone to commit on your behalf? If so, what name and email address would you like used for patch attribution?

Sorry, I should have set the Author property of the commit! I would like to use name "Evgeny Shulgin" and email "[e-mail]". Thanks!

[aaron.ballman]

In D117613#3258886, @Izaron wrote:

In D117613#3258854, @aaron.ballman wrote:

LGTM! Do you need someone to commit on your behalf? If so, what name and email address would you like used for patch attribution?

Sorry, I should have set the Author property of the commit! I would like to use name "Evgeny Shulgin" and email "[e-mail]". Thanks!

No worries! I've commit on your behalf in b80db150cdba17b3d4970389025f95b1c93482b8 (I also added a short release note, which I forgot to mention during the review). Thank you for the new matcher!

clang/docs/LibASTMatchersReference.html

 <tr><td colspan="4" class="doc" id="hasTrailingReturn0"><pre>Matches a function declared with a trailing return type.
 
 Example matches Y (matcher = functionDecl(hasTrailingReturn()))
 int X() {}
 auto Y() -&gt; int {}
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html">FunctionDecl</a>&gt;</td><td class="name" onclick="toggle('isConsteval1')"><a name="isConsteval1Anchor">isConsteval</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isConsteval1"><pre>Matches consteval function declarations and if consteval.
+
+Given:
+  consteval int a();
+  void b() { if consteval {} }
+  void c() { if ! consteval {} }
+  void d() { if ! consteval {} else {} }
+functionDecl(isConsteval())
+  matches the declaration of "int a()".
+ifStmt(isConsteval())
+  matches the if statement in "void b()", "void c()", "void d()".
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1FunctionDecl.html">FunctionDecl</a>&gt;</td><td class="name" onclick="toggle('isConstexpr1')"><a name="isConstexpr1Anchor">isConstexpr</a></td><td></td></tr>
 <tr><td colspan="4" class="doc" id="isConstexpr1"><pre>Matches constexpr variable and function declarations,
        and if constexpr.
 
 Given:
   constexpr int foo = 42;
   constexpr int bar();
   void baz() { if constexpr(1 &gt; 0) {} }
functionDecl(parameterCountIs(2))
   matches g and h
 functionProtoType(parameterCountIs(2))
   matches g and h
 functionProtoType(parameterCountIs(3))
   matches k
 </pre></td></tr>
 
 
+<tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1IfStmt.html">IfStmt</a>&gt;</td><td class="name" onclick="toggle('isConsteval2')"><a name="isConsteval2Anchor">isConsteval</a></td><td></td></tr>
+<tr><td colspan="4" class="doc" id="isConsteval2"><pre>Matches consteval function declarations and if consteval.
+
+Given:
+  consteval int a();
+  void b() { if consteval {} }
+  void c() { if ! consteval {} }
+  void d() { if ! consteval {} else {} }
+functionDecl(isConsteval())
+  matches the declaration of "int a()".
+ifStmt(isConsteval())
+  matches the if statement in "void b()", "void c()", "void d()".
+</pre></td></tr>
+
+
 <tr><td>Matcher&lt;<a href="https://clang.llvm.org/doxygen/classclang_1_1IfStmt.html">IfStmt</a>&gt;</td><td class="name" onclick="toggle('isConstexpr2')"><a name="isConstexpr2Anchor">isConstexpr</a></td><td></td></tr>
 <tr><td colspan="4" class="doc" id="isConstexpr2"><pre>Matches constexpr variable and function declarations,
        and if constexpr.
 
 Given:
   constexpr int foo = 42;
   constexpr int bar();
   void baz() { if constexpr(1 &gt; 0) {} }

clang/include/clang/ASTMatchers/ASTMatchers.h

AST_POLYMORPHIC_MATCHER(isNoThrow,
 
   // Assume the best for any unresolved exception specification.
   if (isUnresolvedExceptionSpec(FnTy->getExceptionSpecType()))
     return true;
 
   return FnTy->isNothrow();
 }
 
+/// Matches consteval function declarations and if consteval.
+///
+/// Given:
+/// \code
+///   consteval int a();
+///   void b() { if consteval {} }
+///   void c() { if ! consteval {} }
+///   void d() { if ! consteval {} else {} }
+/// \endcode
+/// functionDecl(isConsteval())
+///   matches the declaration of "int a()".
+/// ifStmt(isConsteval())
+///   matches the if statement in "void b()", "void c()", "void d()".
+AST_POLYMORPHIC_MATCHER(isConsteval,
+                        AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl, IfStmt)) {
+  return Node.isConsteval();
+}
+
 /// Matches constexpr variable and function declarations,
 ///        and if constexpr.
 ///
 /// Given:
 /// \code
 ///   constexpr int foo = 42;
 ///   constexpr int bar();
 ///   void baz() { if constexpr(1 > 0) {} }

clang/lib/ASTMatchers/Dynamic/Registry.cpp

RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(isBitField);
   REGISTER_MATCHER(isCatchAll);
   REGISTER_MATCHER(isClass);
   REGISTER_MATCHER(isClassMessage);
   REGISTER_MATCHER(isClassMethod);
   REGISTER_MATCHER(isComparisonOperator);
   REGISTER_MATCHER(isConst);
   REGISTER_MATCHER(isConstQualified);
+  REGISTER_MATCHER(isConsteval);
   REGISTER_MATCHER(isConstexpr);
   REGISTER_MATCHER(isCopyAssignmentOperator);
   REGISTER_MATCHER(isCopyConstructor);
   REGISTER_MATCHER(isDefaultConstructor);
   REGISTER_MATCHER(isDefaulted);
   REGISTER_MATCHER(isDefinition);
   REGISTER_MATCHER(isDelegatingConstructor);
   REGISTER_MATCHER(isDeleted);

clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

EXPECT_TRUE(
       notMatches("void f() noexcept(false);", functionDecl(isNoThrow())));
   EXPECT_TRUE(matches("void f() noexcept;", functionDecl(isNoThrow())));
 
   EXPECT_TRUE(
       notMatches("void f() noexcept(false);", functionProtoType(isNoThrow())));
   EXPECT_TRUE(matches("void f() noexcept;", functionProtoType(isNoThrow())));
 }
 
+TEST_P(ASTMatchersTest, IsConsteval) {
+  if (!GetParam().isCXX20OrLater())
+    return;
+
+  EXPECT_TRUE(matches("consteval int bar();",
+                      functionDecl(hasName("bar"), isConsteval())));
+  EXPECT_TRUE(notMatches("constexpr int bar();",
+                         functionDecl(hasName("bar"), isConsteval())));
+  EXPECT_TRUE(
+      notMatches("int bar();", functionDecl(hasName("bar"), isConsteval())));
+}
+
+TEST_P(ASTMatchersTest, IsConsteval_MatchesIfConsteval) {
+  if (!GetParam().isCXX20OrLater())
+    return;
+
+  EXPECT_TRUE(matches("void baz() { if consteval {} }", ifStmt(isConsteval())));
+  EXPECT_TRUE(
+      matches("void baz() { if ! consteval {} }", ifStmt(isConsteval())));
+  EXPECT_TRUE(matches("void baz() { if ! consteval {} else {} }",
+                      ifStmt(isConsteval())));
+  EXPECT_TRUE(
+      matches("void baz() { if not consteval {} }", ifStmt(isConsteval())));
+  EXPECT_TRUE(notMatches("void baz() { if constexpr(1 > 0) {} }",
+                         ifStmt(isConsteval())));
+  EXPECT_TRUE(
+      notMatches("void baz() { if (1 > 0) {} }", ifStmt(isConsteval())));
+}
+
 TEST_P(ASTMatchersTest, IsConstexpr) {
   if (!GetParam().isCXX11OrLater()) {
     return;
   }
 
   EXPECT_TRUE(matches("constexpr int foo = 42;",
                       varDecl(hasName("foo"), isConstexpr())));
   EXPECT_TRUE(matches("constexpr int bar();",