[Sema][ObjC] Mark C union fields that have non-trivial ObjC ownership
qualifications as unavailable if the union is declared in a system
header
r365985 stopped marking those fields as unavailable, which caused the
union's NonTrivialToPrimitive* bits to be set to true. This patch
restores the behavior prior to r365985, except that users can explicitly
specify the ownership qualification of the field to instruct the
compiler not to mark it as unavailable.
rdar://problem/53420753
Differential Revision: https://reviews.llvm.org/D65256
llvm-svn: 371276
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 94a5add..f488ef7 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/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.
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 279332a..023d1fc 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11255,6 +11255,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> {
@@ -11308,7 +11317,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) {}
@@ -11372,7 +11382,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) {}
@@ -11437,7 +11448,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,
@@ -16509,6 +16521,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()) {
@@ -16526,7 +16553,8 @@
}
}
- if (Record && !getLangOpts().CPlusPlus && !FD->hasAttr<UnavailableAttr>()) {
+ if (Record && !getLangOpts().CPlusPlus &&
+ !shouldIgnoreForRecordTriviality(FD)) {
QualType FT = FD->getType();
if (FT.isNonTrivialToPrimitiveDefaultInitialize()) {
Record->setNonTrivialToPrimitiveDefaultInitialize(true);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 7f00981..816d37e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -15714,27 +15714,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);
diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 4281a6a..a563402 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/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;