njames93 added inline comments.
================
Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:7621
+/// matches the expression 'delete Ptr'.
+AST_MATCHER_P_OVERLOAD(CXXDeleteExpr, deletes, internal::Matcher<Expr>,
+ InnerMatcher, 1) {
----------------
aaron.ballman wrote:
> Why add this instead of continuing to use `has()`? (I worry a bit that we'll
> want to add a verb for different subexpression matchers and it won't scale
> well or we won't add verbs for different subexpressions and this'll introduce
> a new inconsistency.)
This change is about lowering the barrier to entry. While `has` can be very
powerful, its interface isn't newcomer friendly.
You can put any matcher in, that may not make sense or ever be capable of
actually matching but it will happily compile without emitting any warning.
using `deletes` forces some type checking on the argument you provide.
The explicit names are also much better for documentation and API discovery.
When running in clang query with completions this is what appears at the top of
the list
```
clang-query> m cxxDeleteExpr(
Matcher<CXXDeleteExpr> deletes(Matcher<Expr>)
Matcher<CXXDeleteExpr> isArray()
```
A similar case is made for the matchers documentation.
================
Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:163
+inline bool isArrayOperator(const CXXDeleteExpr &Node) {
+ return Node.isArrayFormAsWritten();
+}
----------------
aaron.ballman wrote:
> Do we need/want this interface to consider whether the user is matching
> things spelled in source or not?
From what I can see the only time isArrayFrom and isArrayFromAsWritten
differentiate is if normal `delete` operator is used on an arrayType. However
that behaviour isn't standard afaik and was only added to be consistent with
gcc and edg. If that case comes up we emit a fixit to add the `[]`.
Maybe there is merit to check traversal kind before assuming tho.
================
Comment at: clang/lib/ASTMatchers/Dynamic/Registry.cpp:269
REGISTER_MATCHER(hasAnyUsingShadowDecl);
- REGISTER_MATCHER(hasArgument);
REGISTER_MATCHER(hasArgumentOfType);
----------------
aaron.ballman wrote:
> Why removing this?
Accident. Will re add.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97681/new/
https://reviews.llvm.org/D97681
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
