[libc-commits] [libc] [libc][NFC] Rename LIBC_COMPILER_HAS_FLOAT128 to LIBC_HAS_FLOAT128 for consistency (PR #81870)

Nick Desaulniers via libc-commits libc-commits at lists.llvm.org
Thu Feb 15 10:29:29 PST 2024


nickdesaulniers wrote:

> We do have a [coding style for macros](https://libc.llvm.org/dev/code_style.html#macro-style) but it is not quite consistent (hence this fix). Basically the naming should reflect where it is defined:

Cool, and I see in this change that you update the docs.  I guess after the update we still don't document `LIBC_COMPILER_HAS_C23_FLOAT16`, or `LIBC_COMPILER_HAS_C23_FLOAT128`.  I feel like before this change, `LIBC_COMPILER_HAS_FLOAT128` is consistent with those (but not `LIBC_HAS_FLOAT16`). After this change `LIBC_HAS_FLOAT128` is consistent with `LIBC_HAS_FLOAT16` (but now no longer with `LIBC_COMPILER_HAS_C23_FLOAT16`). So something still feels off for me.  If the distinction is intentional, looking at https://libc.llvm.org/dev/code_style.html#macro-style I still think we're missing documentation for the distinction; or we need to additionally change `LIBC_COMPILER_HAS_C23_FLOAT16`, or `LIBC_COMPILER_HAS_C23_FLOAT128` (and `LIBC_COMPILER_HAS_FLOAT128_EXTENSION`) in the same fashion as this PR to be entirely consistent.  Otherwise I guess my hangup with this PR is that to me it looks like trading consistency with one set of macros for consistency with a different set, without making both sets consistent as one set.

> The LIBC_COMPILER_HAS_ macros are about the compiler supporting something non standard.
In that regard the following ones are correct:
> LIBC_COMPILER_HAS_C23_FLOAT16
> LIBC_COMPILER_HAS_C23_FLOAT128

But those were standardized, in C23. `LIBC_COMPILER_HAS_FLOAT128_EXTENSION` is the only thing that appears non-standard to me.

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


More information about the libc-commits mailing list