[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