[PATCH] D105637: [clang][Analyzer] Add symbol uninterestingness to bug report.

[Artem Dergachev via Phabricator via cfe-commits]

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