From f01048743ab300595c265a74ec31858f0cc59267 Mon Sep 17 00:00:00 2001 From: Ryan Prichard Date: Fri, 10 May 2024 01:37:09 -0700 Subject: [PATCH] Define String8 and String16 operator<=> Previously, in C++20 mode, using <=> on String8 or String16 would compare the pointer values from the implicit conversion operators returning const char* and const char16_t*. Instead, compare the string content. This is especially a problem with STL containers that prefer <=> when it is available. See https://godbolt.org/z/cc1hW17h3 for a demo of the problem. Bug: 339775405 Test: treehugger Change-Id: I5ee6af96dee0c9968a1bab1ad96729e45bb1ac50 --- libutils/binder/String16_test.cpp | 44 +++++++++++++++++++++++ libutils/binder/String8_test.cpp | 46 +++++++++++++++++++++++- libutils/binder/include/utils/String16.h | 36 +++++++++++++++++++ libutils/binder/include/utils/String8.h | 36 +++++++++++++++++++ 4 files changed, 161 insertions(+), 1 deletion(-) diff --git a/libutils/binder/String16_test.cpp b/libutils/binder/String16_test.cpp index 6f4642e8f..83cc5990b 100644 --- a/libutils/binder/String16_test.cpp +++ b/libutils/binder/String16_test.cpp @@ -16,6 +16,8 @@ #include #include +#include +#include #include @@ -257,3 +259,45 @@ TEST(String16Test, insert) { EXPECT_EQ(NO_MEMORY, s.insert(3, u"", SIZE_MAX)); EXPECT_STR16EQ(u"foo!bar", s.c_str()); } + +TEST(String16Test, comparisons) { + const char16_t* cstr1 = u"abc"; + const char16_t* cstr2 = u"def"; + + // str1 and str1b will point to different blocks of memory but with equal contents. + String16 str1(cstr1); + String16 str1b(cstr1); + String16 str2(cstr2); + + EXPECT_TRUE((str1 <=> str1b) == 0); + EXPECT_FALSE(str1 != str1b); + EXPECT_FALSE(str1 < str1b); + EXPECT_TRUE(str1 <= str1b); + EXPECT_TRUE(str1 == str1b); + EXPECT_TRUE(str1 >= str1b); + EXPECT_FALSE(str1 > str1b); + + EXPECT_TRUE((str1 <=> str2) < 0); + EXPECT_TRUE((str2 <=> str1) > 0); + EXPECT_TRUE(str1 != str2); + EXPECT_TRUE(str1 < str2); + EXPECT_TRUE(str1 <= str2); + EXPECT_FALSE(str1 == str2); + EXPECT_FALSE(str1 >= str2); + EXPECT_FALSE(str1 > str2); + + // Verify that pre-C++20 comparison operators work with a std::pair of a String8, which only + // provides <=> in C++20 and up. See b/339775405. + + std::pair pair1(str1, 13); + std::pair pair1b(str1b, 13); + std::pair pair2(str2, 13); + + EXPECT_TRUE(pair1 == pair1b); + EXPECT_FALSE(pair1 < pair1b); + EXPECT_FALSE(pair1 > pair1b); + + EXPECT_TRUE(pair1 != pair2); + EXPECT_TRUE(pair1 < pair2); + EXPECT_FALSE(pair1 > pair2); +} diff --git a/libutils/binder/String8_test.cpp b/libutils/binder/String8_test.cpp index 6f7882a38..fc3c32941 100644 --- a/libutils/binder/String8_test.cpp +++ b/libutils/binder/String8_test.cpp @@ -17,8 +17,10 @@ #define LOG_TAG "String8_test" #include -#include #include +#include +#include +#include #include @@ -132,3 +134,45 @@ TEST_F(String8Test, removeAll) { EXPECT_TRUE(s.removeAll("o")); EXPECT_STREQ("Hell, wrld!", s.c_str()); } + +TEST_F(String8Test, comparisons) { + const char* cstr1 = "abc"; + const char* cstr2 = "def"; + + // str1 and str1b will point to different blocks of memory but with equal contents. + String8 str1(cstr1); + String8 str1b(cstr1); + String8 str2(cstr2); + + EXPECT_TRUE((str1 <=> str1b) == 0); + EXPECT_FALSE(str1 != str1b); + EXPECT_FALSE(str1 < str1b); + EXPECT_TRUE(str1 <= str1b); + EXPECT_TRUE(str1 == str1b); + EXPECT_TRUE(str1 >= str1b); + EXPECT_FALSE(str1 > str1b); + + EXPECT_TRUE((str1 <=> str2) < 0); + EXPECT_TRUE((str2 <=> str1) > 0); + EXPECT_TRUE(str1 != str2); + EXPECT_TRUE(str1 < str2); + EXPECT_TRUE(str1 <= str2); + EXPECT_FALSE(str1 == str2); + EXPECT_FALSE(str1 >= str2); + EXPECT_FALSE(str1 > str2); + + // Verify that pre-C++20 comparison operators work with a std::pair of a String8, which only + // provides <=> in C++20 and up. See b/339775405. + + std::pair pair1(str1, 13); + std::pair pair1b(str1b, 13); + std::pair pair2(str2, 13); + + EXPECT_TRUE(pair1 == pair1b); + EXPECT_FALSE(pair1 < pair1b); + EXPECT_FALSE(pair1 > pair1b); + + EXPECT_TRUE(pair1 != pair2); + EXPECT_TRUE(pair1 < pair2); + EXPECT_FALSE(pair1 > pair2); +} diff --git a/libutils/binder/include/utils/String16.h b/libutils/binder/include/utils/String16.h index c7135766b..867dbac34 100644 --- a/libutils/binder/include/utils/String16.h +++ b/libutils/binder/include/utils/String16.h @@ -29,6 +29,10 @@ #define HAS_STRING_VIEW #endif +#if __cplusplus >= 202002L +#include +#endif + // --------------------------------------------------------------------------- namespace android { @@ -105,6 +109,9 @@ public: inline bool operator!=(const String16& other) const; inline bool operator>=(const String16& other) const; inline bool operator>(const String16& other) const; +#if __cplusplus >= 202002L + inline std::strong_ordering operator<=>(const String16& other) const; +#endif inline bool operator<(const char16_t* other) const; inline bool operator<=(const char16_t* other) const; @@ -112,6 +119,9 @@ public: inline bool operator!=(const char16_t* other) const; inline bool operator>=(const char16_t* other) const; inline bool operator>(const char16_t* other) const; +#if __cplusplus >= 202002L + inline std::strong_ordering operator<=>(const char16_t* other) const; +#endif inline operator const char16_t*() const; @@ -334,6 +344,19 @@ inline bool String16::operator>(const String16& other) const return strzcmp16(mString, size(), other.mString, other.size()) > 0; } +#if __cplusplus >= 202002L +inline std::strong_ordering String16::operator<=>(const String16& other) const { + int result = strzcmp16(mString, size(), other.mString, other.size()); + if (result == 0) { + return std::strong_ordering::equal; + } else if (result < 0) { + return std::strong_ordering::less; + } else { + return std::strong_ordering::greater; + } +} +#endif + inline bool String16::operator<(const char16_t* other) const { return strcmp16(mString, other) < 0; @@ -364,6 +387,19 @@ inline bool String16::operator>(const char16_t* other) const return strcmp16(mString, other) > 0; } +#if __cplusplus >= 202002L +inline std::strong_ordering String16::operator<=>(const char16_t* other) const { + int result = strcmp16(mString, other); + if (result == 0) { + return std::strong_ordering::equal; + } else if (result < 0) { + return std::strong_ordering::less; + } else { + return std::strong_ordering::greater; + } +} +#endif + inline String16::operator const char16_t*() const { return mString; diff --git a/libutils/binder/include/utils/String8.h b/libutils/binder/include/utils/String8.h index 6d250723b..e0d7588f6 100644 --- a/libutils/binder/include/utils/String8.h +++ b/libutils/binder/include/utils/String8.h @@ -36,6 +36,10 @@ #define HAS_STRING_VIEW #endif +#if __cplusplus >= 202002L +#include +#endif + // --------------------------------------------------------------------------- namespace android { @@ -106,6 +110,9 @@ public: inline bool operator!=(const String8& other) const; inline bool operator>=(const String8& other) const; inline bool operator>(const String8& other) const; +#if __cplusplus >= 202002L + inline std::strong_ordering operator<=>(const String8& other) const; +#endif inline bool operator<(const char* other) const; inline bool operator<=(const char* other) const; @@ -113,6 +120,9 @@ public: inline bool operator!=(const char* other) const; inline bool operator>=(const char* other) const; inline bool operator>(const char* other) const; +#if __cplusplus >= 202002L + inline std::strong_ordering operator<=>(const char* other) const; +#endif inline operator const char*() const; @@ -302,6 +312,19 @@ inline bool String8::operator>(const String8& other) const return strcmp(mString, other.mString) > 0; } +#if __cplusplus >= 202002L +inline std::strong_ordering String8::operator<=>(const String8& other) const { + int result = strcmp(mString, other.mString); + if (result == 0) { + return std::strong_ordering::equal; + } else if (result < 0) { + return std::strong_ordering::less; + } else { + return std::strong_ordering::greater; + } +} +#endif + inline bool String8::operator<(const char* other) const { return strcmp(mString, other) < 0; @@ -332,6 +355,19 @@ inline bool String8::operator>(const char* other) const return strcmp(mString, other) > 0; } +#if __cplusplus >= 202002L +inline std::strong_ordering String8::operator<=>(const char* other) const { + int result = strcmp(mString, other); + if (result == 0) { + return std::strong_ordering::equal; + } else if (result < 0) { + return std::strong_ordering::less; + } else { + return std::strong_ordering::greater; + } +} +#endif + inline String8::operator const char*() const { return mString;