ahatanak updated this revision to Diff 219191. ahatanak marked 2 inline comments as done. ahatanak added a comment.
Rename function and fix comments. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65256/new/ https://reviews.llvm.org/D65256 Files: include/clang/AST/ASTContext.h lib/AST/ASTContext.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaExpr.cpp lib/Sema/SemaType.cpp test/SemaObjC/Inputs/non-trivial-c-union.h test/SemaObjC/non-trivial-c-union.m
Index: test/SemaObjC/non-trivial-c-union.m =================================================================== --- test/SemaObjC/non-trivial-c-union.m +++ test/SemaObjC/non-trivial-c-union.m @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -fobjc-runtime-has-weak -verify %s +// RUN: %clang_cc1 -fsyntax-only -fblocks -fobjc-arc -fobjc-runtime-has-weak -I %S/Inputs -verify %s + +#include "non-trivial-c-union.h" typedef union { // expected-note 12 {{'U0' has subobjects that are non-trivial to default-initialize}} expected-note 36 {{'U0' has subobjects that are non-trivial to destruct}} expected-note 28 {{'U0' has subobjects that are non-trivial to copy}} id f0; // expected-note 12 {{f0 has type '__strong id' that is non-trivial to default-initialize}} expected-note 36 {{f0 has type '__strong id' that is non-trivial to destruct}} expected-note 28 {{f0 has type '__strong id' that is non-trivial to copy}} @@ -80,3 +82,7 @@ void testVolatileLValueToRValue(volatile U0 *a) { (void)*a; // expected-error {{cannot use volatile type 'volatile U0' where it causes an lvalue-to-rvalue conversion since it is a union that is non-trivial to destruct}} // expected-error {{cannot use volatile type 'volatile U0' where it causes an lvalue-to-rvalue conversion since it is a union that is non-trivial to copy}} } + +void unionInSystemHeader0(U0_SystemHeader); + +void unionInSystemHeader1(U1_SystemHeader); // expected-error {{cannot use type 'U1_SystemHeader' for a function/method parameter since it is a union that is non-trivial to destruct}} expected-error {{cannot use type 'U1_SystemHeader' for a function/method parameter since it is a union that is non-trivial to copy}} Index: test/SemaObjC/Inputs/non-trivial-c-union.h =================================================================== --- /dev/null +++ test/SemaObjC/Inputs/non-trivial-c-union.h @@ -0,0 +1,19 @@ +// For backward compatibility, fields of C unions declared in system headers +// that have non-trivial ObjC ownership qualifications are marked as unavailable +// unless the qualifier is explicit and __strong. + +#pragma clang system_header + +typedef __strong id StrongID; + +typedef union { + id f0; + _Nonnull id f1; + __weak id f2; + StrongID f3; +} U0_SystemHeader; + +typedef union { // expected-note {{'U1_SystemHeader' has subobjects that are non-trivial to destruct}} expected-note {{'U1_SystemHeader' has subobjects that are non-trivial to copy}} + __strong id f0; // expected-note {{f0 has type '__strong id' that is non-trivial to destruct}} expected-note {{f0 has type '__strong id' that is non-trivial to copy}} + _Nonnull id f1; +} U1_SystemHeader; Index: lib/Sema/SemaType.cpp =================================================================== --- lib/Sema/SemaType.cpp +++ lib/Sema/SemaType.cpp @@ -6027,36 +6027,6 @@ } } -/// Does this type have a "direct" ownership qualifier? That is, -/// is it written like "__strong id", as opposed to something like -/// "typeof(foo)", where that happens to be strong? -static bool hasDirectOwnershipQualifier(QualType type) { - // Fast path: no qualifier at all. - assert(type.getQualifiers().hasObjCLifetime()); - - while (true) { - // __strong id - if (const AttributedType *attr = dyn_cast<AttributedType>(type)) { - if (attr->getAttrKind() == attr::ObjCOwnership) - return true; - - type = attr->getModifiedType(); - - // X *__strong (...) - } else if (const ParenType *paren = dyn_cast<ParenType>(type)) { - type = paren->getInnerType(); - - // That's it for things we want to complain about. In particular, - // we do not want to look through typedefs, typeof(expr), - // typeof(type), or any other way that the type is somehow - // abstracted. - } else { - - return false; - } - } -} - /// handleObjCOwnershipTypeAttr - Process an objc_ownership /// attribute on the specified type. /// @@ -6132,7 +6102,7 @@ if (Qualifiers::ObjCLifetime previousLifetime = type.getQualifiers().getObjCLifetime()) { // If it's written directly, that's an error. - if (hasDirectOwnershipQualifier(type)) { + if (S.Context.hasDirectOwnershipQualifier(type)) { S.Diag(AttrLoc, diag::err_attr_objc_ownership_redundant) << type; return true; Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -15700,27 +15700,11 @@ // Warn about implicitly autoreleasing indirect parameters captured by blocks. if (const auto *PT = CaptureType->getAs<PointerType>()) { - // This function finds out whether there is an AttributedType of kind - // attr::ObjCOwnership in Ty. The existence of AttributedType of kind - // attr::ObjCOwnership implies __autoreleasing was explicitly specified - // rather than being added implicitly by the compiler. - auto IsObjCOwnershipAttributedType = [](QualType Ty) { - while (const auto *AttrTy = Ty->getAs<AttributedType>()) { - if (AttrTy->getAttrKind() == attr::ObjCOwnership) - return true; - - // Peel off AttributedTypes that are not of kind ObjCOwnership. - Ty = AttrTy->getModifiedType(); - } - - return false; - }; - QualType PointeeTy = PT->getPointeeType(); if (!Invalid && PointeeTy->getAs<ObjCObjectPointerType>() && PointeeTy.getObjCLifetime() == Qualifiers::OCL_Autoreleasing && - !IsObjCOwnershipAttributedType(PointeeTy)) { + !S.Context.hasDirectOwnershipQualifier(PointeeTy)) { if (BuildAndDiagnose) { SourceLocation VarLoc = Var->getLocation(); S.Diag(Loc, diag::warn_block_capture_autoreleasing); Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -11139,6 +11139,15 @@ namespace { +bool shouldIgnoreForRecordTriviality(const FieldDecl *FD) { + // Ignore unavailable fields. A field can be marked as unavailable explicitly + // in the source code or implicitly by the compiler if it is in a union + // defined in a system header and has non-trivial ObjC ownership + // qualifications. We don't want those fields to participate in determining + // whether the containing union is non-trivial. + return FD->hasAttr<UnavailableAttr>(); +} + struct DiagNonTrivalCUnionDefaultInitializeVisitor : DefaultInitializedTypeVisitor<DiagNonTrivalCUnionDefaultInitializeVisitor, void> { @@ -11192,7 +11201,8 @@ << 0 << 0 << QT.getUnqualifiedType() << ""; for (const FieldDecl *FD : RD->fields()) - asDerived().visit(FD->getType(), FD, InNonTrivialUnion); + if (!shouldIgnoreForRecordTriviality(FD)) + asDerived().visit(FD->getType(), FD, InNonTrivialUnion); } void visitTrivial(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {} @@ -11256,7 +11266,8 @@ << 0 << 1 << QT.getUnqualifiedType() << ""; for (const FieldDecl *FD : RD->fields()) - asDerived().visit(FD->getType(), FD, InNonTrivialUnion); + if (!shouldIgnoreForRecordTriviality(FD)) + asDerived().visit(FD->getType(), FD, InNonTrivialUnion); } void visitTrivial(QualType QT, const FieldDecl *FD, bool InNonTrivialUnion) {} @@ -11321,7 +11332,8 @@ << 0 << 2 << QT.getUnqualifiedType() << ""; for (const FieldDecl *FD : RD->fields()) - asDerived().visit(FD->getType(), FD, InNonTrivialUnion); + if (!shouldIgnoreForRecordTriviality(FD)) + asDerived().visit(FD->getType(), FD, InNonTrivialUnion); } void preVisit(QualType::PrimitiveCopyKind PCK, QualType QT, @@ -16393,6 +16405,21 @@ << FixItHint::CreateInsertion(FD->getLocation(), "*"); QualType T = Context.getObjCObjectPointerType(FD->getType()); FD->setType(T); + } else if (Record && Record->isUnion() && + FD->getType().hasNonTrivialObjCLifetime() && + getSourceManager().isInSystemHeader(FD->getLocation()) && + !getLangOpts().CPlusPlus && !FD->hasAttr<UnavailableAttr>() && + (FD->getType().getObjCLifetime() != Qualifiers::OCL_Strong || + !Context.hasDirectOwnershipQualifier(FD->getType()))) { + // For backward compatibility, fields of C unions declared in system + // headers that have non-trivial ObjC ownership qualifications are marked + // as unavailable unless the qualifier is explicit and __strong. This can + // break ABI compatibility between programs compiled with ARC and MRR, but + // is a better option than rejecting programs using those unions under + // ARC. + FD->addAttr(UnavailableAttr::CreateImplicit( + Context, "", UnavailableAttr::IR_ARCFieldWithOwnership, + FD->getLocation())); } else if (getLangOpts().ObjC && getLangOpts().getGC() != LangOptions::NonGC && Record && !Record->hasObjectMember()) { @@ -16410,7 +16437,8 @@ } } - if (Record && !getLangOpts().CPlusPlus && !FD->hasAttr<UnavailableAttr>()) { + if (Record && !getLangOpts().CPlusPlus && + !shouldIgnoreForRecordTriviality(FD)) { QualType FT = FD->getType(); if (FT.isNonTrivialToPrimitiveDefaultInitialize()) { Record->setNonTrivialToPrimitiveDefaultInitialize(true); Index: lib/AST/ASTContext.cpp =================================================================== --- lib/AST/ASTContext.cpp +++ lib/AST/ASTContext.cpp @@ -7949,6 +7949,28 @@ return false; } +bool ASTContext::hasDirectOwnershipQualifier(QualType Ty) const { + while (true) { + // __strong id + if (const AttributedType *Attr = dyn_cast<AttributedType>(Ty)) { + if (Attr->getAttrKind() == attr::ObjCOwnership) + return true; + + Ty = Attr->getModifiedType(); + + // X *__strong (...) + } else if (const ParenType *Paren = dyn_cast<ParenType>(Ty)) { + Ty = Paren->getInnerType(); + + // We do not want to look through typedefs, typeof(expr), + // typeof(type), or any other way that the type is somehow + // abstracted. + } else { + return false; + } + } +} + //===----------------------------------------------------------------------===// // ObjCQualifiedIdTypesAreCompatible - Compatibility testing for qualified id's. //===----------------------------------------------------------------------===// Index: include/clang/AST/ASTContext.h =================================================================== --- include/clang/AST/ASTContext.h +++ include/clang/AST/ASTContext.h @@ -2045,6 +2045,11 @@ /// types. bool areCompatibleVectorTypes(QualType FirstVec, QualType SecondVec); + /// Return true if the type has been explicitly qualified with ObjC ownership. + /// A type may be implicitly qualified with ownership under ObjC ARC, and in + /// some cases the compiler treats these differently. + bool hasDirectOwnershipQualifier(QualType Ty) const; + /// Return true if this is an \c NSObject object with its \c NSObject /// attribute set. static bool isObjCNSObjectType(QualType Ty) {
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits