vsavchenko added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/BugSuppression.cpp:70
+
+class CacheInitializer : public RecursiveASTVisitor<CacheInitializer> {
+public:
----------------
NoQ wrote:
> vsavchenko wrote:
> > NoQ wrote:
> > > A recursive visitor would cause you to visit nested declarations (eg.,
> > > lambdas, or methods of nested classes) multiple times. Is this necessary?
> > > A plain old `ConstStmtVisitor` that's taught to visit child statements
> > > recursively could easily avoid that. That's probably cheap though.
> > I don't see how I can easily use `StmtVisitor`s without A LOT OF
> > boilerplate code that will essentially produce a `RecursiveASTVisitor` (it
> > is very possible that I just don't know about some trick). I guess we can
> > add some optimization if needed, but I never had performance problems with
> > recursive visitors.
> The typical idiom is as follows:
> ```lang=c++
> void VisitChildren(const Stmt *S) {
> // Not a visitor callback - just a helper function.
> for (const Stmt *ChildS : S->children())
> if (ChildS)
> Visit(ChildS);
> }
>
> void VisitStmt(const Stmt *S) {
> // The default behavior that makes the visitor recursive over statements.
> VisitChildren(S);
> }
>
> void VisitAttributedStmt(const AttributedStmt *AS) {
> VisitAttributedNode(AS);
> VisitChildren(AS); // This *optionally* goes into every overridden function.
> }
> ```
>
> Also you'll probably have to manually unwrap `VisitVarDecl` into
> `VisitDeclStmt` with a loop over decls. But generally i think that's
> relatively little boilerplate for the much better control it provides.
>
> > I never had performance problems with recursive visitors
>
> Me too until i started doing `clang-tidy --enable-check-profile` :D So, like,
> i don't think that's going to be a real problem but kind of why not avoid it
> entirely.
That makes sense, but it's not a no-brainer tradeoff IMO. It simply gives me
confidence that I'm not losing any portion of the AST out of my sight.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93110/new/
https://reviews.llvm.org/D93110
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
