[PATCH] misc-uninitialized-field

[Jonathan B Coe]

================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:14
@@ +13,3 @@
+#include <unordered_set>
+#include <sstream>
+#include <iterator>
----------------
alexfh wrote:
> #includes should be sorted (e.g. using the llvm-include-order check, `clang-tidy -fix` should do the trick).
I've applied my understanding of http://llvm.org/docs/CodingStandards.html#include-style. The clang-tidy check you suggested running made no changes to the code.

Please let me know if I've misunderstood anything.

================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:24
@@ +23,3 @@
+
+static bool fieldRequiresInit(const clang::FieldDecl *f) {
+  if (f->getType()->isPointerType()) 
----------------
alexfh wrote:
> Variable names should start with a capital letter.
I'll change this. 
Out of curiosity, why is this LLVM's way of naming variables? I've not seen this style used elsewhere.

================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:25
@@ +24,3 @@
+static bool fieldRequiresInit(const clang::FieldDecl *f) {
+  if (f->getType()->isPointerType()) 
+    return true;
----------------
alexfh wrote:
> alexfh wrote:
> >   return f->getType()->isPointerType() || f->getType()->isBuiltinType();
> Did you miss this comment?
yes. I will address this.

================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:56
@@ +55,3 @@
+    
+    for (clang::CXXCtorInitializer *Init : Ctor->inits()) {
+      const FieldDecl *MemberField = Init->getMember();
----------------
alexfh wrote:
> What about fields initialized in the constructor body?
My check does not take that into account. I think it's hard to do in general so I decided to let the user decide to ignore the check and proposed change. I presume that the optimiser is smart enough to do the right thing if a variable is obviously initialized in the constructor body and constructor field initializers.


================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:56
@@ +55,3 @@
+    
+    for (clang::CXXCtorInitializer *Init : Ctor->inits()) {
+      const FieldDecl *MemberField = Init->getMember();
----------------
alexfh wrote:
> jbcoe wrote:
> > alexfh wrote:
> > > What about fields initialized in the constructor body?
> > My check does not take that into account. I think it's hard to do in general so I decided to let the user decide to ignore the check and proposed change. I presume that the optimiser is smart enough to do the right thing if a variable is obviously initialized in the constructor body and constructor field initializers.
> > 
> What about this?
I got confused by comment submission. Comment added.

================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:70
@@ +69,3 @@
+
+    std::stringstream ss;
+    for (const auto MemberField : MemberFields) 
----------------
alexfh wrote:
> alexfh wrote:
> > In LLVM code, `llvm::raw_string_ostream` or `llvm::raw_svector_ostream` are used instead of `std::stringstream`.
> Did you miss this comment?
I'm using a string instead. llvm::raw_string_ostream takes a string ref as a constuctor argument and the code is simpler if I just use the string directly.

http://reviews.llvm.org/D10553

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/