rjmccall added a comment.
Asking for a minor tweak, but feel free to commit with that.
================
Comment at: lib/Sema/SemaDeclCXX.cpp:7084
+ if (FD->getParent()->isUnion() &&
+ shouldDeleteForVariantObjCPtrMember(FD, FieldType))
----------------
ahatanak wrote:
> rjmccall wrote:
> > rjmccall wrote:
> > > I believe the right check for variant-ness is `inUnion()`, not
> > > `FD->getParent()->isUnion()`, since the latter can miss cases with e.g.
> > > anonymous `struct`s.
> > Can you try adding a test case here for an anonymous struct nested within
> > an anonymous union?
> Added a test case for an anonymous struct nested within an anonymous union
> (`struct S2` in arc-0x.mm).
>
> I found out that the diagnostic messages clang prints for arc-0x.mm are the
> same whether `inUnion()` or `FD->getParent()->isUnion()` is called. This is
> what I discovered:
>
> `SpecialMemberDeletionInfo` is used in two cases:
>
> 1. To find the deletedness of the special functions in the AST after a class
> is parsed.
>
> 2. When a deleted special function is used, provide more details on why the
> function is deleted and print note diagnostics.
>
> Since the nested classes are parsed before the enclosing classes, we have all
> the information we need about the members that are directly contained to
> correctly infer the deletedness of a class' special functions. So the first
> case should work fine regardless of which method is called.
>
> It doesn't make a difference for the second case either because
> `SpecialMemberDeletionInfo` doesn't try to find out which member of an
> anonymous struct is causing the special function to be deleted; it only does
> so for anonymous unions (which happens inside the if statement near comment
> "// Some additional restrictions exist on the variant members." at line 7140).
Alright. Another place where we could generally improve diagnostics, then.
================
Comment at: test/SemaObjCXX/arc-0x.mm:174
+ union {
+ struct { // expected-note 6 {{'S2' is implicitly deleted because variant
field '' has a non-trivial}}
+ id f0;
----------------
Worth adding a FIXME saying that this note should go on `f1`?
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D57438/new/
https://reviews.llvm.org/D57438
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
