aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/SemaFixItUtils.cpp:208-210 + if (T->isArrayType()) { + return " = {}"; + } ---------------- tbaeder wrote: > aaron.ballman wrote: > > I don't think this is a good change, consider: > > ``` > > int array[] = {}; > > ``` > > zero-sized arrays are an extension in both C and C++, and the empty > > initializer is a GNU extension in C (at least until C2x). > The code in `SemaInit.cpp` is weird in that it only holds if the fixit isn't > empty... so if that change does away, a `constexpr int arr[2];` also doesn't > use the new error message :( Ugh, well that's rather unfortunate. But I'm not certain the improved diagnostic is worth the incorrect fix-it unless the array has specified bounds. Perhaps another way to address this is to work on the diagnostic behavior to be more like: ``` error: definition of constexpr variable with array type needs an explicit initializer constexpr int a[]; ^ ``` Does that seem more plausible? ================ Comment at: clang/lib/Sema/SemaInit.cpp:8063 // handled in the Failed() branch above. - QualType DestType = Entity.getType(); - S.Diag(Kind.getLocation(), DiagID) - << DestType << (bool)DestType->getAs<RecordType>() - << FixItHint::CreateInsertion(ZeroInitializationFixitLoc, - ZeroInitializationFixit); + if (!DestType->getAs<RecordType>() && VD && VD->isConstexpr()) { + // Use a more useful diagnostic for constexpr variables. ---------------- tbaeder wrote: > aaron.ballman wrote: > > Why the check for a record type? > For record types, the current error message would be: > ``` > default initialization of an object of const type 'const S3' without a > user-provided default constructor > ``` > which gives more information than just "must be initialized by a constant > expression". Yeah, that is a better diagnostic, thanks! Too bad the diagnostic logic is separated like this though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131662/new/ https://reviews.llvm.org/D131662 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits