[PATCH] D76605: [analyzer] Display the checker name in the text output

Kristóf Umann via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 23 07:36:32 PDT 2020


Szelethus marked 3 inline comments as done.
Szelethus added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Core/TextDiagnostics.cpp:98
       const PathDiagnostic *PD = *I;
+      std::string WarningMsg =
+          (ShouldDisplayCheckerName ? " [" + PD->getCheckerName() + "]" : "")
----------------
martong wrote:
> `StringRef`? `getCheckerName()` returns with `StringRef`...
Mind that we're constructing a string here from a bunch of smaller ones, we need an owning container. With that said, I could use a string stream instead.


================
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());
----------------
martong wrote:
> Why the `.str()` ?
`StringRef` no longer converts to `std::string` implicitly.


================
Comment at: clang/test/Analysis/incorrect-checker-names.mm:1
+// RUN: %clang_analyze_cc1 -fblocks -verify %s -Wno-objc-root-class \
+// RUN:   -analyzer-checker=core \
----------------
martong wrote:
> This seems like a copy of an existing test file, though I don't know which. Wouldn't it be better to adjust the original test file?
Good observation, these test cases are indeed copied from other files. The justification behind it however is to highlight that these are the checkers that need fixing. Adding `FIXME`s to other thousand-line test files don't grab the attention enough :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76605





More information about the cfe-commits mailing list