ziqingluo-90 added inline comments.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:54-55
+ bool TraverseDecl(Decl *Node) {
+ if (!Node)
+ return true;
+ if (!match(*Node))
----------------
NoQ wrote:
> ziqingluo-90 wrote:
> > steakhal wrote:
> > > Can `Node` be ever null if the visitor is initiated only be
> > > `AST_MATCHER_P`?
> > Honestly I do not know the exact answer to your question. I was imagining
> > that an AST node could have a null to be one of its children.
> >
> > Our plan later is to make this matcher a general alternative to
> > `forEachDescendant`, so I think the check for null here is not
> > over-defensive.
> There can definitely be null children in the AST. Eg. `for(;;) {}` has null
> initializer, null condition, null increment, non-null body. I guess this is
> more about whether `RecursiveASTVisitor` checks for that automatically before
> invoking callbacks.
> I guess this is more about whether RecursiveASTVisitor checks for that
> automatically before invoking callbacks.
The override-ed `TraverseDecl` method in the base class `RecursiveASTVisitor`
also has a null-check. So I feel like this null-check is needed here.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:59
+ // To skip callables:
+ if (llvm::isa<FunctionDecl, BlockDecl, ObjCMethodDecl>(Node))
+ return true;
----------------
NoQ wrote:
> `llvm::` is unnecessary, we're under `using namespace llvm`.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:71
+ // To skip callables:
+ if (llvm::isa<LambdaExpr>(Node))
+ return true;
----------------
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
