[PATCH] D30009: Add support for '#pragma clang attribute'

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: docs/LanguageExtensions.rst:2418
+In general, the attributes are applied to a declaration only when there would
+have been no error or warning for that attribute if it was specified explicitly.
+An attribute is applied to each relevant declaration individually,
----------------
So then this code does not produce an error?
```
#pragma clang attribute push(hot)

void func(void) __attribute__((cold));

#pragma clang attribute pop
```


================
Comment at: docs/LanguageExtensions.rst:2428
+whether or not an attribute is supported by the pragma by referring to the
+:doc:`individual documentation for that attribute <AttributeReference>`.
----------------
This doesn't make clear what would happen with something like:
```
  #pragma clang attribute push(cdecl)

  void function(void (*fp)(int));

  #pragma clang attribute pop
```
Naively, I would expect the cdecl attribute to appertain to both `function` and `fp`, but I'm guessing that's not what actually happens (because cdecl is both a decl and a type attribute at the same time).

Also, why is this not available for __declspec attributes?

Why do C++ attributes require the syntax to be present in the #pragma, but GNU-style attributes do not? There are also MS-style attribute (currently unsupported, but there have been some efforts to include them in the past) and __declspec attributes as well. Why not require the syntax so that any of them can be supported?


================
Comment at: include/clang/Basic/Attr.td:308-311
+  // - An attribute requires delayed parsing (LateParsed is on)
+  // - An attribute has no GNU/CXX11 spelling
+  // - An attribute has no subject list
+  // - An attribute derives from a StmtAttr or a TypeAttr
----------------
I have strong reservations about this -- users are going to have no idea what attributes are and are not supported because they're not going to know whether the attribute has a subject list or requires delayed parsing. We have a considerable number of attributes for which the Subjects line is currently commented out simply because no one has bothered to fix that. This means those attributes can't be used with this pragma until someone fixes that, but when it happens, they magically can be used, which is a good thing. But the converse is more problematic -- if there's an existing Subjects line that gets removed because a subject is added that is difficult to express in TableGen it may break user code.

We can fix the discoverability issues somewhat by updating the documentation emitter to spit out some wording that says when an attribute is/is not supported by this feature, but that only works for attributes which have documentation and so it's not a particularly reliable workaround.


Repository:
  rL LLVM

https://reviews.llvm.org/D30009