aaron.ballman added inline comments.
================ Comment at: clang-tidy/modernize/UseDefaultMemberInitCheck.cpp:34 + case Type::STK_IntegralComplex: + return InitType->isCharType() ? "'\\0'" : "0"; + case Type::STK_Floating: ---------------- malcolm.parsons wrote: > 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. I can agree that this may be a matter of preference, but my point still stands. If we're going to use a literal type that matches the variable type, we should do that consistently. So if you want to go with `'\0'` for a `char`, please use `L'\0'` for a `wchar_t`, and similar for the other character types. You may also want to consider using the `f` suffix for a `float` rather than relying on the narrowing conversion from double to float. Similar for the complex type suffixes. https://reviews.llvm.org/D26750 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits