rjmccall added inline comments.
================ Comment at: lib/Sema/SemaDeclCXX.cpp:7033 + // member of an ObjC pointer type, except when it has an in-class initializer, + // it doesn't make the defaulted default constructor defined as deleted. + if (!FieldType.hasNonTrivialObjCLifetime() || ---------------- This comment should be talking about non-trivial ownership rather than just general ObjC pointer-ish-ness. ================ Comment at: lib/Sema/SemaDeclCXX.cpp:7036 + (CSM == Sema::CXXDefaultConstructor && FD->hasInClassInitializer())) + return false; + ---------------- I feel like this would be clearer as separate `if` statements; you can put separate comments on each. ================ Comment at: lib/Sema/SemaDeclCXX.cpp:7084 + if (FD->getParent()->isUnion() && + shouldDeleteForVariantObjCPtrMember(FD, FieldType)) ---------------- 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. ================ Comment at: test/SemaObjCXX/arc-0x.mm:164 + union { + union { // expected-note 2 {{'S1' is implicitly deleted because variant field '' has a non-trivial}} expected-note 4 {{'S1' is implicitly deleted because field '' has a deleted}} + id f0; // expected-note 2 {{'' is implicitly deleted because variant field 'f0' is an ObjC pointer}} ---------------- ahatanak wrote: > The diagnostic message here should say the special function is deleted > because the anonymous union's corresponding special function is deleted, but > when diagnosing a deleted copy assignment operator, it says the anonymous > union's special function is non-trivial. I'm not sure this is a bug, but I > see the same diagnostic message when I compile the following non-ObjC code: > > ``` > struct S0 { > S0(const S0 &); > S0 &operator=(const S0 &); > int *p; > }; > > struct S1 { > union { > union { // copy assignment operator of 'S1' is implicitly deleted because > variant field '' has a non-trivial copy assignment operator > S0 s10; > int b; > }; > int c; > }; > ~S1(); > }; > > S1 *x0; > > void testC1(S1 *a0) { > *a0 = *x0; // error: object of type 'S1' cannot be assigned because its > copy assignment operator is implicitly deleted > *a0 = static_cast<S1&&>(*x0); // error: object of type 'S1' cannot be > assigned because its copy assignment operator is implicitly deleted > } > ``` > > It seems that this happens because the following code in > `Sema::ShouldDeleteSpecialMember` is preventing the method declaration from > being marked as deleted: > > ``` > // For an anonymous struct or union, the copy and assignment special members > // will never be used, so skip the check. For an anonymous union declared at > // namespace scope, the constructor and destructor are used. > if (CSM != CXXDefaultConstructor && CSM != CXXDestructor && > RD->isAnonymousStructOrUnion()) > return false; > ``` Well, if it's not different from the ordinary C++ treatment, I think we can justify just improving QoI there separately. 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