ahatanak created this revision.
ahatanak added a reviewer: rjmccall.
ahatanak added a project: clang.
Herald added subscribers: dexonsmith, jkorous.
This patch fixes a bug where clang doesn’t reject union fields of non-trivial
struct types:
struct S0 {
id x;
};
struct S1 {
id y;
};
union U0 {
struct S0 s0; // no diagnostics.
struct S1 s1; // no diagnostics.
};
union U1 {
id x; // clang rejects ObjC pointer fields in unions.
};
void test(union U0 a) {
// Both ‘S0::x’ and ‘S1::y' are destructed in the IR.
}
rdar://problem/46677858
Repository:
rC Clang
https://reviews.llvm.org/D55659
Files:
include/clang/AST/Type.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
test/SemaObjC/arc-decls.m
test/SemaObjCXX/objc-weak.mm
Index: test/SemaObjCXX/objc-weak.mm
===================================================================
--- test/SemaObjCXX/objc-weak.mm
+++ test/SemaObjCXX/objc-weak.mm
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++98 -Wno-c++0x-extensions -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fobjc-runtime-has-weak -fobjc-weak -fblocks -Wno-objc-root-class -std=c++11 -Wno-c++0x-extensions -verify %s
@interface AnObject
@property(weak) id value;
@@ -9,14 +10,19 @@
@end
struct S {
- __weak id a; // expected-note {{because type 'S' has a member with __weak ownership}}
+ __weak id a;
};
union U {
__weak id a; // expected-error {{ARC forbids Objective-C objects in union}}
- S b; // expected-error {{union member 'b' has a non-trivial copy constructor}}
+ S b;
};
+#if __cplusplus < 201103L
+// expected-note@-9 {{because type 'S' has a member with __weak ownership}}
+// expected-error@-5 {{union member 'b' has a non-trivial copy constructor}}
+#endif
+
void testCast(AnObject *o) {
__weak id a = reinterpret_cast<__weak NOWEAK *>(o); // expected-error {{class is incompatible with __weak references}} \
// expected-error {{explicit ownership qualifier on cast result has no effect}} \
Index: test/SemaObjC/arc-decls.m
===================================================================
--- test/SemaObjC/arc-decls.m
+++ test/SemaObjC/arc-decls.m
@@ -10,6 +10,10 @@
id u; // expected-error {{ARC forbids Objective-C objects in union}}
};
+union u_nontrivial_c {
+ struct A a; // expected-error {{ARC forbids non-trivial C types in union}}
+};
+
@interface I {
struct A a;
struct B {
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -15760,13 +15760,13 @@
// Verify that all the fields are okay.
SmallVector<FieldDecl*, 32> RecFields;
- bool ObjCFieldLifetimeErrReported = false;
+ bool NonTrivialPrimitiveFieldErrReported = false;
for (ArrayRef<Decl *>::iterator i = Fields.begin(), end = Fields.end();
i != end; ++i) {
FieldDecl *FD = cast<FieldDecl>(*i);
// Get the type for the field.
- const Type *FDTy = FD->getType().getTypePtr();
+ QualType FDTy = FD->getType();
if (!FD->isAnonymousStructOrUnion()) {
// Remember all fields written by the user.
@@ -15868,6 +15868,40 @@
FD->setInvalidDecl();
EnclosingDecl->setInvalidDecl();
continue;
+ } else if (FDTy->isObjCObjectType()) {
+ /// A field cannot be an Objective-c object
+ Diag(FD->getLocation(), diag::err_statically_allocated_object)
+ << FixItHint::CreateInsertion(FD->getLocation(), "*");
+ QualType T = Context.getObjCObjectPointerType(FD->getType());
+ FD->setType(T);
+ } else if (Record && !NonTrivialPrimitiveFieldErrReported &&
+ Record->isUnion() &&
+ (FDTy.hasNonTrivialObjCLifetime() ||
+ (!getLangOpts().CPlusPlus &&
+ (FDTy.isNonTrivialToPrimitiveDefaultInitialize() ||
+ !FDTy.isTrivialOrTrivialVolatileToPrimitiveCopy() ||
+ !FDTy.isTrivialOrTrivialVolatileToPrimitiveMove() ||
+ FDTy.isDestructedType())))) {
+ // It's an error to have a field that has a non-trivial ObjC lifetime or
+ // a non-trivial C type in a union.
+ // We don't want to report this in a system header, though,
+ // so we just make the field unavailable.
+ // FIXME: that's really not sufficient; we need to make the type
+ // itself invalid to, say, initialize or copy.
+ SourceLocation loc = FD->getLocation();
+ if (getSourceManager().isInSystemHeader(loc)) {
+ if (!FD->hasAttr<UnavailableAttr>()) {
+ FD->addAttr(UnavailableAttr::CreateImplicit(Context, "",
+ UnavailableAttr::IR_ARCFieldWithOwnership, loc));
+ }
+ } else if (FDTy.hasNonTrivialObjCLifetime()) {
+ Diag(FD->getLocation(), diag::err_arc_objc_object_in_tag)
+ << FDTy->isBlockPointerType() << Record->getTagKind();
+ } else {
+ Diag(FD->getLocation(),
+ diag::err_arc_nontrivial_primitive_type_in_union);
+ }
+ NonTrivialPrimitiveFieldErrReported = true;
} else if (const RecordType *FDTTy = FDTy->getAs<RecordType>()) {
if (Record && FDTTy->getDecl()->hasFlexibleArrayMember()) {
// A type which contains a flexible array member is considered to be a
@@ -15899,33 +15933,6 @@
Record->setHasObjectMember(true);
if (Record && FDTTy->getDecl()->hasVolatileMember())
Record->setHasVolatileMember(true);
- } else if (FDTy->isObjCObjectType()) {
- /// A field cannot be an Objective-c object
- Diag(FD->getLocation(), diag::err_statically_allocated_object)
- << FixItHint::CreateInsertion(FD->getLocation(), "*");
- QualType T = Context.getObjCObjectPointerType(FD->getType());
- FD->setType(T);
- } else if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
- Record && !ObjCFieldLifetimeErrReported && Record->isUnion()) {
- // It's an error in ARC or Weak if a field has lifetime.
- // We don't want to report this in a system header, though,
- // so we just make the field unavailable.
- // FIXME: that's really not sufficient; we need to make the type
- // itself invalid to, say, initialize or copy.
- QualType T = FD->getType();
- if (T.hasNonTrivialObjCLifetime()) {
- SourceLocation loc = FD->getLocation();
- if (getSourceManager().isInSystemHeader(loc)) {
- if (!FD->hasAttr<UnavailableAttr>()) {
- FD->addAttr(UnavailableAttr::CreateImplicit(Context, "",
- UnavailableAttr::IR_ARCFieldWithOwnership, loc));
- }
- } else {
- Diag(FD->getLocation(), diag::err_arc_objc_object_in_tag)
- << T->isBlockPointerType() << Record->getTagKind();
- }
- ObjCFieldLifetimeErrReported = true;
- }
} else if (getLangOpts().ObjC &&
getLangOpts().getGC() != LangOptions::NonGC &&
Record && !Record->hasObjectMember()) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5280,6 +5280,8 @@
def err_arc_objc_object_in_tag : Error<
"ARC forbids %select{Objective-C objects|blocks}0 in "
"%select{struct|interface|union|<<ERROR>>|enum}1">;
+def err_arc_nontrivial_primitive_type_in_union : Error<
+ "ARC forbids non-trivial C types in union">;
def err_arc_objc_property_default_assign_on_object : Error<
"ARC forbids synthesizing a property of an Objective-C object "
"with unspecified ownership or storage attribute">;
Index: include/clang/AST/Type.h
===================================================================
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1137,6 +1137,11 @@
/// kind.
PrimitiveCopyKind isNonTrivialToPrimitiveCopy() const;
+ bool isTrivialOrTrivialVolatileToPrimitiveCopy() const {
+ PrimitiveCopyKind Kind = isNonTrivialToPrimitiveCopy();
+ return Kind == PCK_Trivial || Kind == PCK_VolatileTrivial;
+ }
+
/// Check if this is a non-trivial type that would cause a C struct
/// transitively containing this type to be non-trivial to destructively
/// move and return the kind. Destructive move in this context is a C++-style
@@ -1145,6 +1150,10 @@
/// source object is placed in an uninitialized state.
PrimitiveCopyKind isNonTrivialToPrimitiveDestructiveMove() const;
+ bool isTrivialOrTrivialVolatileToPrimitiveMove() const {
+ return isTrivialOrTrivialVolatileToPrimitiveCopy();
+ }
+
enum DestructionKind {
DK_none,
DK_cxx_destructor,
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
