NoQ added a comment.
In D138329#3938351 <https://reviews.llvm.org/D138329#3938351>, @xazax.hun wrote:
> Is the problem `forEachDescendant` matching statements inside blocks and
> lambdas? I wonder if this behavior would surprise people, so I think it would
> be better to:
>
> - Potentially add a template bool parameter to `forEachDescendant`
> controlling this behavior.
Yeah, I honestly think that it is the current behavior of `forEachDescendant`
that's counterintuitive and almost never does what the user actually wants.
People typically get surprised when it *does* descend into callables, something
they didn't even think about.
And the idiom
decl(forEachDescendant(..., forCallable(equalsBoundNode("D"))).bind("D")
is not only clumsy and obscure but also algorithmically slower.
We do need a better name though, the difference between "Each" and "Every"
isn't exactly the same as the difference between this matcher's behavior and
that matcher's behavior. I was thinking of `forEachStmtDescendant`, as in it
matches "statement-descendants". An even better name could be
`forEachDescendantInCallable` which describes the behavior precisely and also
connects to the old idiom for discoverability.
> - Review existing uses because I am not entirely sure if the current behavior
> is the right default.
Yeah that's definitely a good idea.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:50-51
+
+ // For the matchers so far used in spanification, we only need to match
+ // `Stmt`s. To override more as needed.
+
----------------
^^
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:59
+ // To skip callables:
+ if (llvm::isa<FunctionDecl, BlockDecl, ObjCMethodDecl>(Node))
+ return true;
----------------
`llvm::` is unnecessary, we're under `using namespace llvm`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138329/new/
https://reviews.llvm.org/D138329
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
