[PATCH] D117613: [ASTMatchers] Add `isConsteval` matcher

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.


================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5179
+///   consteval int bar();
+///   void baz() { if consteval {} }
+/// \endcode
----------------
cor3ntin wrote:
> Izaron wrote:
> > aaron.ballman wrote:
> > > 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.
> > 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 =)
> 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
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117613/new/

https://reviews.llvm.org/D117613