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

Don Hinton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 22 07:00:35 PDT 2019


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();
+    }
----------------
hintonda wrote:
> 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.
> 
Woke up this morning and realized what you meant about the union.  I'll take care of it in the next patch.

thanks again...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754





More information about the cfe-commits mailing list