[ObjC] Pick a 'readwrite' property when synthesizing ambiguous

property and check for incompatible attributes

This commit changes the way ambiguous property synthesis (i.e. when synthesizing
a property that's declared in multiple protocols) is performed. Previously,
Clang synthesized the first property that was found. This lead to problems when
the property was synthesized in a class that conformed to two protocols that
declared that property and a second protocols had a 'readwrite' declaration -
the setter was not synthesized so the class didn't really conform to the second
protocol and user's code would crash at runtime when they would try to set the
property.

This commit ensures that a first readwrite property is selected. This is a
semantic change that changes users code in this manner:

```
@protocol P @property(readonly) int p; @end
@protocol P2 @property(readwrite) id p; @end
@interface I <P2> @end
@implementation I
@syntesize p; // Users previously got a warning here, and Clang synthesized
              // readonly 'int p' here. Now Clang synthesizes readwrite 'id' p..
@end
```

To ensure that this change is safe, the warning about incompatible types is
promoted to an error when this kind of readonly/readwrite ambiguity is detected
in the @implementation. This will ensure that previous code that had this subtle
bug and ignored the warning now will fail to compile with an error, and users
should not get suprises at runtime once they resolve the error.

The commit also extends the ambiguity checker, and now it can detect conflicts
among the different property attributes. An error diagnostic is used for
conflicting attributes, to ensure that the user won't get "suprises" at runtime.

ProtocolPropertyMap is removed in favour of a a set + vector because the map's
order of iteration is non-deterministic, so it couldn't be used to select the
readwrite property.

rdar://31579994

Differential Revision: https://reviews.llvm.org/D35268


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@307903 91177308-0d34-0410-b5e6-96231b3b80d8

2 files changed, 11 insertions, 7 deletions

diff --git a/include/clang/AST/DeclObjC.h b/include/clang/AST/DeclObjC.h
index 26c0cbe82d..1cd6e004f7 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 5a8750e4da..709a96a6c9 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<