[PATCH] D35268: [ObjC] Ambiguous property synthesis should pick the 'readwrite' property and check for incompatible attributes

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.

This patch changes the way ambiguous property synthesis (i.e. when synthesizing a property that's declared in multiple protocols) is performed. Previously, we 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 patch ensures that a first readwrite property is selected. This is a semantic change that breaks users code like this:

  @protocol P @property(readonly) int p; @end
  @protocol P2 @property(readwrite) id p; @end
  @interface I <P2> @end
  @implementation I
  @syntesize p; // We previously got a warning here, but we've synthesized readonly 'int p' here. Now we synthesize readwrite 'id' p..
  @end

I'm not sure if this kind of change is OK. WDYT? I promoted the warning about incompatible types when this kind of readonly/readwrite ambiguity is detected in the `@implementation`, but the problem is that the @interface will still use int `p` for lookup I think. In theory this promotion to an error should avoid user code breakage I think though, but let me know if I missed something.

I also extended the ambiguity checker, and now it can detect conflicts among the different property attributes. I decided to use an error diagnostic for this kind of incompatibility. Let me know if you think this should be relaxed in some cases.

I also got rid of `ProtocolPropertyMap` in favour of a a set + vector because the map's order of iteration is non-deterministic, so I couldn't rely on it to select the readwrite property.

rdar://31579994

Thanks for taking a look,
Alex


Repository:
  rL LLVM

https://reviews.llvm.org/D35268

Files:
  include/clang/AST/DeclObjC.h
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/DeclObjC.cpp
  lib/Sema/SemaObjCProperty.cpp
  test/CodeGenObjC/arc-property.m
  test/SemaObjC/arc-property-decl-attrs.m
  test/SemaObjC/property-ambiguous-synthesis.m

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35268.106060.patch
Type: text/x-patch
Size: 18597 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170711/0515e710/attachment-0001.bin>