rnk added inline comments.

================
Comment at: docs/LanguageExtensions.rst:2349
+attribute is supported by the pragma by referring to the
+:doc:`individual documentation for that attribute <AttributeReference>`.
----------------
arphaman wrote:
> efriedma wrote:
> > arphaman wrote:
> > > arphaman wrote:
> > > > efriedma wrote:
> > > > > I'm wondering if we can tweak the approach so that we don't have to 
> > > > > separately define how this works for each attribute; for example, 
> > > > > `#pragma clang attribute_function_declaration push(...)` would apply 
> > > > > to each function declaration, `#pragma clang 
> > > > > attribute_global_variable_declaration push(...)` would apply to each 
> > > > > global variable declaration, etc.
> > > > I agree with this idea, I think it would be useful to have the ability 
> > > > to specify the target declarations. I do think it would be better to 
> > > > use the 'clang attribute' umbrella pragma, and maybe add an extra 
> > > > argument to the 'push', e.g.:
> > > > 
> > > > ```
> > > > #pragma attribute push (annotate("functions-only"), 
> > > > applicable_to=function) // or maybe received_by=?
> > > > #pragma attribute push (annotate("struct+enum"), applicable_to=struct, 
> > > > applicable_to=enum)
> > > > ```
> > > I think that the further tweaks that control which declarations receive 
> > > the attributes should be kept for a follow-up patch.
> > I'm not sure we can wait to address this, especially if we're turning this 
> > on for all attributes by default.  There's a compatibility hazard here: if 
> > the push doesn't specify what declarations it applies to, we can never 
> > change AttributeAppliesToDecl for any existing attribute.
> Are you saying that we should never allow a push without specifying which 
> declarations should the attribute apply to? I think that would make this 
> feature less useful, as some attributes have a well defined set of 
> declarations that they apply to. Even if we will change the attribute's 
> subjects in the future, I think it might be more likely that the users 
> would've wanted to apply the attribute to the updated compiler-determined 
> subject list.
I agree, users shouldn't have to repeat that attribute "target" applies to 
functions and attribute "warn_unused" applies to record types. Asking the user 
to figure it out just makes it harder to use.


Repository:
  rL LLVM

https://reviews.llvm.org/D30009



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

Reply via email to