[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Don Hinton via Phabricator via cfe-commits]

hintonda marked an inline comment as done.
hintonda added inline comments.


================
Comment at: clang/lib/Sema/SemaInit.cpp:2069-2072
+    if (!VerifyOnly && HasDesignatedInit && SemaRef.getLangOpts().CPlusPlus2a) {
+      SemaRef.Diag(Init->getBeginLoc(), diag::ext_c20_designated_init)
+          << "mixed" << Init->getSourceRange();
+    }
----------------
rsmith wrote:
> I think it would be preferable to diagnose the "mixed" case in the parser rather than here (it's a grammatical restriction in C++, after all). I'm worried that handling it here will get some cases wrong, such as perhaps:
> 
> ```
> struct A {
>   union { int x, y; };
>   int z;
> };
> A a = { .x = 123, 456 }; // should be rejected, but I think this patch might accept
> ```
> 
> ... and it might also get template instantiation cases wrong for a case like:
> 
> ```
> struct Q { Q(); };
> struct A { Q x, y; };
> template<typename T> void f() {
>   A a = {.y = Q()};
> }
> void g() { f<int>(); }
> ```
> 
> ... where we might possibly end up passing an `InitListExpr` representing `{Q(), .y = Q()}` into `InitListChecker`.
> 
> I think the only C++20 restriction that it makes sense to check in `InitListChecker` is that the field names are in the right order; everything else should be checked earlier. This would also match the semantics of overload resolution, for which the "fields are in the right order" check is deferred until after a function is selected, whereas all the other checks are performed eagerly.
Will work on moving these to the parser.

Btw, the first one is diagnosed correctly, but doesn't complain about the second.  Not sure I grok the problem there either since Q has a default ctor.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59754/new/

https://reviews.llvm.org/D59754