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

Malcolm Parsons via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 20 06:42:35 PST 2016


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:
> malcolm.parsons wrote:
> > 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?
> Ah, I think I made a confusing statement. By "incorrect", I meant, "inconsistent." For int, you would get 0, for char, you would get '\0', but for wchar_t you would get 0 instead of L'\0', for char16_t you would get 0 instead of u'\0', etc. Rather than deal with all of the prefixing, I think it's better to just initialize with 0 for all integral types.
All of these scalar types could be initialised with 0, but I prefer nullptr, false, '\0' and 0.0 for pointers, bool, char and float respectively.
If I used wchar_t, char16_t or char32_t, I'd probably want the prefixing.


https://reviews.llvm.org/D26750





More information about the cfe-commits mailing list