Index: include/clang/AST/DeclObjC.h =================================================================== --- include/clang/AST/DeclObjC.h +++ include/clang/AST/DeclObjC.h @@ -1039,10 +1039,9 @@ typedef llvm::DenseMap, ObjCPropertyDecl*> PropertyMap; - - typedef llvm::DenseMap - ProtocolPropertyMap; - + + typedef llvm::SmallDenseSet ProtocolPropertySet; + typedef llvm::SmallVector PropertyDeclOrder; /// This routine collects list of properties to be implemented in the class. @@ -2159,7 +2158,8 @@ 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; } Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -808,8 +808,10 @@ "property type %0 is incompatible with type %1 inherited from %2">, InGroup>; 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>; +def err_protocol_property_mismatch: Error; 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 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< Index: lib/AST/DeclObjC.cpp =================================================================== --- lib/AST/DeclObjC.cpp +++ lib/AST/DeclObjC.cpp @@ -1889,25 +1889,23 @@ } } - 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); } } Index: lib/Sema/SemaObjCProperty.cpp =================================================================== --- lib/Sema/SemaObjCProperty.cpp +++ lib/Sema/SemaObjCProperty.cpp @@ -814,53 +814,185 @@ 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(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 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 @@ } } if (Synthesize && isa(property->getDeclContext())) - DiagnosePropertyMismatchDeclInProtocols(*this, AtLoc, IDecl, property); - + property = SelectPropertyForSynthesisFromProtocols(*this, AtLoc, IDecl, + property); + } else if ((CatImplClass = dyn_cast(ClassImpDecl))) { if (Synthesize) { Diag(AtLoc, diag::err_synthesize_category_decl); Index: test/CodeGenObjC/arc-property.m =================================================================== --- test/CodeGenObjC/arc-property.m +++ test/CodeGenObjC/arc-property.m @@ -131,4 +131,24 @@ - (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 +@property (copy, nonatomic, readwrite) Test3 *someId; +@end + +@interface ABC_Class +@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 } Index: test/SemaObjC/arc-property-decl-attrs.m =================================================================== --- test/SemaObjC/arc-property-decl-attrs.m +++ test/SemaObjC/arc-property-decl-attrs.m @@ -121,3 +121,107 @@ @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 +@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 +@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 +@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 +@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 +@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 +@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 +@end +@implementation TypeVsSetter +@synthesize prop; // expected-note {{property synthesized here}} +@end Index: test/SemaObjC/property-ambiguous-synthesis.m =================================================================== --- test/SemaObjC/property-ambiguous-synthesis.m +++ 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