george.karpenkov added a comment. The code looks good apart from a few minor nits. I think I would prefer a new category created for this checker instead of using `alpha`; let's see what @NoQ has to say.
================ Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:305 + HelpText<"Reports uninitialized members in constructors">, + DescFile<"CtorUninitializedMemberChecker.cpp">; + ---------------- `alpha` might not be a right place for this checker. The meaning of `alpha` seems to be "this checker is currently too immature to be used by default because it generates too many FPs". This is not the case for the uninitialized-fields checker, as it finds non-bugs by design, and each project might want to decide whether using such checker is a right fit. I would suggest introducing a new category here, e.g. `bugprone` (other suggestions: `lint`?) @NoQ any objections? ================ 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); ---------------- "returns" ================ Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:212 + + std::string WarningMsg = std::to_string(UninitFields.size()) + + " uninitialized field" + ---------------- nitpicking: llvm::Twine is normally used for such constructs ================ Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:448 +bool isVoidPointer(const FieldDecl *FD) { + QualType T = FD->getType(); + ---------------- Might be a naive question, but what about a chain intermixing references and pointers? Wouldn't it be simpler to write ``` while (T) if (T->isVoidPointerType()) return true; T = T->getPointeeType(); ``` ? ================ Comment at: test/Analysis/ctor-uninitialized-member.cpp:817 + UnionWithRecord(){}; + } u; // xpected-note{{uninitialized field 'this->u'}} + ---------------- What happened to `e`? Here and in all notes below. https://reviews.llvm.org/D45532 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits