[libcxx-commits] [libcxx] [libc++][RFC] Refactor attributes to [[attribute_macro]] (PR #130099)

[Mark de Wever via libcxx-commits]

mordante wrote:

I've looked a few times at the patch to form an opinion, hence the late reaction.

I'm not too fond of the attribute syntax, it makes the text longer this means that there fits less on a line. I agree it's nice when the editor does proper syntax highlighting, however I read quite a lot of code during reviewing on GitHub where we have no syntax highlighting.

I have the feeling this proposal does not really showcase what the final code looks like, I still see code like `[[__libcpp_deprecated_in_cxx20()]] _LIBCPP_HIDE_FROM_ABI void` is this intended to remain like this or become `[[__libcpp_deprecated_in_cxx20(), _LIBCPP_HIDE_FROM_ABI]] void`?
I really would like to see one or more files changed to what your final vision is, I'm perfectly fine when the CI directly fails for the code. That would really help me to better evaluate what you envision. Maybe you can do these changes in a separate draft PR so it's easy to review them.

I really dislike changing macros to lowercase. This makes it harder for the reader to know when something is or is not a macro. Once we do that for "attributes" there is a precedence to use lowercase macros which I think is not a direction we should move to. Using uppercase macros is the industry standard, so it's easy for new contributors to determine whether or not something is a macro. (Regardless whether we use `_LIBCPP_FOO` or `[[_LIBCPP_FOO]]`).

I agree with @ldionne we should pursue the deprecated diagnostic in a separate patch. I don't mind to see that in the example I requested above.

https://github.com/llvm/llvm-project/pull/130099