NoQ added a comment.
Looks great, thanks for reusing the reusables!
================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2260-2261
+ InterestingSymbols.erase(sym);
+ if (const auto *meta = dyn_cast<SymbolMetadata>(sym))
+ markNotInteresting(meta->getRegion());
+}
----------------
You're saying, e.g., "If string length is not interesting then neither is the
string itself". Or, dunno, "If the raw pointer value is not interesting then
neither is a smart pointer that was holding it".
I'm not sure about that. I'm pretty sure that no checkers are currently
affected by this code but I still don't understand your point.
I don't understand the original code in `markInteresting()` either; do we have
even one test to cover it?
Also note that what you've written is not a correct negation of the original
code. The correct negation (that would keep the region-metadata relationship in
sync as originally intended) would be "if the region loses interestingness, so
does the metadata". Or it has to go both ways. I'm still not sure if any of
this matters though.
Maybe we should eliminate these extra clauses entirely. If you're not willing
to investigate whether this is all dead code or it actually does something, I'd
like to suggest a "FIXME: Is this necessary?" (or something like that) both
here and in the original code.
================
Comment at: clang/unittests/StaticAnalyzer/Reusables.h:132
+// A consumer to verify the generated diagnostics.
+class DiagnosticVerifyConsumer : public PathDiagnosticConsumer {
+ ExpectedDiagsTy ExpectedDiags;
----------------
We already have [[
https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html |
VerifyDiagnosticConsumer ]]. Maybe `VerifyPathDiagnosticConsumer` to put
emphasis on path-sensitivity (which implies usefulness in unittests because
`VerifyDiagnosticConsumer` operates when all diagnostics are already
indistinguishably flattened) and on its similarity to
`VerifyDiagnosticConsumer`?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105637/new/
https://reviews.llvm.org/D105637
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
