Skip to content

[libc++][NFC] Simplify the implementation of string and string_views operator== #117184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 13, 2024
Merged

[libc++][NFC] Simplify the implementation of string and string_views operator== #117184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 8 additions & 32 deletions libcxx/include/string
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1881,11 +1881,6 @@ public:
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __clear_and_shrink() _NOEXCEPT;

private:
template <class _Alloc>
inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool friend
operator==(const basic_string<char, char_traits<char>, _Alloc>& __lhs,
const basic_string<char, char_traits<char>, _Alloc>& __rhs) _NOEXCEPT;

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __shrink_or_extend(size_type __target_capacity);

_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_STRING_INTERNAL_MEMORY_ACCESS bool
Expand Down Expand Up @@ -3842,36 +3837,9 @@ template <class _CharT, class _Traits, class _Allocator>
inline _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool
operator==(const basic_string<_CharT, _Traits, _Allocator>& __lhs,
const basic_string<_CharT, _Traits, _Allocator>& __rhs) _NOEXCEPT {
# if _LIBCPP_STD_VER >= 20
return basic_string_view<_CharT, _Traits>(__lhs) == basic_string_view<_CharT, _Traits>(__rhs);
# else
size_t __lhs_sz = __lhs.size();
return __lhs_sz == __rhs.size() && _Traits::compare(__lhs.data(), __rhs.data(), __lhs_sz) == 0;
# endif
}

template <class _Allocator>
inline _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool
operator==(const basic_string<char, char_traits<char>, _Allocator>& __lhs,
const basic_string<char, char_traits<char>, _Allocator>& __rhs) _NOEXCEPT {
size_t __sz = __lhs.size();
if (__sz != __rhs.size())
return false;
return char_traits<char>::compare(__lhs.data(), __rhs.data(), __sz) == 0;
}

# if _LIBCPP_STD_VER <= 17
template <class _CharT, class _Traits, class _Allocator>
inline _LIBCPP_HIDE_FROM_ABI bool
operator==(const _CharT* __lhs, const basic_string<_CharT, _Traits, _Allocator>& __rhs) _NOEXCEPT {
typedef basic_string<_CharT, _Traits, _Allocator> _String;
_LIBCPP_ASSERT_NON_NULL(__lhs != nullptr, "operator==(char*, basic_string): received nullptr");
size_t __lhs_len = _Traits::length(__lhs);
if (__lhs_len != __rhs.size())
return false;
return __rhs.compare(0, _String::npos, __lhs, __lhs_len) == 0;
}
# endif // _LIBCPP_STD_VER <= 17

template <class _CharT, class _Traits, class _Allocator>
inline _LIBCPP_CONSTEXPR_SINCE_CXX20 _LIBCPP_HIDE_FROM_ABI bool
Expand All @@ -3890,6 +3858,14 @@ operator==(const basic_string<_CharT, _Traits, _Allocator>& __lhs, const _CharT*
return __lhs.compare(0, _String::npos, __rhs, __rhs_len) == 0;
}

# if _LIBCPP_STD_VER <= 17
template <class _CharT, class _Traits, class _Allocator>
inline _LIBCPP_HIDE_FROM_ABI bool
operator==(const _CharT* __lhs, const basic_string<_CharT, _Traits, _Allocator>& __rhs) _NOEXCEPT {
return __rhs == __lhs;
}
# endif // _LIBCPP_STD_VER <= 17

# if _LIBCPP_STD_VER >= 20

template <class _CharT, class _Traits, class _Allocator>
Expand Down
40 changes: 12 additions & 28 deletions libcxx/include/string_view
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -722,16 +722,19 @@ basic_string_view(_Range) -> basic_string_view<ranges::range_value_t<_Range>>;

// [string.view.comparison]

# if _LIBCPP_STD_VER >= 20

template <class _CharT, class _Traits>
_LIBCPP_HIDE_FROM_ABI constexpr bool operator==(basic_string_view<_CharT, _Traits> __lhs,
type_identity_t<basic_string_view<_CharT, _Traits>> __rhs) noexcept {
// The dummy default template parameters are used to work around a MSVC issue with mangling, see VSO-409326 for details.
// This applies to the other sufficient overloads below for the other comparison operators.
template <class _CharT, class _Traits, int = 1>
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI bool
operator==(basic_string_view<_CharT, _Traits> __lhs,
__type_identity_t<basic_string_view<_CharT, _Traits> > __rhs) _NOEXCEPT {
if (__lhs.size() != __rhs.size())
return false;
return __lhs.compare(__rhs) == 0;
}

# if _LIBCPP_STD_VER >= 20

template <class _CharT, class _Traits>
_LIBCPP_HIDE_FROM_ABI constexpr auto operator<=>(basic_string_view<_CharT, _Traits> __lhs,
type_identity_t<basic_string_view<_CharT, _Traits>> __rhs) noexcept {
Expand All @@ -757,51 +760,32 @@ operator==(basic_string_view<_CharT, _Traits> __lhs, basic_string_view<_CharT, _
return __lhs.compare(__rhs) == 0;
}

// The dummy default template parameters are used to work around a MSVC issue with mangling, see VSO-409326 for details.
// This applies to the other sufficient overloads below for the other comparison operators.
template <class _CharT, class _Traits, int = 1>
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI bool
operator==(basic_string_view<_CharT, _Traits> __lhs,
__type_identity_t<basic_string_view<_CharT, _Traits> > __rhs) _NOEXCEPT {
if (__lhs.size() != __rhs.size())
return false;
return __lhs.compare(__rhs) == 0;
}

template <class _CharT, class _Traits, int = 2>
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI bool
operator==(__type_identity_t<basic_string_view<_CharT, _Traits> > __lhs,
basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT {
if (__lhs.size() != __rhs.size())
return false;
return __lhs.compare(__rhs) == 0;
return __lhs == __rhs;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not recursive? What did I miss?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This calls the operator==(basic_string_view<_CharT, _Traits> __lhs, basic_string_view<_CharT, _Traits> __rhs).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must be missing the point of the __type_identity argument here. It seems to be central to my misunderstanding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The __type_identity is to allow types that are convertible to string_view to be compared to a string_view.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But how are these overloads not ambiguous with the one that takes two basic_string_views when you pass actual string views (that match both overloads)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gentle ping.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that if you have more exact matches for the template parameter that is preferred over an argument that happens to have the same type. Consider e.g.

template <class T, class U>
void func(T, U);

template <class T>
void func(T, T);

Here the second overload is preferred, since it requires that the two arguments match exactly. I'm not 100% sure what the rule here is though.

}

// operator !=
template <class _CharT, class _Traits>
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI bool
operator!=(basic_string_view<_CharT, _Traits> __lhs, basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT {
if (__lhs.size() != __rhs.size())
return true;
return __lhs.compare(__rhs) != 0;
return !(__lhs == __rhs);
}

template <class _CharT, class _Traits, int = 1>
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI bool
operator!=(basic_string_view<_CharT, _Traits> __lhs,
__type_identity_t<basic_string_view<_CharT, _Traits> > __rhs) _NOEXCEPT {
if (__lhs.size() != __rhs.size())
return true;
return __lhs.compare(__rhs) != 0;
return !(__lhs == __rhs);
}

template <class _CharT, class _Traits, int = 2>
_LIBCPP_CONSTEXPR_SINCE_CXX14 _LIBCPP_HIDE_FROM_ABI bool
operator!=(__type_identity_t<basic_string_view<_CharT, _Traits> > __lhs,
basic_string_view<_CharT, _Traits> __rhs) _NOEXCEPT {
if (__lhs.size() != __rhs.size())
return true;
return __lhs.compare(__rhs) != 0;
return !(__lhs == __rhs);
}

// operator <
Expand Down
Loading