[PATCH] D26750: [clang-tidy] Add modernize-use-default-member-init check

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:34
+  case Type::STK_IntegralComplex:
+    return InitType->isCharType() ? "'\\0'" : "0";
+  case Type::STK_Floating:
----------------
aaron.ballman wrote:
> This is incorrect if the char type is not a narrow character type. I would probably just initialize the integral with `0`, regardless of whether it was a character or not. You should add a test for char, wchar_t, char16_t (et al), and probably all of the other types (just to make sure we handle them properly and don't introduce later regressions).
`isCharType()` returns true for narrow character types only.
So if this is incorrect, wouldn't your suggestion be incorrect too?


================
Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:112
+  }
+}
+
----------------
aaron.ballman wrote:
> You'll need to add the `llvm_unreachable()` here in order to avoid MSVC warnings about not all control paths returning a value.
AFAIK, that's when a switch covers all valid enum values but doesn't have a default case.
This switch has a default case.


https://reviews.llvm.org/D26750