[PATCH] D83961: [Analyzer] Fix bug report source locations in minimal output.

Mon Jul 20 04:05:10 PDT 2020

Szelethus added a comment.

I find the summary here a bit lacking. We detected this issue in D83120 <https://reviews.llvm.org/D83120>, where a lot of discussion can be found, so its worth linking in. On its own, I'm not immediately sold on whether this is the correct solution, and even if it is, how does it solve the problem. I bet you had to struggle a bit to understand the related machinery to fix this, it'd be invaluable to share the knowledge you gained here as well.

I took a look myself, and the issue fixed here seems to be that `PathDiagnostic`'s constructor doesn't set the associated `PathDiagnosticLocation` itself, and `generateEmptyDiagnosticForReport` pretty much resolves to that. The thing I'm still struggling with, is that I'm not terribly sure whether this the same issue raised in the previous patch.

> Fix of the following problem:
> If the bug report equivalence class contains multiple
> reports and no (minimal) analyzer output was requested
> it can happen that the wrong location is used for the warning.

I have two burning questions about this:

- Are we sure that we used the correct (with the shortest **bug path**) bug report, but associated it with the wrong location? Mind that everything you touched here, as I understand it, affects sorting, not uniqueing.
- If so, some (even if incorrect) location must've been used, where did that come from?

In D83961#2158128 <https://reviews.llvm.org/D83961#2158128>, @balazske wrote:

> Big part of the test failures is with the `osx.cocoa.RetainCount` checker.

Every time :^)

================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:3007
     // of the issue.
+    // This can happen if report is BasicBugReport.
     if (PD->path.empty()) {
----------------
Is this the *only* instance when that happens? How about the `text-minimal` output?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83961/new/

https://reviews.llvm.org/D83961