beccadax added a comment.

> My reading of 
> https://clang.llvm.org/docs/LanguageExtensions.html#specifying-an-attribute-for-multiple-declarations-pragma-clang-attribute
>  suggests that any() /could/ apply to everything. (`The any rule applies 
> attributes to all declarations that are matched by at least one of the rules 
> in the any.`)

My reading of that rule suggest that an `any()` with no subrules would not 
match anything because, if you fed in a declaration, none of the (nonexistent) 
subrules of the `any()` would match it.

> My recollection of the discussions when we adopted #pragma clang attribute is 
> that we tried our best to limit the damage from the pragma over-applying 
> attributes, but I don't recall if we explicitly talked about any() with no 
> subjects. What would make me comfortable with this patch is 1) doing some 
> archeology to find the original reviews for pragma clang attribute to see if 
> we made any sort of explicit decision here but forgot to document it. If we 
> didn't make an explicit decision in this area, we should make one now. We can 
> either decide "any() with no subjects applies to everything", "any() with no 
> subjects is dangerous and will apply to nothing", or "any() with no subjects 
> is dangerous and we will diagnose if the user tries this" (probably other 
> ideas we could consider). Ultimately, I would like to see the pragma clang 
> attribute documentation updated before/when we land this bit so that our 
> documentation clearly sets user expectations.

Looking at https://reviews.llvm.org/D30009, I don't see any relevant discussion 
of how `any()` (without subrules) should behave. What I do see is that they 
started with "match everything always" behavior, but concerns about users not 
knowing which declarations it would be applied to (and how that behavior might 
change as the compiler evolved) led them to the idea of requiring users to list 
the declarations they expected the compiler to match and having a way to allow 
a subset <https://reviews.llvm.org/D30009#679481>; then they realized that only 
the subset version was actually useful 
<https://reviews.llvm.org/D30009#706171>. What I take from this is that there 
was clearly concern about users not being able to tell what the pragma would 
affect and they changed the design so that users would need to specify it 
clearly, which suggests that a way to leave it unspecified was seen as an 
anti-goal.

Looking at the implementation of 
`Parser::ParsePragmaAttributeSubjectMatchRuleSet()`, it looks like `any` or 
`any()` would cause a parse error—we return `true` (as in the error branches) 
if we do not parse an open parenthesis or do not parse an identifier after the 
open parenthesis—which again makes me think that the design intentionally left 
out any mechanism to match all subjects.

We don't have a statement that there is and should be no way to apply 
`attribute push` to everything, but I think we can conclude that having such a 
capability wasn't a goal of the final design.

What would you like me to do from here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112773/new/

https://reviews.llvm.org/D112773

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

Reply via email to