This is an archive of the discontinued LLVM Phabricator instance.

[Analyzer][BugReporter] Replace the example bug report with the one used to generate PathDiagnostic
Needs ReviewPublic

Authored by OikawaKirie on Dec 14 2021, 2:23 AM.

Details

Summary

In function BugReporter::FlushReport, function PathSensitiveBugReporter::generatePathDiagnostics will re-select one report from the equivalence class to construct the PathDiagnostic. It makes the report used to generate PathDiagnostic may be different from the one used in function BugReporter::FlushReport. Changes to the bug report via a BugReporterVisitor may not be reflected when generating report notes or fix-hints.

This patch tries to fix this problem by using the report used to generate PathDiagnostic directly in function BugReporter::FlushReport to generate notes.

Diff Detail

Unit TestsFailed

Event Timeline

OikawaKirie created this revision.Dec 14 2021, 2:23 AM
OikawaKirie requested review of this revision.Dec 14 2021, 2:23 AM

First off, your patch is great, and I'm pretty sure we want it!

I remember working around here, and I either have never quite understood what makes exampleReport an example, or have long forgotten. Can we not just rename it to chosenReport, or simply report?

clang/lib/StaticAnalyzer/Core/BugReporter.cpp
3064–3074

Wouldn't it make more sense to create initialize (instead of resetting) report at the call to generateDiagnosticForConsumerMap()? This just looks a bit convoluted. We could turn everything up to that point into a predicate function.

OikawaKirie edited the summary of this revision. (Show Details)

First off, your patch is great, and I'm pretty sure we want it!

I remember working around here, and I either have never quite understood what makes exampleReport an example, or have long forgotten. Can we not just rename it to chosenReport, or simply report?

Thanks a lot.

New updates:

  1. add a new BugReporter::generateDiagnosticForConsumerMap method to handle the job before adding notes
  2. add a BugReport field to DiagnosticForConsumerMapTy
  3. move findReportInEquivalenceClass calls to generateDiagnosticForConsumerMap methods
  4. the exampleReport variable in method findReportInEquivalenceClass is unchanged, others are removed as they are no longer required
Szelethus added inline comments.Jan 4 2022, 1:54 AM
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
76

Some comments about this field would be welcome!

clang/lib/StaticAnalyzer/Core/BugReporter.cpp
3076–3077

Considering that the consumers are already a part of the BugReport, the overload on generateDiagnosticForConsumerMap feels too arbitrary. Wouldn't a predicate function instead of an overload be more fitting?

3210

Ohh, a variable with its pants down!

3286–3290

Really happy to see exampleReport gone as a parameter.