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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
