[libc-commits] [libc] [libc] Fix multiply-defined global functions in ctype tests (PR #119055)

Fri Dec 6 22:51:24 PST 2024

llvmbot wrote:




@llvm/pr-subscribers-libc

Author: Roland McGrath (frobtech)

<details>
<summary>Changes</summary>

For whatever reason, each ctype test contains its own copy of
some identical helper source code.  These local helpers were
defined with external linkage for no apparent reason.  This leads
to multiple definition errors when linking these tests together.

This change moves each file's local helper code into an anonymous
namespace so it has internal linkage.  It's notable that the libc
test code does not follow the most common norm of gtest-style
code where all the `TEST(...)` cases themselves are defined
inside an anonymous namespace (along with whatever other local
helpers they use); whether libc's tests should follow that usual
convention can be addressed holistically in future discussion.

The replacement of numerous cut&paste'd copies of identical
helper code with sharing the source code in some usual fashion is
also left for later cleanup.

This change only makes the test code not straightforwardly have
multiple definition errors that prevent linking a test executable
at all.


---
Full diff: https://github.com/llvm/llvm-project/pull/119055.diff


8 Files Affected:

- (modified) libc/test/src/ctype/isalnum_test.cpp (+14-10) 
- (modified) libc/test/src/ctype/isalpha_test.cpp (+14-10) 
- (modified) libc/test/src/ctype/isdigit_test.cpp (+14-10) 
- (modified) libc/test/src/ctype/islower_test.cpp (+14-10) 
- (modified) libc/test/src/ctype/isupper_test.cpp (+14-10) 
- (modified) libc/test/src/ctype/isxdigit_test.cpp (+15-11) 
- (modified) libc/test/src/ctype/tolower_test.cpp (+14-10) 
- (modified) libc/test/src/ctype/toupper_test.cpp (+14-10) 


``````````diff

diff --git a/libc/test/src/ctype/isalnum_test.cpp b/libc/test/src/ctype/isalnum_test.cpp
index 18ddd2b14b8c88..cfc54e95e922d0 100644
--- a/libc/test/src/ctype/isalnum_test.cpp
+++ b/libc/test/src/ctype/isalnum_test.cpp
@@ -11,16 +11,7 @@
 
 #include "test/UnitTest/Test.h"
 
-TEST(LlvmLibcIsAlNum, SimpleTest) {
-  EXPECT_NE(LIBC_NAMESPACE::isalnum('a'), 0);
-  EXPECT_NE(LIBC_NAMESPACE::isalnum('B'), 0);
-  EXPECT_NE(LIBC_NAMESPACE::isalnum('3'), 0);
-
-  EXPECT_EQ(LIBC_NAMESPACE::isalnum(' '), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isalnum('?'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isalnum('\0'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isalnum(-1), 0);
-}
+namespace {
 
 // TODO: Merge the ctype tests using this framework.
 constexpr char ALNUM_ARRAY[] = {
@@ -38,6 +29,19 @@ bool in_span(int ch, LIBC_NAMESPACE::cpp::span<const char> arr) {
   return false;
 }
 
+} // namespace
+
+TEST(LlvmLibcIsAlNum, SimpleTest) {
+  EXPECT_NE(LIBC_NAMESPACE::isalnum('a'), 0);
+  EXPECT_NE(LIBC_NAMESPACE::isalnum('B'), 0);
+  EXPECT_NE(LIBC_NAMESPACE::isalnum('3'), 0);
+
+  EXPECT_EQ(LIBC_NAMESPACE::isalnum(' '), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isalnum('?'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isalnum('\0'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isalnum(-1), 0);
+}
+
 TEST(LlvmLibcIsAlNum, DefaultLocale) {
   // Loops through all characters, verifying that numbers and letters
   // return non-zero integer and everything else returns a zero.
diff --git a/libc/test/src/ctype/isalpha_test.cpp b/libc/test/src/ctype/isalpha_test.cpp
index e54b580dbe264e..1e439cf973e35d 100644
--- a/libc/test/src/ctype/isalpha_test.cpp
+++ b/libc/test/src/ctype/isalpha_test.cpp
@@ -11,16 +11,7 @@
 
 #include "test/UnitTest/Test.h"
 
-TEST(LlvmLibcIsAlpha, SimpleTest) {
-  EXPECT_NE(LIBC_NAMESPACE::isalpha('a'), 0);
-  EXPECT_NE(LIBC_NAMESPACE::isalpha('B'), 0);
-
-  EXPECT_EQ(LIBC_NAMESPACE::isalpha('3'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isalpha(' '), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isalpha('?'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isalpha('\0'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isalpha(-1), 0);
-}
+namespace {
 
 // TODO: Merge the ctype tests using this framework.
 constexpr char ALPHA_ARRAY[] = {
@@ -37,6 +28,19 @@ bool in_span(int ch, LIBC_NAMESPACE::cpp::span<const char> arr) {
   return false;
 }
 
+} // namespace
+
+TEST(LlvmLibcIsAlpha, SimpleTest) {
+  EXPECT_NE(LIBC_NAMESPACE::isalpha('a'), 0);
+  EXPECT_NE(LIBC_NAMESPACE::isalpha('B'), 0);
+
+  EXPECT_EQ(LIBC_NAMESPACE::isalpha('3'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isalpha(' '), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isalpha('?'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isalpha('\0'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isalpha(-1), 0);
+}
+
 TEST(LlvmLibcIsAlpha, DefaultLocale) {
   // Loops through all characters, verifying that letters return a
   // non-zero integer and everything else returns zero.
diff --git a/libc/test/src/ctype/isdigit_test.cpp b/libc/test/src/ctype/isdigit_test.cpp
index adea55e59c74df..0ee132d7f85151 100644
--- a/libc/test/src/ctype/isdigit_test.cpp
+++ b/libc/test/src/ctype/isdigit_test.cpp
@@ -11,16 +11,7 @@
 
 #include "test/UnitTest/Test.h"
 
-TEST(LlvmLibcIsDigit, SimpleTest) {
-  EXPECT_NE(LIBC_NAMESPACE::isdigit('3'), 0);
-
-  EXPECT_EQ(LIBC_NAMESPACE::isdigit('a'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isdigit('B'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isdigit(' '), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isdigit('?'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isdigit('\0'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isdigit(-1), 0);
-}
+namespace {
 
 // TODO: Merge the ctype tests using this framework.
 constexpr char DIGIT_ARRAY[] = {
@@ -34,6 +25,19 @@ bool in_span(int ch, LIBC_NAMESPACE::cpp::span<const char> arr) {
   return false;
 }
 
+} // namespace
+
+TEST(LlvmLibcIsDigit, SimpleTest) {
+  EXPECT_NE(LIBC_NAMESPACE::isdigit('3'), 0);
+
+  EXPECT_EQ(LIBC_NAMESPACE::isdigit('a'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isdigit('B'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isdigit(' '), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isdigit('?'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isdigit('\0'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isdigit(-1), 0);
+}
+
 TEST(LlvmLibcIsDigit, DefaultLocale) {
   // Loops through all characters, verifying that numbers and letters
   // return non-zero integer and everything else returns a zero.
diff --git a/libc/test/src/ctype/islower_test.cpp b/libc/test/src/ctype/islower_test.cpp
index f9414bd8cbd095..f877171abb9a3d 100644
--- a/libc/test/src/ctype/islower_test.cpp
+++ b/libc/test/src/ctype/islower_test.cpp
@@ -11,16 +11,7 @@
 
 #include "test/UnitTest/Test.h"
 
-TEST(LlvmLibcIsLower, SimpleTest) {
-  EXPECT_NE(LIBC_NAMESPACE::islower('a'), 0);
-
-  EXPECT_EQ(LIBC_NAMESPACE::islower('B'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::islower('3'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::islower(' '), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::islower('?'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::islower('\0'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::islower(-1), 0);
-}
+namespace {
 
 // TODO: Merge the ctype tests using this framework.
 constexpr char LOWER_ARRAY[] = {
@@ -35,6 +26,19 @@ bool in_span(int ch, LIBC_NAMESPACE::cpp::span<const char> arr) {
   return false;
 }
 
+} // namespace
+
+TEST(LlvmLibcIsLower, SimpleTest) {
+  EXPECT_NE(LIBC_NAMESPACE::islower('a'), 0);
+
+  EXPECT_EQ(LIBC_NAMESPACE::islower('B'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::islower('3'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::islower(' '), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::islower('?'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::islower('\0'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::islower(-1), 0);
+}
+
 TEST(LlvmLibcIsLower, DefaultLocale) {
   // Loops through all characters, verifying that numbers and letters
   // return non-zero integer and everything else returns a zero.
diff --git a/libc/test/src/ctype/isupper_test.cpp b/libc/test/src/ctype/isupper_test.cpp
index 94def1a9dcccdb..151ed23f0ac996 100644
--- a/libc/test/src/ctype/isupper_test.cpp
+++ b/libc/test/src/ctype/isupper_test.cpp
@@ -11,16 +11,7 @@
 
 #include "test/UnitTest/Test.h"
 
-TEST(LlvmLibcIsUpper, SimpleTest) {
-  EXPECT_NE(LIBC_NAMESPACE::isupper('B'), 0);
-
-  EXPECT_EQ(LIBC_NAMESPACE::isupper('a'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isupper('3'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isupper(' '), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isupper('?'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isupper('\0'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isupper(-1), 0);
-}
+namespace {
 
 // TODO: Merge the ctype tests using this framework.
 constexpr char UPPER_ARRAY[] = {
@@ -35,6 +26,19 @@ bool in_span(int ch, LIBC_NAMESPACE::cpp::span<const char> arr) {
   return false;
 }
 
+} // namespace
+
+TEST(LlvmLibcIsUpper, SimpleTest) {
+  EXPECT_NE(LIBC_NAMESPACE::isupper('B'), 0);
+
+  EXPECT_EQ(LIBC_NAMESPACE::isupper('a'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isupper('3'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isupper(' '), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isupper('?'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isupper('\0'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isupper(-1), 0);
+}
+
 TEST(LlvmLibcIsUpper, DefaultLocale) {
   // Loops through all characters, verifying that numbers and letters
   // return non-zero integer and everything else returns a zero.
diff --git a/libc/test/src/ctype/isxdigit_test.cpp b/libc/test/src/ctype/isxdigit_test.cpp
index d7253d549907bf..ec58f0da49223c 100644
--- a/libc/test/src/ctype/isxdigit_test.cpp
+++ b/libc/test/src/ctype/isxdigit_test.cpp
@@ -11,17 +11,7 @@
 
 #include "test/UnitTest/Test.h"
 
-TEST(LlvmLibcIsXdigit, SimpleTest) {
-  EXPECT_NE(LIBC_NAMESPACE::isxdigit('a'), 0);
-  EXPECT_NE(LIBC_NAMESPACE::isxdigit('B'), 0);
-  EXPECT_NE(LIBC_NAMESPACE::isxdigit('3'), 0);
-
-  EXPECT_EQ(LIBC_NAMESPACE::isxdigit('z'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isxdigit(' '), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isxdigit('?'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isxdigit('\0'), 0);
-  EXPECT_EQ(LIBC_NAMESPACE::isxdigit(-1), 0);
-}
+namespace {
 
 // TODO: Merge the ctype tests using this framework.
 constexpr char XDIGIT_ARRAY[] = {
@@ -36,6 +26,20 @@ bool in_span(int ch, LIBC_NAMESPACE::cpp::span<const char> arr) {
   return false;
 }
 
+} // namespace
+
+TEST(LlvmLibcIsXdigit, SimpleTest) {
+  EXPECT_NE(LIBC_NAMESPACE::isxdigit('a'), 0);
+  EXPECT_NE(LIBC_NAMESPACE::isxdigit('B'), 0);
+  EXPECT_NE(LIBC_NAMESPACE::isxdigit('3'), 0);
+
+  EXPECT_EQ(LIBC_NAMESPACE::isxdigit('z'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isxdigit(' '), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isxdigit('?'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isxdigit('\0'), 0);
+  EXPECT_EQ(LIBC_NAMESPACE::isxdigit(-1), 0);
+}
+
 TEST(LlvmLibcIsXdigit, DefaultLocale) {
   // Loops through all characters, verifying that numbers and letters
   // return non-zero integer and everything else returns a zero.
diff --git a/libc/test/src/ctype/tolower_test.cpp b/libc/test/src/ctype/tolower_test.cpp
index 59432c43297b35..fbcc5b8ee0d9d5 100644
--- a/libc/test/src/ctype/tolower_test.cpp
+++ b/libc/test/src/ctype/tolower_test.cpp
@@ -11,16 +11,7 @@
 
 #include "test/UnitTest/Test.h"
 
-TEST(LlvmLibcToLower, SimpleTest) {
-  EXPECT_EQ(LIBC_NAMESPACE::tolower('a'), int('a'));
-  EXPECT_EQ(LIBC_NAMESPACE::tolower('B'), int('b'));
-  EXPECT_EQ(LIBC_NAMESPACE::tolower('3'), int('3'));
-
-  EXPECT_EQ(LIBC_NAMESPACE::tolower(' '), int(' '));
-  EXPECT_EQ(LIBC_NAMESPACE::tolower('?'), int('?'));
-  EXPECT_EQ(LIBC_NAMESPACE::tolower('\0'), int('\0'));
-  EXPECT_EQ(LIBC_NAMESPACE::tolower(-1), int(-1));
-}
+namespace {
 
 // TODO: Merge the ctype tests using this framework.
 // Invariant: UPPER_ARR and LOWER_ARR are both the complete alphabet in the same
@@ -45,6 +36,19 @@ int span_index(int ch, LIBC_NAMESPACE::cpp::span<const char> arr) {
   return -1;
 }
 
+} // namespace
+
+TEST(LlvmLibcToLower, SimpleTest) {
+  EXPECT_EQ(LIBC_NAMESPACE::tolower('a'), int('a'));
+  EXPECT_EQ(LIBC_NAMESPACE::tolower('B'), int('b'));
+  EXPECT_EQ(LIBC_NAMESPACE::tolower('3'), int('3'));
+
+  EXPECT_EQ(LIBC_NAMESPACE::tolower(' '), int(' '));
+  EXPECT_EQ(LIBC_NAMESPACE::tolower('?'), int('?'));
+  EXPECT_EQ(LIBC_NAMESPACE::tolower('\0'), int('\0'));
+  EXPECT_EQ(LIBC_NAMESPACE::tolower(-1), int(-1));
+}
+
 TEST(LlvmLibcToLower, DefaultLocale) {
   for (int ch = -255; ch < 255; ++ch) {
     int char_index = span_index(ch, UPPER_ARR);
diff --git a/libc/test/src/ctype/toupper_test.cpp b/libc/test/src/ctype/toupper_test.cpp
index 045b00bbb4b935..409b3cd96ed1ea 100644
--- a/libc/test/src/ctype/toupper_test.cpp
+++ b/libc/test/src/ctype/toupper_test.cpp
@@ -11,16 +11,7 @@
 
 #include "test/UnitTest/Test.h"
 
-TEST(LlvmLibcToUpper, SimpleTest) {
-  EXPECT_EQ(LIBC_NAMESPACE::toupper('a'), int('A'));
-  EXPECT_EQ(LIBC_NAMESPACE::toupper('B'), int('B'));
-  EXPECT_EQ(LIBC_NAMESPACE::toupper('3'), int('3'));
-
-  EXPECT_EQ(LIBC_NAMESPACE::toupper(' '), int(' '));
-  EXPECT_EQ(LIBC_NAMESPACE::toupper('?'), int('?'));
-  EXPECT_EQ(LIBC_NAMESPACE::toupper('\0'), int('\0'));
-  EXPECT_EQ(LIBC_NAMESPACE::toupper(-1), int(-1));
-}
+namespace {
 
 // TODO: Merge the ctype tests using this framework.
 // Invariant: UPPER_ARR and LOWER_ARR are both the complete alphabet in the same
@@ -45,6 +36,19 @@ int span_index(int ch, LIBC_NAMESPACE::cpp::span<const char> arr) {
   return -1;
 }
 
+} // namespace
+
+TEST(LlvmLibcToUpper, SimpleTest) {
+  EXPECT_EQ(LIBC_NAMESPACE::toupper('a'), int('A'));
+  EXPECT_EQ(LIBC_NAMESPACE::toupper('B'), int('B'));
+  EXPECT_EQ(LIBC_NAMESPACE::toupper('3'), int('3'));
+
+  EXPECT_EQ(LIBC_NAMESPACE::toupper(' '), int(' '));
+  EXPECT_EQ(LIBC_NAMESPACE::toupper('?'), int('?'));
+  EXPECT_EQ(LIBC_NAMESPACE::toupper('\0'), int('\0'));
+  EXPECT_EQ(LIBC_NAMESPACE::toupper(-1), int(-1));
+}
+
 TEST(LlvmLibcToUpper, DefaultLocale) {
   for (int ch = -255; ch < 255; ++ch) {
     int char_index = span_index(ch, LOWER_ARR);

``````````

</details>


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