[PATCH] D131662: [clang] Try to improve diagnostics about uninitialized constexpr variables
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 18 07:23:17 PDT 2022
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
More information about the cfe-commits
mailing list