[libc-commits] [libc] [libc] Make `stdc_first_trailing_one` spec compliant (PR #116493)
via libc-commits
libc-commits at lists.llvm.org
Sat Nov 16 09:11:24 PST 2024
https://github.com/strncmp created https://github.com/llvm/llvm-project/pull/116493
# A small description
`stdc_first_trailing_one` was exhibiting non-compliant behaviour in the edge cases of the type it was operating on. More specifically, when the user passes 0, the function shall return 0, since none of the bits can be asserted. The previous revision implemented this behaviour for the max value of the type instead, making the implementation non-compliant.
Spec reference -> N3220: 7.18.10 First Trailing One
# Issue
Fixes #115719
Please note that this is my first contribution and therefore I don't have commit access
# Review request
cc @nickdesaulniers @michaelrj-google
>From c3c4cea90f0c829941aa25491372c3f250153662 Mon Sep 17 00:00:00 2001
From: strncmp <strnik86 at protonmail.com>
Date: Sat, 16 Nov 2024 18:47:09 +0200
Subject: [PATCH] [libc] Make `stdc_first_trailing_one` spec compliant
`stdc_first_trailing_one` was exhibiting non-compliant behaviour in
the edge cases of the type it was operating on. More specifically,
when the user passes 0, the function shall return 0, since none of
the bits can be asserted. The previous revision implemented this
behaviour for the max value of the type instead, making the
implementation non-compliant.
Spec reference -> N3220: 7.18.10 First Trailing One
---
libc/src/__support/big_int.h | 3 +--
libc/src/__support/math_extras.h | 3 +--
libc/test/src/__support/math_extras_test.cpp | 2 +-
libc/test/src/stdbit/stdc_first_trailing_one_uc_test.cpp | 2 +-
libc/test/src/stdbit/stdc_first_trailing_one_ui_test.cpp | 2 +-
libc/test/src/stdbit/stdc_first_trailing_one_ul_test.cpp | 2 +-
libc/test/src/stdbit/stdc_first_trailing_one_ull_test.cpp | 2 +-
libc/test/src/stdbit/stdc_first_trailing_one_us_test.cpp | 2 +-
8 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/libc/src/__support/big_int.h b/libc/src/__support/big_int.h
index a95ab4ff8e1abf..3745a2c0017b31 100644
--- a/libc/src/__support/big_int.h
+++ b/libc/src/__support/big_int.h
@@ -1375,8 +1375,7 @@ first_trailing_zero(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<is_big_int_v<T>, int>
first_trailing_one(T value) {
- return value == cpp::numeric_limits<T>::max() ? 0
- : cpp::countr_zero(value) + 1;
+ return first_trailing_zero(~value);
}
} // namespace LIBC_NAMESPACE_DECL
diff --git a/libc/src/__support/math_extras.h b/libc/src/__support/math_extras.h
index 6f4a006aad2700..dded3f9352be33 100644
--- a/libc/src/__support/math_extras.h
+++ b/libc/src/__support/math_extras.h
@@ -146,8 +146,7 @@ first_trailing_zero(T value) {
template <typename T>
[[nodiscard]] LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_unsigned_v<T>, int>
first_trailing_one(T value) {
- return value == cpp::numeric_limits<T>::max() ? 0
- : cpp::countr_zero(value) + 1;
+ return value == T(0) ? 0 : cpp::countr_zero(value) + 1;
}
template <typename T>
diff --git a/libc/test/src/__support/math_extras_test.cpp b/libc/test/src/__support/math_extras_test.cpp
index 08c090017c1a19..a45c7851328ee0 100644
--- a/libc/test/src/__support/math_extras_test.cpp
+++ b/libc/test/src/__support/math_extras_test.cpp
@@ -91,7 +91,7 @@ TYPED_TEST(LlvmLibcBitTest, FirstTrailingZero, UnsignedTypesNoBigInt) {
}
TYPED_TEST(LlvmLibcBitTest, FirstTrailingOne, UnsignedTypesNoBigInt) {
- EXPECT_EQ(first_trailing_one<T>(cpp::numeric_limits<T>::max()), 0);
+ EXPECT_EQ(first_trailing_one<T>(T(0)), 0);
for (int i = 0U; i != cpp::numeric_limits<T>::digits; ++i)
EXPECT_EQ(first_trailing_one<T>(T(1) << i), i + 1);
}
diff --git a/libc/test/src/stdbit/stdc_first_trailing_one_uc_test.cpp b/libc/test/src/stdbit/stdc_first_trailing_one_uc_test.cpp
index ed2b4921cdada4..9f481e40cee496 100644
--- a/libc/test/src/stdbit/stdc_first_trailing_one_uc_test.cpp
+++ b/libc/test/src/stdbit/stdc_first_trailing_one_uc_test.cpp
@@ -11,7 +11,7 @@
#include "test/UnitTest/Test.h"
TEST(LlvmLibcStdcFirstTrailingOneUcTest, ALL) {
- EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_uc(UCHAR_MAX), 0U);
+ EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_uc(0), 0U);
}
TEST(LlvmLibcStdcFirstTrailingOneUcTest, OneHot) {
diff --git a/libc/test/src/stdbit/stdc_first_trailing_one_ui_test.cpp b/libc/test/src/stdbit/stdc_first_trailing_one_ui_test.cpp
index 137c8a42e407df..b3415d552a0f35 100644
--- a/libc/test/src/stdbit/stdc_first_trailing_one_ui_test.cpp
+++ b/libc/test/src/stdbit/stdc_first_trailing_one_ui_test.cpp
@@ -11,7 +11,7 @@
#include "test/UnitTest/Test.h"
TEST(LlvmLibcStdcFirstTrailingOneUiTest, ALL) {
- EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_ui(UINT_MAX), 0U);
+ EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_ui(0U), 0U);
}
TEST(LlvmLibcStdcFirstTrailingOneUiTest, OneHot) {
diff --git a/libc/test/src/stdbit/stdc_first_trailing_one_ul_test.cpp b/libc/test/src/stdbit/stdc_first_trailing_one_ul_test.cpp
index 3fc1f3f16c60de..8fd7141862803c 100644
--- a/libc/test/src/stdbit/stdc_first_trailing_one_ul_test.cpp
+++ b/libc/test/src/stdbit/stdc_first_trailing_one_ul_test.cpp
@@ -11,7 +11,7 @@
#include "test/UnitTest/Test.h"
TEST(LlvmLibcStdcFirstTrailingOneUlTest, ALL) {
- EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_ul(ULONG_MAX), 0U);
+ EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_ul(0UL), 0U);
}
TEST(LlvmLibcStdcFirstTrailingOneUlTest, OneHot) {
diff --git a/libc/test/src/stdbit/stdc_first_trailing_one_ull_test.cpp b/libc/test/src/stdbit/stdc_first_trailing_one_ull_test.cpp
index 5719e09a5120a0..ad661bd9bf7bc0 100644
--- a/libc/test/src/stdbit/stdc_first_trailing_one_ull_test.cpp
+++ b/libc/test/src/stdbit/stdc_first_trailing_one_ull_test.cpp
@@ -11,7 +11,7 @@
#include "test/UnitTest/Test.h"
TEST(LlvmLibcStdcFirstTrailingOneUllTest, ALL) {
- EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_ull(ULLONG_MAX), 0U);
+ EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_ull(0ULL), 0U);
}
TEST(LlvmLibcStdcFirstTrailingOneUllTest, OneHot) {
diff --git a/libc/test/src/stdbit/stdc_first_trailing_one_us_test.cpp b/libc/test/src/stdbit/stdc_first_trailing_one_us_test.cpp
index 60021552310bee..674bdd2f0bb33d 100644
--- a/libc/test/src/stdbit/stdc_first_trailing_one_us_test.cpp
+++ b/libc/test/src/stdbit/stdc_first_trailing_one_us_test.cpp
@@ -11,7 +11,7 @@
#include "test/UnitTest/Test.h"
TEST(LlvmLibcStdcFirstTrailingOneUsTest, ALL) {
- EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_us(USHRT_MAX), 0U);
+ EXPECT_EQ(LIBC_NAMESPACE::stdc_first_trailing_one_us(0), 0U);
}
TEST(LlvmLibcStdcFirstTrailingOneUsTest, OneHot) {
More information about the libc-commits
mailing list