[PATCH] D45532: [StaticAnalyzer] Checker to find uninitialized fields after a constructor call

Whisperity via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 23 01:55:50 PDT 2018


whisperity added a comment.

@george.karpenkov @NoQ `bugprone.` as a category sounds nice. It also nicely corresponds to the Clang-Tidy `bugprone-` category. It would not be nice to further fragment the "top levels" of checker categories.

I can say with confidence that CodeChecker does not break if the same category name is used by two different analyzers. Does the same stand for XCode / Scan-Build? In this case, introducing the `bugprone` category with the same principle that is behind Tidy's one is a good step.



================
Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:162
+/// We can't know the type of the region that a void pointer points to, so FD
+/// can't be analyzed if this function return true for it.
+bool isVoidPointer(const FieldDecl *FD);
----------------
george.karpenkov wrote:
> "returns"
Actually, this explanation is superfluous. I believe

    Returns if FD can be (transitively) dereferenced to a void* (void**, ...). The type of the region behind a void pointer isn't known, and thus FD can not be analyzed.

should suffice?


================
Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:212
+
+  std::string WarningMsg = std::to_string(UninitFields.size()) +
+                           " uninitialized field" +
----------------
george.karpenkov wrote:
> nitpicking: llvm::Twine is normally used for such constructs
I have also suggested the usage of Twine for this line (it's just the diff that got out of sync with the line numbers!), but I don't recall what @Szelethus' concern was about them. In case we have move semantics enabled, this line will compile into using the moving concatenator.


https://reviews.llvm.org/D45532





More information about the cfe-commits mailing list