[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

Raul Tambre via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 12 07:48:19 PDT 2021


tambre added a comment.

In D77491#2870291 <https://reviews.llvm.org/D77491#2870291>, @jdoerfert wrote:

> First:
> Do I assume right this this feature was simply disabled without any plan to:
>
> - inform the authors (me)
> - update the documentation
> - re-enable support eventually or provide alternatives
>
> XFAILing a test and calling it a day seems inadequate IMHO.
>
> Second:
> Would an approach like this still work: https://reviews.llvm.org/D58531 ?

Informing you would've probably been appropriate in hindsight.
I'm not aware of any relevant documentation that would've been appropriate to update.

Re-enabling the support depends on someone taking up the work to correctly implement the prototype recognition, which seems to be done in D58531 <https://reviews.llvm.org/D58531> and would work as-is.
However, at the time it seemed to me that the whole builtin declaration recognition could use a rewrite to be able to this support this case without hacks and require less manual C++ type juggling.
I considered that work beyond my experience with the codebase and the benefits of this work easily seemed to outweigh the lack of additional annotations for this single function. I received no further feedback on the disabling of that test after explaining the reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77491



More information about the cfe-commits mailing list