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

Roland McGrath via libc-commits libc-commits at lists.llvm.org
Fri Dec 6 22:49:58 PST 2024


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

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.


>From fdf110475ee44d44b9ea8a2968457d75c034bcd5 Mon Sep 17 00:00:00 2001
From: Roland McGrath <mcgrathr at google.com>
Date: Fri, 6 Dec 2024 22:48:02 -0800
Subject: [PATCH] [libc] Fix multiply-defined global functions in ctype tests

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.
---
 libc/test/src/ctype/isalnum_test.cpp  | 24 ++++++++++++++----------
 libc/test/src/ctype/isalpha_test.cpp  | 24 ++++++++++++++----------
 libc/test/src/ctype/isdigit_test.cpp  | 24 ++++++++++++++----------
 libc/test/src/ctype/islower_test.cpp  | 24 ++++++++++++++----------
 libc/test/src/ctype/isupper_test.cpp  | 24 ++++++++++++++----------
 libc/test/src/ctype/isxdigit_test.cpp | 26 +++++++++++++++-----------
 libc/test/src/ctype/tolower_test.cpp  | 24 ++++++++++++++----------
 libc/test/src/ctype/toupper_test.cpp  | 24 ++++++++++++++----------
 8 files changed, 113 insertions(+), 81 deletions(-)

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);



More information about the libc-commits mailing list