[PATCH] D148700: [clang] Add support for “regular” keyword attributes

Wed Jun 14 06:11:09 PDT 2023

aaron.ballman added a comment.

In D148700#4418767 <https://reviews.llvm.org/D148700#4418767>, @rsandifo-arm wrote:

> Hi @jyknight , @rsmith
>
> Do you have any more thoughts on the above?  Quick version is:
>
> 1. Is it OK to have `[[…]]` attributes in the `arm` namespace that affect semantics?

Yes, any attribute in a vendor namespace can do anything it wants, including impacting semantics.

> 1. Is it OK to raise an error for unrecognised attributes in the `arm` namespace (for a measure of future-proofing)?

Yes, part of "anything it wants" includes diagnostic behavior.

> Given the differing views, I'm unsure whether to revert the series and do (1) (and possibly (2)), or whether to leave things as they are.

The reason why we suggested keywords isn't because of concerns with how *Clang* implements the attribute, it's about portability of the code between Clang and other compilers that don't implement the attribute. Compilers do not issue an error diagnostic on unrecognized attributes in a vendor namespace (per spec they're defined to be ignored: https://eel.is/c++draft/dcl.attr#grammar-6.sentence-1 so if you're lucky, you'll get a warning about the attribute being unknown, but there's no guarantee: https://godbolt.org/z/8qaqPzYYG). So when the attribute impacts semantics in such a way that silently ignoring the attribute would lead to Very Bad Outcomes (think: crashes, ABI mismatches, security concerns, etc), using a keyword for the attribute is a better user experience because unknown keywords are not ignored by implementations (they get diagnosed as a syntax error).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148700