[PATCH] D16517: ClangTidy check to flag uninitialized builtin and pointer fields.

[Felix Berger via cfe-commits]

flx added inline comments.

================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:178
@@ +177,3 @@
+
+  SmallPtrSet<const FieldDecl *, 16> FieldsToInit;
+  fieldsRequiringInit(MemberFields, FieldsToInit);
----------------
alexfh wrote:
> What about this? (relates to line 184 now)
My comment on this was not submitted, just in case. Here it is again:

I think he idea is to return immediately if we're delegating to another constructor here. Am I misunderstanding the API call?

The important part is that we return in that case.

================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:124
@@ +123,3 @@
+  std::vector<FieldsInsertion> OrderedFields;
+  OrderedFields.push_back({LastWrittenNonMemberInit, nullptr, {}});
+
----------------
alexfh wrote:
> `.emplace_back(...)`?
Gave a compile error. Probably because there is no explicit constructor with the 3 arguments.

================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:177
@@ +176,3 @@
+  for (CXXCtorInitializer *Init : Ctor->inits()) {
+    if (Init->isDelegatingInitializer())
+      return;
----------------
alexfh wrote:
> I think, this is superfluous. `isMemberInitializer()` should be enough.
I think he idea is to return immediately if we're delegating to another constructor here. Is this covered through some other check?

================
Comment at: clang-tidy/misc/UninitializedFieldCheck.cpp:199
@@ +198,3 @@
+
+    for (const auto *Field : FieldsToInit)
+      Builder << FixItHint::CreateInsertion(
----------------
alexfh wrote:
> nit: We don't use braces only for single-line `if/for` bodies. Same below.
Done. This seems really subtle though when it depends on line length instead of single statement inside of loop.

================
Comment at: test/clang-tidy/misc-uninitialized-field.cpp:1
@@ +1,2 @@
+// RUN: %check_clang_tidy %s misc-uninitialized-field %t
+
----------------
alexfh wrote:
> As usual: tests with templates and macros, please ;)
Thanks for the reminder. Added early return for code inside of macros. Added template test which revealed I had to suppress the check inside of instantiated constructors.


http://reviews.llvm.org/D16517