[libc-commits] [libc] 82ca29c - [libc] Move str{, r}chr implementation to headers
Alex Brachet via libc-commits
libc-commits at lists.llvm.org
Mon Apr 10 18:17:27 PDT 2023
Author: Alex Brachet
Date: 2023-04-11T01:16:56Z
New Revision: 82ca29ce54d3b7d8e47aa151e0ce3dff37727c1a
URL: https://github.com/llvm/llvm-project/commit/82ca29ce54d3b7d8e47aa151e0ce3dff37727c1a
DIFF: https://github.com/llvm/llvm-project/commit/82ca29ce54d3b7d8e47aa151e0ce3dff37727c1a.diff
LOG: [libc] Move str{,r}chr implementation to headers
Differential Revision: https://reviews.llvm.org/D147463
Added:
libc/test/src/string/StrchrTest.h
Modified:
libc/src/string/CMakeLists.txt
libc/src/string/strchr.cpp
libc/src/string/strchrnul.cpp
libc/src/string/string_utils.h
libc/src/string/strrchr.cpp
libc/test/src/string/CMakeLists.txt
libc/test/src/string/strchr_test.cpp
libc/test/src/string/strrchr_test.cpp
utils/bazel/llvm-project-overlay/libc/test/src/string/BUILD.bazel
Removed:
################################################################################
diff --git a/libc/src/string/CMakeLists.txt b/libc/src/string/CMakeLists.txt
index ee43c934aedbb..f6980ca632bb9 100644
--- a/libc/src/string/CMakeLists.txt
+++ b/libc/src/string/CMakeLists.txt
@@ -105,6 +105,8 @@ add_entrypoint_object(
strchr.cpp
HDRS
strchr.h
+ DEPENDS
+ .string_utils
)
add_entrypoint_object(
@@ -323,6 +325,8 @@ add_entrypoint_object(
strrchr.cpp
HDRS
strrchr.h
+ DEPENDS
+ .string_utils
)
add_entrypoint_object(
diff --git a/libc/src/string/strchr.cpp b/libc/src/string/strchr.cpp
index 724c2a2d16490..3b804e94404ec 100644
--- a/libc/src/string/strchr.cpp
+++ b/libc/src/string/strchr.cpp
@@ -9,15 +9,13 @@
#include "src/string/strchr.h"
#include "src/__support/common.h"
+#include "src/string/string_utils.h"
namespace __llvm_libc {
// TODO: Look at performance benefits of comparing words.
LLVM_LIBC_FUNCTION(char *, strchr, (const char *src, int c)) {
- const char ch = static_cast<char>(c);
- for (; *src && *src != ch; ++src)
- ;
- return *src == ch ? const_cast<char *>(src) : nullptr;
+ return internal::strchr_implementation(src, c);
}
} // namespace __llvm_libc
diff --git a/libc/src/string/strchrnul.cpp b/libc/src/string/strchrnul.cpp
index 4dc28cd216f44..d8dce65dca527 100644
--- a/libc/src/string/strchrnul.cpp
+++ b/libc/src/string/strchrnul.cpp
@@ -14,10 +14,7 @@
namespace __llvm_libc {
LLVM_LIBC_FUNCTION(char *, strchrnul, (const char *src, int c)) {
- const char ch = static_cast<char>(c);
- for (; *src && *src != ch; ++src)
- ;
- return const_cast<char *>(src);
+ return internal::strchr_implementation<false>(src, c);
}
} // namespace __llvm_libc
diff --git a/libc/src/string/string_utils.h b/libc/src/string/string_utils.h
index c778ccf170135..572d47d639565 100644
--- a/libc/src/string/string_utils.h
+++ b/libc/src/string/string_utils.h
@@ -224,6 +224,25 @@ LIBC_INLINE size_t strlcpy(char *__restrict dst, const char *__restrict src,
return len;
}
+template <bool ReturnNull = true>
+constexpr static char *strchr_implementation(const char *src, int c) {
+ char ch = static_cast<char>(c);
+ for (; *src && *src != ch; ++src)
+ ;
+ char *ret = ReturnNull ? nullptr : const_cast<char *>(src);
+ return *src == ch ? const_cast<char *>(src) : ret;
+}
+
+constexpr static char *strrchr_implementation(const char *src, int c) {
+ char ch = static_cast<char>(c);
+ char *last_occurrence = nullptr;
+ for (; *src; ++src) {
+ if (*src == ch)
+ last_occurrence = const_cast<char *>(src);
+ }
+ return last_occurrence;
+}
+
} // namespace internal
} // namespace __llvm_libc
diff --git a/libc/src/string/strrchr.cpp b/libc/src/string/strrchr.cpp
index 2e987fa2b8604..d821327a998d3 100644
--- a/libc/src/string/strrchr.cpp
+++ b/libc/src/string/strrchr.cpp
@@ -9,17 +9,12 @@
#include "src/string/strrchr.h"
#include "src/__support/common.h"
+#include "src/string/string_utils.h"
namespace __llvm_libc {
LLVM_LIBC_FUNCTION(char *, strrchr, (const char *src, int c)) {
- const char ch = static_cast<char>(c);
- char *last_occurrence = nullptr;
- for (; *src; ++src) {
- if (*src == ch)
- last_occurrence = const_cast<char *>(src);
- }
- return last_occurrence;
+ return internal::strrchr_implementation(src, c);
}
} // namespace __llvm_libc
diff --git a/libc/test/src/string/CMakeLists.txt b/libc/test/src/string/CMakeLists.txt
index 9d5933edf3b0d..ed9d38aa482a1 100644
--- a/libc/test/src/string/CMakeLists.txt
+++ b/libc/test/src/string/CMakeLists.txt
@@ -84,6 +84,12 @@ add_libc_unittest(
libc.src.string.strcat
)
+add_header_library(
+ strchr_test_support
+ HDRS
+ StrchrTest.h
+)
+
add_libc_unittest(
strchr_test
SUITE
@@ -92,6 +98,7 @@ add_libc_unittest(
strchr_test.cpp
DEPENDS
libc.src.string.strchr
+ .strchr_test_support
)
add_libc_unittest(
@@ -306,6 +313,7 @@ add_libc_unittest(
strrchr_test.cpp
DEPENDS
libc.src.string.strrchr
+ .strchr_test_support
)
add_libc_unittest(
diff --git a/libc/test/src/string/StrchrTest.h b/libc/test/src/string/StrchrTest.h
new file mode 100644
index 0000000000000..1c5bee52863cc
--- /dev/null
+++ b/libc/test/src/string/StrchrTest.h
@@ -0,0 +1,201 @@
+//===-- Tests for str{,r}chr and {,r}index functions ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "test/UnitTest/Test.h"
+
+template <auto Func> struct StrchrTest : public __llvm_libc::testing::Test {
+ void findsFirstCharacter() {
+ const char *src = "abcde";
+
+ // Should return original string since 'a' is the first character.
+ ASSERT_STREQ(Func(src, 'a'), "abcde");
+ // Source string should not change.
+ ASSERT_STREQ(src, "abcde");
+ }
+
+ void findsMiddleCharacter() {
+ const char *src = "abcde";
+
+ // Should return characters after (and including) 'c'.
+ ASSERT_STREQ(Func(src, 'c'), "cde");
+ // Source string should not change.
+ ASSERT_STREQ(src, "abcde");
+ }
+
+ void findsLastCharacterThatIsNotNullTerminator() {
+ const char *src = "abcde";
+
+ // Should return 'e' and null-terminator.
+ ASSERT_STREQ(Func(src, 'e'), "e");
+ // Source string should not change.
+ ASSERT_STREQ(src, "abcde");
+ }
+
+ void findsNullTerminator() {
+ const char *src = "abcde";
+
+ // Should return null terminator.
+ ASSERT_STREQ(Func(src, '\0'), "");
+ // Source string should not change.
+ ASSERT_STREQ(src, "abcde");
+ }
+
+ void characterNotWithinStringShouldReturnNullptr() {
+ // Since 'z' is not within the string, should return nullptr.
+ ASSERT_STREQ(Func("123?", 'z'), nullptr);
+ }
+
+ void theSourceShouldNotChange() {
+ const char *src = "abcde";
+ // When the character is found, the source string should not change.
+ Func(src, 'd');
+ ASSERT_STREQ(src, "abcde");
+ // Same case for when the character is not found.
+ Func(src, 'z');
+ ASSERT_STREQ(src, "abcde");
+ // Same case for when looking for nullptr.
+ Func(src, '\0');
+ ASSERT_STREQ(src, "abcde");
+ }
+
+ void shouldFindFirstOfDuplicates() {
+ // '1' is duplicated in the string, but it should find the first copy.
+ ASSERT_STREQ(Func("abc1def1ghi", '1'), "1def1ghi");
+
+ const char *dups = "XXXXX";
+ // Should return original string since 'X' is the first character.
+ ASSERT_STREQ(Func(dups, 'X'), dups);
+ }
+
+ void emptyStringShouldOnlyMatchNullTerminator() {
+ // Null terminator should match.
+ ASSERT_STREQ(Func("", '\0'), "");
+ // All other characters should not match.
+ ASSERT_STREQ(Func("", 'Z'), nullptr);
+ ASSERT_STREQ(Func("", '3'), nullptr);
+ ASSERT_STREQ(Func("", '*'), nullptr);
+ }
+};
+
+template <auto Func> struct StrrchrTest : public __llvm_libc::testing::Test {
+ void findsFirstCharacter() {
+ const char *src = "abcde";
+
+ // Should return original string since 'a' is the first character.
+ ASSERT_STREQ(Func(src, 'a'), "abcde");
+ // Source string should not change.
+ ASSERT_STREQ(src, "abcde");
+ }
+
+ void findsMiddleCharacter() {
+ const char *src = "abcde";
+
+ // Should return characters after (and including) 'c'.
+ ASSERT_STREQ(Func(src, 'c'), "cde");
+ // Source string should not change.
+ ASSERT_STREQ(src, "abcde");
+ }
+
+ void findsLastCharacterThatIsNotNullTerminator() {
+ const char *src = "abcde";
+
+ // Should return 'e' and null-terminator.
+ ASSERT_STREQ(Func(src, 'e'), "e");
+ // Source string should not change.
+ ASSERT_STREQ(src, "abcde");
+ }
+
+ void findsNullTerminator() {
+ const char *src = "abcde";
+
+ // Should return null terminator.
+ ASSERT_STREQ(Func(src, '\0'), "");
+ // Source string should not change.
+ ASSERT_STREQ(src, "abcde");
+ }
+
+ void findsLastBehindFirstNullTerminator() {
+ const char src[6] = {'a', 'a', '\0', 'b', '\0', 'c'};
+ // 'b' is behind a null terminator, so should not be found.
+ ASSERT_STREQ(Func(src, 'b'), nullptr);
+ // Same goes for 'c'.
+ ASSERT_STREQ(Func(src, 'c'), nullptr);
+
+ // Should find the second of the two a's.
+ ASSERT_STREQ(Func(src, 'a'), "a");
+ }
+
+ void characterNotWithinStringShouldReturnNullptr() {
+ // Since 'z' is not within the string, should return nullptr.
+ ASSERT_STREQ(Func("123?", 'z'), nullptr);
+ }
+
+ void shouldFindLastOfDuplicates() {
+ // '1' is duplicated in the string, but it should find the last copy.
+ ASSERT_STREQ(Func("abc1def1ghi", '1'), "1ghi");
+
+ const char *dups = "XXXXX";
+ // Should return the last occurrence of 'X'.
+ ASSERT_STREQ(Func(dups, 'X'), "X");
+ }
+
+ void emptyStringShouldOnlyMatchNullTerminator() {
+ // Null terminator should match.
+ ASSERT_STREQ(Func("", '\0'), "");
+ // All other characters should not match.
+ ASSERT_STREQ(Func("", 'A'), nullptr);
+ ASSERT_STREQ(Func("", '2'), nullptr);
+ ASSERT_STREQ(Func("", '*'), nullptr);
+ }
+};
+
+#define STRCHR_TEST(name, func) \
+ using LlvmLibc##name##Test = StrchrTest<func>; \
+ TEST_F(LlvmLibc##name##Test, FindsFirstCharacter) { findsFirstCharacter(); } \
+ TEST_F(LlvmLibc##name##Test, FindsMiddleCharacter) { \
+ findsMiddleCharacter(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, FindsLastCharacterThatIsNotNullTerminator) { \
+ findsLastCharacterThatIsNotNullTerminator(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, FindsNullTerminator) { findsNullTerminator(); } \
+ TEST_F(LlvmLibc##name##Test, CharacterNotWithinStringShouldReturnNullptr) { \
+ characterNotWithinStringShouldReturnNullptr(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, TheSourceShouldNotChange) { \
+ theSourceShouldNotChange(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, ShouldFindFirstOfDuplicates) { \
+ shouldFindFirstOfDuplicates(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, EmptyStringShouldOnlyMatchNullTerminator) { \
+ emptyStringShouldOnlyMatchNullTerminator(); \
+ }
+
+#define STRRCHR_TEST(name, func) \
+ using LlvmLibc##name##Test = StrrchrTest<func>; \
+ TEST_F(LlvmLibc##name##Test, FindsFirstCharacter) { findsFirstCharacter(); } \
+ TEST_F(LlvmLibc##name##Test, FindsMiddleCharacter) { \
+ findsMiddleCharacter(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, FindsLastCharacterThatIsNotNullTerminator) { \
+ findsLastCharacterThatIsNotNullTerminator(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, FindsNullTerminator) { findsNullTerminator(); } \
+ TEST_F(LlvmLibc##name##Test, FindsLastBehindFirstNullTerminator) { \
+ findsLastBehindFirstNullTerminator(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, CharacterNotWithinStringShouldReturnNullptr) { \
+ characterNotWithinStringShouldReturnNullptr(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, ShouldFindLastOfDuplicates) { \
+ shouldFindLastOfDuplicates(); \
+ } \
+ TEST_F(LlvmLibc##name##Test, EmptyStringShouldOnlyMatchNullTerminator) { \
+ emptyStringShouldOnlyMatchNullTerminator(); \
+ }
diff --git a/libc/test/src/string/strchr_test.cpp b/libc/test/src/string/strchr_test.cpp
index c61927925f167..684b631a6e9bd 100644
--- a/libc/test/src/string/strchr_test.cpp
+++ b/libc/test/src/string/strchr_test.cpp
@@ -6,77 +6,9 @@
//
//===----------------------------------------------------------------------===//
+#include "StrchrTest.h"
+
#include "src/string/strchr.h"
#include "test/UnitTest/Test.h"
-TEST(LlvmLibcStrChrTest, FindsFirstCharacter) {
- const char *src = "abcde";
-
- // Should return original string since 'a' is the first character.
- ASSERT_STREQ(__llvm_libc::strchr(src, 'a'), "abcde");
- // Source string should not change.
- ASSERT_STREQ(src, "abcde");
-}
-
-TEST(LlvmLibcStrChrTest, FindsMiddleCharacter) {
- const char *src = "abcde";
-
- // Should return characters after (and including) 'c'.
- ASSERT_STREQ(__llvm_libc::strchr(src, 'c'), "cde");
- // Source string should not change.
- ASSERT_STREQ(src, "abcde");
-}
-
-TEST(LlvmLibcStrChrTest, FindsLastCharacterThatIsNotNullTerminator) {
- const char *src = "abcde";
-
- // Should return 'e' and null-terminator.
- ASSERT_STREQ(__llvm_libc::strchr(src, 'e'), "e");
- // Source string should not change.
- ASSERT_STREQ(src, "abcde");
-}
-
-TEST(LlvmLibcStrChrTest, FindsNullTerminator) {
- const char *src = "abcde";
-
- // Should return null terminator.
- ASSERT_STREQ(__llvm_libc::strchr(src, '\0'), "");
- // Source string should not change.
- ASSERT_STREQ(src, "abcde");
-}
-
-TEST(LlvmLibcStrChrTest, CharacterNotWithinStringShouldReturnNullptr) {
- // Since 'z' is not within the string, should return nullptr.
- ASSERT_STREQ(__llvm_libc::strchr("123?", 'z'), nullptr);
-}
-
-TEST(LlvmLibcStrChrTest, TheSourceShouldNotChange) {
- const char *src = "abcde";
- // When the character is found, the source string should not change.
- __llvm_libc::strchr(src, 'd');
- ASSERT_STREQ(src, "abcde");
- // Same case for when the character is not found.
- __llvm_libc::strchr(src, 'z');
- ASSERT_STREQ(src, "abcde");
- // Same case for when looking for null terminator.
- __llvm_libc::strchr(src, '\0');
- ASSERT_STREQ(src, "abcde");
-}
-
-TEST(LlvmLibcStrChrTest, ShouldFindFirstOfDuplicates) {
- // '1' is duplicated in the string, but it should find the first copy.
- ASSERT_STREQ(__llvm_libc::strchr("abc1def1ghi", '1'), "1def1ghi");
-
- const char *dups = "XXXXX";
- // Should return original string since 'X' is the first character.
- ASSERT_STREQ(__llvm_libc::strchr(dups, 'X'), dups);
-}
-
-TEST(LlvmLibcStrChrTest, EmptyStringShouldOnlyMatchNullTerminator) {
- // Null terminator should match.
- ASSERT_STREQ(__llvm_libc::strchr("", '\0'), "");
- // All other characters should not match.
- ASSERT_STREQ(__llvm_libc::strchr("", 'Z'), nullptr);
- ASSERT_STREQ(__llvm_libc::strchr("", '3'), nullptr);
- ASSERT_STREQ(__llvm_libc::strchr("", '*'), nullptr);
-}
+STRCHR_TEST(Strchr, __llvm_libc::strchr)
diff --git a/libc/test/src/string/strrchr_test.cpp b/libc/test/src/string/strrchr_test.cpp
index ba901cc8dc79e..02659c010ada1 100644
--- a/libc/test/src/string/strrchr_test.cpp
+++ b/libc/test/src/string/strrchr_test.cpp
@@ -6,75 +6,9 @@
//
//===----------------------------------------------------------------------===//
+#include "StrchrTest.h"
+
#include "src/string/strrchr.h"
#include "test/UnitTest/Test.h"
-TEST(LlvmLibcStrRChrTest, FindsFirstCharacter) {
- const char *src = "abcde";
-
- // Should return original string since 'a' is the first character.
- ASSERT_STREQ(__llvm_libc::strrchr(src, 'a'), "abcde");
- // Source string should not change.
- ASSERT_STREQ(src, "abcde");
-}
-
-TEST(LlvmLibcStrRChrTest, FindsMiddleCharacter) {
- const char *src = "abcde";
-
- // Should return characters after (and including) 'c'.
- ASSERT_STREQ(__llvm_libc::strrchr(src, 'c'), "cde");
- // Source string should not change.
- ASSERT_STREQ(src, "abcde");
-}
-
-TEST(LlvmLibcStrRChrTest, FindsLastCharacterThatIsNotNullTerminator) {
- const char *src = "abcde";
-
- // Should return 'e' and null-terminator.
- ASSERT_STREQ(__llvm_libc::strrchr(src, 'e'), "e");
- // Source string should not change.
- ASSERT_STREQ(src, "abcde");
-}
-
-TEST(LlvmLibcStrRChrTest, FindsNullTerminator) {
- const char *src = "abcde";
-
- // Should return null terminator.
- ASSERT_STREQ(__llvm_libc::strrchr(src, '\0'), "");
- // Source string should not change.
- ASSERT_STREQ(src, "abcde");
-}
-
-TEST(LlvmLibcStrRChrTest, FindsLastBehindFirstNullTerminator) {
- const char src[6] = {'a', 'a', '\0', 'b', '\0', 'c'};
- // 'b' is behind a null terminator, so should not be found.
- ASSERT_STREQ(__llvm_libc::strrchr(src, 'b'), nullptr);
- // Same goes for 'c'.
- ASSERT_STREQ(__llvm_libc::strrchr(src, 'c'), nullptr);
-
- // Should find the second of the two a's.
- ASSERT_STREQ(__llvm_libc::strrchr(src, 'a'), "a");
-}
-
-TEST(LlvmLibcStrRChrTest, CharacterNotWithinStringShouldReturnNullptr) {
- // Since 'z' is not within the string, should return nullptr.
- ASSERT_STREQ(__llvm_libc::strrchr("123?", 'z'), nullptr);
-}
-
-TEST(LlvmLibcStrRChrTest, ShouldFindLastOfDuplicates) {
- // '1' is duplicated in the string, but it should find the last copy.
- ASSERT_STREQ(__llvm_libc::strrchr("abc1def1ghi", '1'), "1ghi");
-
- const char *dups = "XXXXX";
- // Should return the last occurrence of 'X'.
- ASSERT_STREQ(__llvm_libc::strrchr(dups, 'X'), "X");
-}
-
-TEST(LlvmLibcStrRChrTest, EmptyStringShouldOnlyMatchNullTerminator) {
- // Null terminator should match.
- ASSERT_STREQ(__llvm_libc::strrchr("", '\0'), "");
- // All other characters should not match.
- ASSERT_STREQ(__llvm_libc::strrchr("", 'A'), nullptr);
- ASSERT_STREQ(__llvm_libc::strrchr("", '2'), nullptr);
- ASSERT_STREQ(__llvm_libc::strrchr("", '*'), nullptr);
-}
+STRRCHR_TEST(Strrchr, __llvm_libc::strrchr)
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/string/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/string/BUILD.bazel
index 72b75a3975026..90ae8bba97171 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/string/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/string/BUILD.bazel
@@ -42,12 +42,19 @@ libc_test(
],
)
+cc_library(
+ name = "strchr_test_helper",
+ hdrs = ["StrchrTest.h"],
+ deps = ["//libc/test/UnitTest:LibcUnitTest"],
+)
+
libc_test(
name = "strchr_test",
srcs = ["strchr_test.cpp"],
libc_function_deps = [
"//libc:strchr",
],
+ deps = [":strchr_test_helper"],
)
libc_test(
@@ -80,6 +87,7 @@ libc_test(
libc_function_deps = [
"//libc:strrchr",
],
+ deps = [":strchr_test_helper"],
)
libc_test(
More information about the libc-commits
mailing list