[PATCH] D51507: Allow all supportable attributes to be used with #pragma clang attribute.

Kristina Brooks via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 30 19:29:59 PDT 2018


kristina added inline comments.


================
Comment at: test/Misc/pragma-attribute-supported-attributes-list.test:100
+// CHECK-NEXT: ObjCReturnsInnerPointer (SubjectMatchRule_objc_method, SubjectMatchRule_objc_property)
+// CHECK-NEXT: ObjCRootClass (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRuntimeName (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_protocol)
----------------
rsmith wrote:
> kristina wrote:
> > There is only one root class in a given runtime (ie. NSObject for objc4, or Object for older Apple runtimes, ObjFW also has a different root class). Having more than one makes no sense especially in the same lexical context as there are no use cases I can think of where you would have more than one active ObjC runtime within a process.
> Thanks. Yes, this attribute probably doesn't make very much sense to use in conjunction with the pragma.
> 
> So, do we explicitly disallow it, or do we allow it with the expectation that it's likely that no-one will ever want to use it? (That is, do we want to disallow cases that are not useful, or merely cases that are not meaningful?) I don't have a strong opinion here, but I'm mildly inclined towards allowing the pragma on any attribute where it's meaningful, as there may be useful uses that we just didn't think of, and it costs nothing to permit.
Yes in theory it could only be used on a single interface in which case it would be perfectly valid. Otherwise when used incorrectly it would issue a diagnostic. As per your inclination of allowing it for all attributes I would say leave it allowed, as it **can** be used in a correct way. Diagnostics are sufficient enough to point out when it happens to apply the attribute to more than one interface.

So given your comment, I would say leave it as allowed.


Repository:
  rC Clang

https://reviews.llvm.org/D51507





More information about the cfe-commits mailing list