https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/117364
>From 37923ec9c6a0bbabd2640f7e95549bf560974e59 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Fri, 22 Nov 2024 14:18:33 -0500 Subject: [PATCH 1/4] [C23] Fixed the value of BOOL_WIDTH --- clang/docs/ReleaseNotes.rst | 2 ++ clang/lib/Frontend/InitPreprocessor.cpp | 10 ++++++++- clang/test/C/C23/n2412.c | 27 ++++++++++++++++++++++++ clang/test/Preprocessor/init-aarch64.c | 4 ++-- clang/test/Preprocessor/init-loongarch.c | 4 ++-- clang/test/Preprocessor/init.c | 2 +- 6 files changed, 43 insertions(+), 6 deletions(-) create mode 100644 clang/test/C/C23/n2412.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 8bd06fadfdc984..ff8c2f92e875f8 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -332,6 +332,8 @@ C23 Feature Support - Clang now supports `N3029 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3029.htm>`_ Improved Normal Enumerations. - Clang now officially supports `N3030 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3030.htm>`_ Enhancements to Enumerations. Clang already supported it as an extension, so there were no changes to compiler behavior. +- Fixed the value of ``BOOL_WIDTH`` in ``<limits.h>`` to return ``1`` + explicitly, as mandated by the standard. Non-comprehensive list of changes in this release ------------------------------------------------- diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp index 9a0fdb175ff29e..6ecb201fd46970 100644 --- a/clang/lib/Frontend/InitPreprocessor.cpp +++ b/clang/lib/Frontend/InitPreprocessor.cpp @@ -1103,7 +1103,15 @@ static void InitializePredefinedMacros(const TargetInfo &TI, assert(TI.getCharWidth() == 8 && "Only support 8-bit char so far"); Builder.defineMacro("__CHAR_BIT__", Twine(TI.getCharWidth())); - Builder.defineMacro("__BOOL_WIDTH__", Twine(TI.getBoolWidth())); + // The macro is specifying the number of bits in the value representation, + // not the number of bits in the object representation, which is what + // getBoolWidth() will return. For the bool/_Bool data type, there is only + // ever one bit in the value representation. See C23 6.2.6.2p2 for the rules + // in C. Note that [basic.fundamental]p10 allows an implementation-defined + // value representation for bool; Clang represents bool as an i1 when + // lowering to LLVM, so this value is also correct for C++ for our + // implementation. + Builder.defineMacro("__BOOL_WIDTH__", "1"); Builder.defineMacro("__SHRT_WIDTH__", Twine(TI.getShortWidth())); Builder.defineMacro("__INT_WIDTH__", Twine(TI.getIntWidth())); Builder.defineMacro("__LONG_WIDTH__", Twine(TI.getLongWidth())); diff --git a/clang/test/C/C23/n2412.c b/clang/test/C/C23/n2412.c new file mode 100644 index 00000000000000..7d4f32ae68a734 --- /dev/null +++ b/clang/test/C/C23/n2412.c @@ -0,0 +1,27 @@ +// RUN: %clang_cc1 -verify -std=c23 -ffreestanding %s + +/* WG14 N2412: Clang 14 + * Two's complement sign representation + */ +// expected-no-diagnostics + +#include <limits.h> + +// GH117348 -- BOOL_WIDTH was accidentally expanding to the number of bits in +// the object representation (8) rather than the number of bits in the value +// representation (1). +static_assert(BOOL_WIDTH == 1); + +// Validate the other macro requirements. +static_assert(CHAR_WIDTH == SCHAR_WIDTH); +static_assert(CHAR_WIDTH == UCHAR_WIDTH); +static_assert(CHAR_WIDTH == CHAR_BIT); + +static_assert(USHRT_WIDTH >= 16); +static_assert(UINT_WIDTH >= 16); +static_assert(ULONG_WIDTH >= 32); +static_assert(ULLONG_WIDTH >= 64); +static_assert(BITINT_MAXWIDTH >= ULLONG_WIDTH); + +static_assert(MB_LEN_MAX >= 1); + diff --git a/clang/test/Preprocessor/init-aarch64.c b/clang/test/Preprocessor/init-aarch64.c index c52c49a94e016e..8ee6c6ba60af43 100644 --- a/clang/test/Preprocessor/init-aarch64.c +++ b/clang/test/Preprocessor/init-aarch64.c @@ -44,7 +44,7 @@ // AARCH64: #define __BIGGEST_ALIGNMENT__ 16 // AARCH64_BE-NEXT: #define __BIG_ENDIAN__ 1 // AARCH64-NEXT: #define __BITINT_MAXWIDTH__ 128 -// AARCH64-NEXT: #define __BOOL_WIDTH__ 8 +// AARCH64-NEXT: #define __BOOL_WIDTH__ 1 // AARCH64_BE-NEXT: #define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__ // AARCH64_LE-NEXT: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ // AARCH64-NEXT: #define __CHAR16_TYPE__ unsigned short @@ -785,7 +785,7 @@ // ARM64EC-MSVC: #define __ATOMIC_SEQ_CST 5 // ARM64EC-MSVC: #define __BIGGEST_ALIGNMENT__ 16 // ARM64EC-MSVC: #define __BITINT_MAXWIDTH__ 128 -// ARM64EC-MSVC: #define __BOOL_WIDTH__ 8 +// ARM64EC-MSVC: #define __BOOL_WIDTH__ 1 // ARM64EC-MSVC: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ // ARM64EC-MSVC: #define __CHAR16_TYPE__ unsigned short // ARM64EC-MSVC: #define __CHAR32_TYPE__ unsigned int diff --git a/clang/test/Preprocessor/init-loongarch.c b/clang/test/Preprocessor/init-loongarch.c index 0eb6977a2553c9..0e3320f01b328c 100644 --- a/clang/test/Preprocessor/init-loongarch.c +++ b/clang/test/Preprocessor/init-loongarch.c @@ -25,7 +25,7 @@ // LA32-NEXT: #define __ATOMIC_SEQ_CST 5 // LA32: #define __BIGGEST_ALIGNMENT__ 16 // LA32: #define __BITINT_MAXWIDTH__ 128 -// LA32: #define __BOOL_WIDTH__ 8 +// LA32: #define __BOOL_WIDTH__ 1 // LA32: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ // LA32: #define __CHAR16_TYPE__ unsigned short // LA32: #define __CHAR32_TYPE__ unsigned int @@ -346,7 +346,7 @@ // LA64-NEXT: #define __ATOMIC_SEQ_CST 5 // LA64: #define __BIGGEST_ALIGNMENT__ 16 // LA64: #define __BITINT_MAXWIDTH__ 128 -// LA64: #define __BOOL_WIDTH__ 8 +// LA64: #define __BOOL_WIDTH__ 1 // LA64: #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ // LA64: #define __CHAR16_TYPE__ unsigned short // LA64: #define __CHAR32_TYPE__ unsigned int diff --git a/clang/test/Preprocessor/init.c b/clang/test/Preprocessor/init.c index c177975114332a..05225c120b13de 100644 --- a/clang/test/Preprocessor/init.c +++ b/clang/test/Preprocessor/init.c @@ -1617,7 +1617,7 @@ // WEBASSEMBLY-NEXT:#define __ATOMIC_SEQ_CST 5 // WEBASSEMBLY-NEXT:#define __BIGGEST_ALIGNMENT__ 16 // WEBASSEMBLY-NEXT:#define __BITINT_MAXWIDTH__ 128 -// WEBASSEMBLY-NEXT:#define __BOOL_WIDTH__ 8 +// WEBASSEMBLY-NEXT:#define __BOOL_WIDTH__ 1 // WEBASSEMBLY-NEXT:#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__ // WEBASSEMBLY-NEXT:#define __CHAR16_TYPE__ unsigned short // WEBASSEMBLY-NEXT:#define __CHAR32_TYPE__ unsigned int >From b5b4d4eed1ee430fa6c23905b44c81d137c86570 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Fri, 22 Nov 2024 14:21:06 -0500 Subject: [PATCH 2/4] Update release note to mention this fixes a user-reported issue --- clang/docs/ReleaseNotes.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ff8c2f92e875f8..d26e1739213d09 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -333,7 +333,7 @@ C23 Feature Support - Clang now supports `N3029 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3029.htm>`_ Improved Normal Enumerations. - Clang now officially supports `N3030 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3030.htm>`_ Enhancements to Enumerations. Clang already supported it as an extension, so there were no changes to compiler behavior. - Fixed the value of ``BOOL_WIDTH`` in ``<limits.h>`` to return ``1`` - explicitly, as mandated by the standard. + explicitly, as mandated by the standard. Fixes #GH117348 Non-comprehensive list of changes in this release ------------------------------------------------- >From bc9047ee3584d613bb13d737226353baca955ea8 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Mon, 25 Nov 2024 07:46:21 -0500 Subject: [PATCH 3/4] Reword comment based on review feedback --- clang/lib/Frontend/InitPreprocessor.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp index 6ecb201fd46970..fd3fa26b7b4b68 100644 --- a/clang/lib/Frontend/InitPreprocessor.cpp +++ b/clang/lib/Frontend/InitPreprocessor.cpp @@ -1103,14 +1103,14 @@ static void InitializePredefinedMacros(const TargetInfo &TI, assert(TI.getCharWidth() == 8 && "Only support 8-bit char so far"); Builder.defineMacro("__CHAR_BIT__", Twine(TI.getCharWidth())); - // The macro is specifying the number of bits in the value representation, - // not the number of bits in the object representation, which is what - // getBoolWidth() will return. For the bool/_Bool data type, there is only - // ever one bit in the value representation. See C23 6.2.6.2p2 for the rules - // in C. Note that [basic.fundamental]p10 allows an implementation-defined - // value representation for bool; Clang represents bool as an i1 when - // lowering to LLVM, so this value is also correct for C++ for our - // implementation. + // The macro is specifying the number of bits in the width, not the number of + // bits the object requires for its in-memory representation, which is what + // getBoolWidth() will return. The bool/_Bool data type is only ever one bit + // wide. See C23 6.2.6.2p2 for the rules in C. Note that + // C++23 [basic.fundamental]p10 allows an implementation-defined value + // representation for bool; when lowing to LLVM, Clang represents bool as an + // i8 in memory but as an i1 needing the value, so '1' is also correct for + // C++ for our implementation. Builder.defineMacro("__BOOL_WIDTH__", "1"); Builder.defineMacro("__SHRT_WIDTH__", Twine(TI.getShortWidth())); Builder.defineMacro("__INT_WIDTH__", Twine(TI.getIntWidth())); >From d2902b9b75a1f9fc8575242e89e8f9fa0c7dc553 Mon Sep 17 00:00:00 2001 From: Aaron Ballman <aa...@aaronballman.com> Date: Mon, 25 Nov 2024 07:47:35 -0500 Subject: [PATCH 4/4] Fixing a typo and shortening the wording a bit. --- clang/lib/Frontend/InitPreprocessor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp index fd3fa26b7b4b68..de37b1da87249f 100644 --- a/clang/lib/Frontend/InitPreprocessor.cpp +++ b/clang/lib/Frontend/InitPreprocessor.cpp @@ -1109,8 +1109,8 @@ static void InitializePredefinedMacros(const TargetInfo &TI, // wide. See C23 6.2.6.2p2 for the rules in C. Note that // C++23 [basic.fundamental]p10 allows an implementation-defined value // representation for bool; when lowing to LLVM, Clang represents bool as an - // i8 in memory but as an i1 needing the value, so '1' is also correct for - // C++ for our implementation. + // i8 in memory but as an i1 when the value is needed, so '1' is also correct + // for C++. Builder.defineMacro("__BOOL_WIDTH__", "1"); Builder.defineMacro("__SHRT_WIDTH__", Twine(TI.getShortWidth())); Builder.defineMacro("__INT_WIDTH__", Twine(TI.getIntWidth())); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits