NoQ added inline comments.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugSuppression.h:30-31
+public:
+ /// Return true if the given bug report was explicitly suppressed by the
user.
+ bool isSuppressed(const BugReport &);
+ /// Return true if the bug reported at the given location was explicitly
----------------
vsavchenko wrote:
> NoQ wrote:
> > So, like, i wish this could be replaced with `bool isSuppressed(const
> > PathDiagnostic &);` so that to untie this from Static Analyzer-specific
> > `BugReport` object.
> >
> > There's no straightforward way to jump from `BugReport` to its specific
> > `PathDiagnostic`, especially given that the same bug report may produce
> > different `PathDiagnostic`s for different consumers. But by the time
> > suppressions kick in we can be certain that `PathDiagnostic` objects are
> > fully constructed.
> >
> > It's an interesting question whether different `PathDiagnostic`s for the
> > same bug report may interact with suppressions differently. For now this
> > can't happen because all of them will have the same location and the same
> > uniqueing location. We should obviously avoid such differences, probably
> > protect ourselves against them with an assertion.
> In order to move fully from `BugReport` to `PathDiagnostic`, we can pre-map
> the whole TU with suppressed "areas" (in the same way we do it now for
> declarations with issues). If we do that, we need to decide where and when
> such initialization should take place.
We still have access to decl-with-issue and uniqueing-decl in `PathDiagnostic`
if that's what seems problematic to you (?)
================
Comment at: clang/lib/StaticAnalyzer/Core/BugSuppression.cpp:70
+
+class CacheInitializer : public RecursiveASTVisitor<CacheInitializer> {
+public:
----------------
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93110/new/
https://reviews.llvm.org/D93110
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
