Szelethus marked 3 inline comments as done.
Szelethus added inline comments.
================
Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:313-316
+ANALYZER_OPTION(bool, ShouldDisplayCheckerNameForText, "display-checker-name",
+ "Display the checker name for textual outputs",
+ true)
+
----------------
NoQ wrote:
> Why do we need an option? Is it just for tests? Is it for clang-tidy to avoid
> printing the checker name twice?
>
> Why do we need an option?
Well, we don't, but there is just no reason not to make this configurable, as
seen in the debug checker test file. It would be unnecessary noise in their
case.
> Is it just for tests?
Tests are a big motivating factor, but it just doesn't hurt to know (just like
in clang-tidy!). I suspect this change affects developers or powerusers more
than any others, and could accelerate debugging and/or configuring just a tiny
bit more.
> Is it for clang-tidy to avoid printing the checker name twice?
Clang-tidy isnt affected, as they use the PD_NONE output type, not PD_TEXT.
================
Comment at: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp:112
reportPiece(NoteID, Piece->getLocation().asLocation(),
- Piece->getString(), Piece->getRanges(),
Piece->getFixits());
+ Piece->getString().str(), Piece->getRanges(),
+ Piece->getFixits());
----------------
NoQ wrote:
> Szelethus wrote:
> > martong wrote:
> > > Why the `.str()` ?
> > `StringRef` no longer converts to `std::string` implicitly.
> But it seems to have been fine before(?)
Mind that I changed the parameter of `reportPiece` to `std::string` from
`StringRef`.
================
Comment at: clang/test/Analysis/incorrect-checker-names.cpp:6
+ int x = 0;
+ // FIXME: This shouldn't be tied to a modeling checker.
+ return &x; // expected-warning{{Address of stack memory associated with
local variable 'x' returned to caller [core.StackAddrEscapeBase]}}
----------------
NoQ wrote:
> Do you have a plan to address this FIXME? Does clang-tidy have the same
> problem?
It does not. This is the issue of emitting reports under the wrong name, and I
do have plans to address it, though I'm very anxious about
`RetainCountChecker`. Its huge, the modeling/bug emission parts of it a very
much intertwined, and me dum dum about ObjC.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76605/new/
https://reviews.llvm.org/D76605
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
