diff --git a/include/clang/AST/DeclObjC.h b/include/clang/AST/DeclObjC.h
index 26c0cbe82d1763175f798e5b8fc2e4d8fc9a4cb0..1cd6e004f751f47415c429fc96e470a3bce329fc 100644
--- a/include/clang/AST/DeclObjC.h
+++ b/include/clang/AST/DeclObjC.h
@@ -1039,10 +1039,9 @@ public:
typedef llvm::DenseMap<std::pair<IdentifierInfo*,
unsigned/*isClassProperty*/>,
ObjCPropertyDecl*> PropertyMap;
-
- typedef llvm::DenseMap<const ObjCProtocolDecl *, ObjCPropertyDecl*>
- ProtocolPropertyMap;
-
+
+ typedef llvm::SmallDenseSet<const ObjCProtocolDecl *, 8> ProtocolPropertySet;
+
typedef llvm::SmallVector<ObjCPropertyDecl*, 8> PropertyDeclOrder;
/// This routine collects list of properties to be implemented in the class.
@@ -2159,7 +2158,8 @@ public:
PropertyDeclOrder &PO) const override;
void collectInheritedProtocolProperties(const ObjCPropertyDecl *Property,
- ProtocolPropertyMap &PM) const;
+ ProtocolPropertySet &PS,
+ PropertyDeclOrder &PO) const;
static bool classof(const Decl *D) { return classofKind(D->getKind()); }
static bool classofKind(Kind K) { return K == ObjCProtocol; }
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 5a8750e4dab62fef54ce62aea0e3f22418f8202e..709a96a6c957bcc09d12b329b0080af16b811fde 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -808,8 +808,10 @@ def warn_property_types_are_incompatible : Warning<
"property type %0 is incompatible with type %1 inherited from %2">,
InGroup<DiagGroup<"incompatible-property-type">>;
def warn_protocol_property_mismatch : Warning<
- "property of type %0 was selected for synthesis">,
+ "property %select{of type %1|with attribute '%1'|without attribute '%1'|with "
+ "getter %1|with setter %1}0 was selected for synthesis">,
InGroup<DiagGroup<"protocol-property-synthesis-ambiguity">>;
+def err_protocol_property_mismatch: Error<warn_protocol_property_mismatch.Text>;
def err_undef_interface : Error<"cannot find interface declaration for %0">;
def err_category_forward_interface : Error<
"cannot define %select{category|class extension}0 for undefined class %1">;
@@ -1088,7 +1090,9 @@ def err_category_property : Error<
def note_property_declare : Note<
"property declared here">;
def note_protocol_property_declare : Note<
- "it could also be property of type %0 declared here">;
+ "it could also be property "
+ "%select{of type %1|without attribute '%1'|with attribute '%1'|with getter "
+ "%1|with setter %1}0 declared here">;
def note_property_synthesize : Note<
"property synthesized here">;
def err_synthesize_category_decl : Error<
diff --git a/lib/AST/DeclObjC.cpp b/lib/AST/DeclObjC.cpp
index a0ec0c2b251e181ebf70eef6f77a9b653f9b595d..d8bdb6369e9475891171ca2701362db51ac6fae2 100644
--- a/lib/AST/DeclObjC.cpp
+++ b/lib/AST/DeclObjC.cpp
@@ -1889,25 +1889,23 @@ void ObjCProtocolDecl::collectPropertiesToImplement(PropertyMap &PM,
}
}
-
void ObjCProtocolDecl::collectInheritedProtocolProperties(
- const ObjCPropertyDecl *Property,
- ProtocolPropertyMap &PM) const {
+ const ObjCPropertyDecl *Property, ProtocolPropertySet &PS,
+ PropertyDeclOrder &PO) const {
if (const ObjCProtocolDecl *PDecl = getDefinition()) {
- bool MatchFound = false;
+ if (!PS.insert(PDecl).second)
+ return;
for (auto *Prop : PDecl->properties()) {
if (Prop == Property)
continue;
if (Prop->getIdentifier() == Property->getIdentifier()) {
- PM[PDecl] = Prop;
- MatchFound = true;
- break;
+ PO.push_back(Prop);
+ return;
}
}
// Scan through protocol's protocols which did not have a matching property.
- if (!MatchFound)
- for (const auto *PI : PDecl->protocols())
- PI->collectInheritedProtocolProperties(Property, PM);
+ for (const auto *PI : PDecl->protocols())
+ PI->collectInheritedProtocolProperties(Property, PS, PO);
}
}
diff --git a/lib/Sema/SemaObjCProperty.cpp b/lib/Sema/SemaObjCProperty.cpp
index 62a771bcffa0e2392e1de4c32de078ba90d73893..e1e85dfd5e55eb298c6ab2f168e519cce2873bb9 100644
--- a/lib/Sema/SemaObjCProperty.cpp
+++ b/lib/Sema/SemaObjCProperty.cpp
@@ -814,53 +814,185 @@ static void setImpliedPropertyAttributeForReadOnlyProperty(
property->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_weak);
}
-/// DiagnosePropertyMismatchDeclInProtocols - diagnose properties declared
-/// in inherited protocols with mismatched types. Since any of them can
-/// be candidate for synthesis.
-static void
-DiagnosePropertyMismatchDeclInProtocols(Sema &S, SourceLocation AtLoc,
+static bool
+isIncompatiblePropertyAttribute(unsigned Attr1, unsigned Attr2,
+ ObjCPropertyDecl::PropertyAttributeKind Kind) {
+ return (Attr1 & Kind) != (Attr2 & Kind);
+}
+
+static bool areIncompatiblePropertyAttributes(unsigned Attr1, unsigned Attr2,
+ unsigned Kinds) {
+ return ((Attr1 & Kinds) != 0) != ((Attr2 & Kinds) != 0);
+}
+
+/// SelectPropertyForSynthesisFromProtocols - Finds the most appropriate
+/// property declaration that should be synthesised in all of the inherited
+/// protocols. It also diagnoses properties declared in inherited protocols with
+/// mismatched types or attributes, since any of them can be candidate for
+/// synthesis.
+static ObjCPropertyDecl *
+SelectPropertyForSynthesisFromProtocols(Sema &S, SourceLocation AtLoc,
ObjCInterfaceDecl *ClassDecl,
ObjCPropertyDecl *Property) {
- ObjCInterfaceDecl::ProtocolPropertyMap PropMap;
+ assert(isa<ObjCProtocolDecl>(Property->getDeclContext()) &&
+ "Expected a property from a protocol");
+ ObjCInterfaceDecl::ProtocolPropertySet ProtocolSet;
+ ObjCInterfaceDecl::PropertyDeclOrder Properties;
for (const auto *PI : ClassDecl->all_referenced_protocols()) {
if (const ObjCProtocolDecl *PDecl = PI->getDefinition())
- PDecl->collectInheritedProtocolProperties(Property, PropMap);
+ PDecl->collectInheritedProtocolProperties(Property, ProtocolSet,
+ Properties);
}
- if (ObjCInterfaceDecl *SDecl = ClassDecl->getSuperClass())
+ if (ObjCInterfaceDecl *SDecl = ClassDecl->getSuperClass()) {
while (SDecl) {
for (const auto *PI : SDecl->all_referenced_protocols()) {
if (const ObjCProtocolDecl *PDecl = PI->getDefinition())
- PDecl->collectInheritedProtocolProperties(Property, PropMap);
+ PDecl->collectInheritedProtocolProperties(Property, ProtocolSet,
+ Properties);
}
SDecl = SDecl->getSuperClass();
}
-
- if (PropMap.empty())
- return;
-
+ }
+
+ if (Properties.empty())
+ return Property;
+
+ ObjCPropertyDecl *OriginalProperty = Property;
+ size_t SelectedIndex = 0;
+ for (const auto &Prop : llvm::enumerate(Properties)) {
+ // Select the 'readwrite' property if such property exists.
+ if (Property->isReadOnly() && !Prop.value()->isReadOnly()) {
+ Property = Prop.value();
+ SelectedIndex = Prop.index();
+ }
+ }
+ if (Property != OriginalProperty) {
+ // Check that the old property is compatible with the new one.
+ Properties[SelectedIndex] = OriginalProperty;
+ }
+
QualType RHSType = S.Context.getCanonicalType(Property->getType());
- bool FirsTime = true;
- for (ObjCInterfaceDecl::ProtocolPropertyMap::iterator
- I = PropMap.begin(), E = PropMap.end(); I != E; I++) {
- ObjCPropertyDecl *Prop = I->second;
+ unsigned OriginalAttributes = Property->getPropertyAttributes();
+ enum MismatchKind {
+ IncompatibleType = 0,
+ HasNoExpectedAttribute,
+ HasUnexpectedAttribute,
+ DifferentGetter,
+ DifferentSetter
+ };
+ // Represents a property from another protocol that conflicts with the
+ // selected declaration.
+ struct MismatchingProperty {
+ const ObjCPropertyDecl *Prop;
+ MismatchKind Kind;
+ StringRef AttributeName;
+ };
+ SmallVector<MismatchingProperty, 4> Mismatches;
+ for (ObjCPropertyDecl *Prop : Properties) {
+ // Verify the property attributes.
+ unsigned Attr = Prop->getPropertyAttributes();
+ if (Attr != OriginalAttributes) {
+ auto Diag = [&](bool OriginalHasAttribute, StringRef AttributeName) {
+ MismatchKind Kind = OriginalHasAttribute ? HasNoExpectedAttribute
+ : HasUnexpectedAttribute;
+ Mismatches.push_back({Prop, Kind, AttributeName});
+ };
+ if (isIncompatiblePropertyAttribute(OriginalAttributes, Attr,
+ ObjCPropertyDecl::OBJC_PR_copy)) {
+ Diag(OriginalAttributes & ObjCPropertyDecl::OBJC_PR_copy, "copy");
+ continue;
+ }
+ if (areIncompatiblePropertyAttributes(
+ OriginalAttributes, Attr, ObjCPropertyDecl::OBJC_PR_retain |
+ ObjCPropertyDecl::OBJC_PR_strong)) {
+ Diag(OriginalAttributes & (ObjCPropertyDecl::OBJC_PR_retain |
+ ObjCPropertyDecl::OBJC_PR_strong),
+ "retain (or strong)");
+ continue;
+ }
+ if (isIncompatiblePropertyAttribute(OriginalAttributes, Attr,
+ ObjCPropertyDecl::OBJC_PR_atomic)) {
+ Diag(OriginalAttributes & ObjCPropertyDecl::OBJC_PR_atomic, "atomic");
+ continue;
+ }
+ }
+ if (Property->getGetterName() != Prop->getGetterName()) {
+ Mismatches.push_back({Prop, DifferentGetter, ""});
+ continue;
+ }
+ if (!Property->isReadOnly() && !Prop->isReadOnly() &&
+ Property->getSetterName() != Prop->getSetterName()) {
+ Mismatches.push_back({Prop, DifferentSetter, ""});
+ continue;
+ }
QualType LHSType = S.Context.getCanonicalType(Prop->getType());
if (!S.Context.propertyTypesAreCompatible(LHSType, RHSType)) {
bool IncompatibleObjC = false;
QualType ConvertedType;
if (!S.isObjCPointerConversion(RHSType, LHSType, ConvertedType, IncompatibleObjC)
|| IncompatibleObjC) {
- if (FirsTime) {
- S.Diag(Property->getLocation(), diag::warn_protocol_property_mismatch)
- << Property->getType();
- FirsTime = false;
- }
- S.Diag(Prop->getLocation(), diag::note_protocol_property_declare)
- << Prop->getType();
+ Mismatches.push_back({Prop, IncompatibleType, ""});
+ continue;
}
}
}
- if (!FirsTime && AtLoc.isValid())
+
+ if (Mismatches.empty())
+ return Property;
+
+ // Diagnose incompability.
+ {
+ bool HasIncompatibleAttributes = false;
+ for (const auto &Note : Mismatches)
+ HasIncompatibleAttributes =
+ Note.Kind != IncompatibleType ? true : HasIncompatibleAttributes;
+ // Promote the warning to an error if there are incompatible attributes or
+ // incompatible types together with readwrite/readonly incompatibility.
+ auto Diag = S.Diag(Property->getLocation(),
+ Property != OriginalProperty || HasIncompatibleAttributes
+ ? diag::err_protocol_property_mismatch
+ : diag::warn_protocol_property_mismatch);
+ Diag << Mismatches[0].Kind;
+ switch (Mismatches[0].Kind) {
+ case IncompatibleType:
+ Diag << Property->getType();
+ break;
+ case HasNoExpectedAttribute:
+ case HasUnexpectedAttribute:
+ Diag << Mismatches[0].AttributeName;
+ break;
+ case DifferentGetter:
+ Diag << Property->getGetterName();
+ break;
+ case DifferentSetter:
+ Diag << Property->getSetterName();
+ break;
+ }
+ }
+ for (const auto &Note : Mismatches) {
+ auto Diag =
+ S.Diag(Note.Prop->getLocation(), diag::note_protocol_property_declare)
+ << Note.Kind;
+ switch (Note.Kind) {
+ case IncompatibleType:
+ Diag << Note.Prop->getType();
+ break;
+ case HasNoExpectedAttribute:
+ case HasUnexpectedAttribute:
+ Diag << Note.AttributeName;
+ break;
+ case DifferentGetter:
+ Diag << Note.Prop->getGetterName();
+ break;
+ case DifferentSetter:
+ Diag << Note.Prop->getSetterName();
+ break;
+ }
+ }
+ if (AtLoc.isValid())
S.Diag(AtLoc, diag::note_property_synthesize);
+
+ return Property;
}
/// Determine whether any storage attributes were written on the property.
@@ -996,8 +1128,9 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S,
}
}
if (Synthesize && isa<ObjCProtocolDecl>(property->getDeclContext()))
- DiagnosePropertyMismatchDeclInProtocols(*this, AtLoc, IDecl, property);
-
+ property = SelectPropertyForSynthesisFromProtocols(*this, AtLoc, IDecl,
+ property);
+
} else if ((CatImplClass = dyn_cast<ObjCCategoryImplDecl>(ClassImpDecl))) {
if (Synthesize) {
Diag(AtLoc, diag::err_synthesize_category_decl);
diff --git a/test/CodeGenObjC/arc-property.m b/test/CodeGenObjC/arc-property.m
index b8dc18e872bfc3e148cc0f329ad2dc0ca47e8513..edc4d1e853839ac72955a060363dc75ada923409 100644
--- a/test/CodeGenObjC/arc-property.m
+++ b/test/CodeGenObjC/arc-property.m
@@ -131,4 +131,24 @@ void test3(Test3 *t) {
- (void) setCopyMachine: (id) x {}
@end
+// rdar://31579994
+// When synthesizing a property that's declared in multiple protocols, ensure
+// that the setter is emitted if any of these declarations is readwrite.
+@protocol ABC
+@property (copy, nonatomic, readonly) Test3 *someId;
+@end
+@protocol ABC__Mutable <ABC>
+@property (copy, nonatomic, readwrite) Test3 *someId;
+@end
+
+@interface ABC_Class <ABC, ABC__Mutable>
+@end
+
+@implementation ABC_Class
+@synthesize someId = _someId;
+// CHECK: define internal %{{.*}}* @"\01-[ABC_Class someId]"
+// CHECK: define internal void @"\01-[ABC_Class setSomeId:]"(
+@end
+
+
// CHECK: attributes [[NUW]] = { nounwind }
diff --git a/test/SemaObjC/arc-property-decl-attrs.m b/test/SemaObjC/arc-property-decl-attrs.m
index 6c96ba481c4a2a70856b84f739709a3ce1b2b8e6..ee48d310edc0a27f0578706a8ce95017702fc60b 100644
--- a/test/SemaObjC/arc-property-decl-attrs.m
+++ b/test/SemaObjC/arc-property-decl-attrs.m
@@ -121,3 +121,107 @@ __attribute__((objc_root_class))
@implementation I2
@synthesize prop;
@end
+
+// rdar://31579994
+// Verify that the all of the property declarations in inherited protocols are
+// compatible when synthesing a property from a protocol.
+
+@protocol CopyVsAssign1
+@property (copy, nonatomic, readonly) id prop; // expected-error {{property with attribute 'copy' was selected for synthesis}}
+@end
+@protocol CopyVsAssign2
+@property (assign, nonatomic, readonly) id prop; // expected-note {{it could also be property without attribute 'copy' declared here}}
+@end
+
+@interface CopyVsAssign: Foo <CopyVsAssign1, CopyVsAssign2>
+@end
+@implementation CopyVsAssign
+@synthesize prop; // expected-note {{property synthesized here}}
+@end
+
+@protocol RetainVsNonRetain1
+@property (readonly) id prop; // expected-error {{property without attribute 'retain (or strong)' was selected for synthesis}}
+@end
+@protocol RetainVsNonRetain2
+@property (retain, readonly) id prop; // expected-note {{it could also be property with attribute 'retain (or strong)' declared here}}
+@end
+
+@interface RetainVsNonRetain: Foo <RetainVsNonRetain1, RetainVsNonRetain2>
+@end
+@implementation RetainVsNonRetain
+@synthesize prop; // expected-note {{property synthesized here}}
+@end
+
+@protocol AtomicVsNonatomic1
+@property (copy, nonatomic, readonly) id prop; // expected-error {{property without attribute 'atomic' was selected for synthesis}}
+@end
+@protocol AtomicVsNonatomic2
+@property (copy, atomic, readonly) id prop; // expected-note {{it could also be property with attribute 'atomic' declared here}}
+@end
+
+@interface AtomicVsNonAtomic: Foo <AtomicVsNonatomic1, AtomicVsNonatomic2>
+@end
+@implementation AtomicVsNonAtomic
+@synthesize prop; // expected-note {{property synthesized here}}
+@end
+
+@protocol Getter1
+@property (copy, readonly) id prop; // expected-error {{property with getter 'prop' was selected for synthesis}}
+@end
+@protocol Getter2
+@property (copy, getter=x, readonly) id prop; // expected-note {{it could also be property with getter 'x' declared here}}
+@end
+
+@interface GetterVsGetter: Foo <Getter1, Getter2>
+@end
+@implementation GetterVsGetter
+@synthesize prop; // expected-note {{property synthesized here}}
+@end
+
+@protocol Setter1
+@property (copy, readonly) id prop;
+@end
+@protocol Setter2
+@property (copy, setter=setp:, readwrite) id prop; // expected-error {{property with setter 'setp:' was selected for synthesis}}
+@end
+@protocol Setter3
+@property (copy, readwrite) id prop; // expected-note {{it could also be property with setter 'setProp:' declared here}}
+@end
+
+@interface SetterVsSetter: Foo <Setter1, Setter2, Setter3>
+@end
+@implementation SetterVsSetter
+@synthesize prop; // expected-note {{property synthesized here}}
+@end
+
+@protocol TypeVsAttribute1
+@property (assign, atomic, readonly) int prop; // expected-error {{property of type 'int' was selected for synthesis}}
+@end
+@protocol TypeVsAttribute2
+@property (assign, atomic, readonly) id prop; // expected-note {{it could also be property of type 'id' declared here}}
+@end
+@protocol TypeVsAttribute3
+@property (copy, readonly) id prop; // expected-note {{it could also be property with attribute 'copy' declared here}}
+@end
+
+@interface TypeVsAttribute: Foo <TypeVsAttribute1, TypeVsAttribute2, TypeVsAttribute3>
+@end
+@implementation TypeVsAttribute
+@synthesize prop; // expected-note {{property synthesized here}}
+@end
+
+@protocol TypeVsSetter1
+@property (assign, nonatomic, readonly) int prop; // expected-note {{it could also be property of type 'int' declared here}}
+@end
+@protocol TypeVsSetter2
+@property (assign, nonatomic, readonly) id prop; // ok
+@end
+@protocol TypeVsSetter3
+@property (assign, nonatomic, readwrite) id prop; // expected-error {{property of type 'id' was selected for synthesis}}
+@end
+
+@interface TypeVsSetter: Foo <TypeVsSetter1, TypeVsSetter2, TypeVsSetter3>
+@end
+@implementation TypeVsSetter
+@synthesize prop; // expected-note {{property synthesized here}}
+@end
diff --git a/test/SemaObjC/property-ambiguous-synthesis.m b/test/SemaObjC/property-ambiguous-synthesis.m
index 98f59450744c7152c603331cac61eecac8aef5d3..5c652fa472e7c90d8f09e46e9fad5f56c3170fd7 100644
--- a/test/SemaObjC/property-ambiguous-synthesis.m
+++ b/test/SemaObjC/property-ambiguous-synthesis.m
@@ -2,7 +2,7 @@
// rdar://13075400
@protocol FooAsID
-@property (copy) id foo; // expected-note 2 {{it could also be property of type 'id' declared here}} \\
+@property (assign) id foo; // expected-note 2 {{it could also be property of type 'id' declared here}} \\
// expected-warning {{property of type 'id' was selected for synthesis}}
@end