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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
