NoQ added inline comments.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2320-2323
+ void traverseTU(const TranslationUnitDecl *TU) {
+ for (auto I = TU->decls_begin(); I != TU->decls_end(); ++I)
+ TraverseDecl(*I);
+ }
----------------
`RecursiveASTVisitor` does this automatically. Just ask it to `Visit(TU)`.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2335
+ // To analyze callables:
+ if (isa<FunctionDecl, BlockDecl, ObjCMethodDecl>(Node)) {
+ // For code in dependent contexts, we'll do this at instantiation time:
----------------
The intended way to make recursive visitors is to have `TraverseFunctionDecl`,
`TraverseBlockDecl`, etc., which automatically do the `isa` check and fall
through to the superclass if the method isn't defined. Then they can all call a
common method to analyze the body.
This doesn't necessarily lead to better code though, just to more traditional
code, so up to you^^ in this case it probably doesn't matter. But it's
definitely nice to separate the common part (the part that invokes all
individual analyses) into its own method. And then you can eg. isolate the
special `hasBody()` check in the `TraverseFunctionDecl()` method, so that other
code paths didn't have an extra runtime check.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2343-2345
+ // `FunctionDecl->hasBody()` returns true if the function has a body
+ // somewhere defined. But we want to know if this `Node` has a body
+ // child. So we use `doesThisDeclarationHaveABody`:
----------------
I wonder if `ObjCMethodDecl` has the same problem. They too can have prototypes
and definitions separate from each other.
================
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2363-2364
+ if (isa<LambdaExpr>(Node)) {
+ // to visit implicit children of `LambdaExpr`s:
+ IsVisitingLambda = true;
+
----------------
I suspect that you might have reinvented `shouldVisitLambdaBody()`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146342/new/
https://reviews.llvm.org/D146342
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
