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

[Louis Dionne via libcxx-commits]

https://github.com/ldionne requested changes to this pull request.

The PR states a few benefits of this change which mostly revolve around readability. I think I agree with them, once we're past the initial disruption of making a change of this scale and we've gotten used to the new syntax.

I do wonder if the cost of that disruption is worth it though, since the benefits, while nice, seem like a "nice to have". In particular, I think we need to think the following things through:
- Can we apply this syntax to all attributes? If not, then we introduced new inconsistencies in the codebase and I think that greatly reduces the benefit of this change.
- These "attribute" macros can be utilized to apply things that are not pure attributes today. For example, `_LIBCPP_METHOD_TEMPLATE_IMPLICIT_INSTANTIATION_VIS` expands to `inline _LIBCPP_HIDDEN` sometimes. What do we want to do with this (horrible) macro? Do we have other use cases for this capability of a macro to expand to non-attribute things?

Apart from that, I think that the ability to provide a message to some attributes like the deprecation attributes is great. We should pursue that in a separate patch, and probably before this refactoring since I can see it taking a while.

Finally, about naming. I am a bit conflicted with the choice of naming for this new style of attributes. On the one hand, I think it's a good convention to use `_LIBCPP_FOO` for macros. It's consistent with what we do everywhere. On the other hand, I do agree that `_UPPERCASE` makes these macros use up a lot of visual real estate, and I think the code would be more readable with something more low profile. I don't have a fully formed opinion about that yet.

Overall, I think this change may make sense but I'd like us to have answers/positions to the questions/concerns above. We'd also want to make sure that we engage with other frequent contributors to get their opinion on the change since this will touch a lot of things (in a mechanical way).

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