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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith marked an inline comment as done.
rsmith added a comment.

Based on the comments so far, my inclination is to allow `#pragma clang attribute` on all these attributes, including the highlighted ones where it's hard to think of cases where it'd be useful. I think our policy should be that we only disable support for attributes where the pragma is actually meaningless (we couldn't figure out where to apply the attribute, for example), not merely ones where we cannot imagine a use. Please comment if you disagree, of course! :)



================
Comment at: test/Misc/pragma-attribute-supported-attributes-list.test:58
 // CHECK-NEXT: IFunc (SubjectMatchRule_function)
+// CHECK-NEXT: InitPriority (SubjectMatchRule_variable)
 // CHECK-NEXT: InternalLinkage (SubjectMatchRule_variable, SubjectMatchRule_function, SubjectMatchRule_record)
----------------
kristina wrote:
> Adding remark about `init_priority` as an inline comment. Wouldn't make sense semantically as it defeats the point of explicitly specifying static init order and cause a clash instead and causes ordering to become undefined again (as well as issuing a diagnostic I think).
As far as I can tell, it's valid to have multiple globals with the same `init_priority`, and they are initialized in the order in which they are declared within a source file. This also seems useful when combined with the pragma (as a way of moving the initialization of a group of globals earlier / later, without affecting their relative order).


================
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)
----------------
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.


Repository:
  rC Clang

https://reviews.llvm.org/D51507