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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to