[PATCH] D44539: [Sema][Objective-C] Add check to warn when property of objc type has assign attribute

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D44539#1106443, @aaron.ballman wrote:

> In https://reviews.llvm.org/D44539#1106152, @rjmccall wrote:
>
> > This was approved by the Objective-C language group as a default-off warning.
>
>
> We usually do not expose new default-off warnings because experience shows that they rarely ever get enabled by users. If the ObjC group doesn't think this should be on by default, I wonder if it should be included in Clang at all.


That's a fair question to ask.  In this case, I'm in favor of adding it because we have evidence of there being a substantial set of users who would enable it enthusiastically.



================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1018
+def warn_objc_property_assign_on_object : Warning<
+  "'assign' attribute must not be of object type, use 'unsafe_unretained' instead">,
+  InGroup<ObjCPropertyAssignOnObjectType>, DefaultIgnore;
----------------
QF5690 wrote:
> rjmccall wrote:
> > "must" is rather strong for a warning.  Maybe something more like "'assign' attribute on property of object type could be 'unsafe_unretained'"?
> But "could be" is rather weak :) 
> May be "Prefer using explicit 'unsafe_unretained' attribute instead of 'assign' for object types", or "Using explicit 'unsafe_unretained' attribute instead of 'assign' for object types is preferred" (if passive voice is preferred)
Neither of those is quite in the standard diagnostic "voice".  Maybe something like "'assign' property of object type may become a dangling reference; consider using 'unsafe_unretained'"?

Oh, you should probably not warn about `Class` types.


Repository:
  rC Clang

https://reviews.llvm.org/D44539