Skip to content
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.

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

[NFC][libc++] Removes Clang 16 work-arounds. #91636

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mordante
Copy link
Member

@mordante mordante commented May 9, 2024

This was noticed while reviewing the implementation status of
P1614R2 The Mothership has Landed

Drive-by: Add some missing _LIBCPP_HIDE_FROM_ABIs.

@mordante mordante requested a review from a team as a code owner May 9, 2024 18:04
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label May 9, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 9, 2024

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

This was noticed while reviewing the implementation status of
P1614R2 The Mothership has Landed

Drive-by: Add some missing _LIBCPP_HIDE_FROM_ABIs.


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

2 Files Affected:

  • (modified) libcxx/include/queue (+5-2)
  • (modified) libcxx/include/stack (+9-4)
diff --git a/libcxx/include/queue b/libcxx/include/queue
index 8d6f6c667b93b..893cfb29d18e8 100644
--- a/libcxx/include/queue
+++ b/libcxx/include/queue
@@ -425,6 +425,10 @@ public:
   template <class _T1, class _OtherContainer>
   friend _LIBCPP_HIDE_FROM_ABI bool
   operator<(const queue<_T1, _OtherContainer>& __x, const queue<_T1, _OtherContainer>& __y);
+
+  template <class _T1, three_way_comparable _OtherContainer>
+  friend _LIBCPP_HIDE_FROM_ABI compare_three_way_result_t<_OtherContainer>
+  operator<=>(const queue<_T1, _OtherContainer>& __x, const queue<_T1, _OtherContainer>& __y);
 };
 
 #if _LIBCPP_STD_VER >= 17
@@ -494,8 +498,7 @@ inline _LIBCPP_HIDE_FROM_ABI bool operator<=(const queue<_Tp, _Container>& __x,
 template <class _Tp, three_way_comparable _Container>
 _LIBCPP_HIDE_FROM_ABI compare_three_way_result_t<_Container>
 operator<=>(const queue<_Tp, _Container>& __x, const queue<_Tp, _Container>& __y) {
-  // clang 16 bug: declaring `friend operator<=>` causes "use of overloaded operator '*' is ambiguous" errors
-  return __x.__get_container() <=> __y.__get_container();
+  return __x.c <=> __y.c;
 }
 
 #endif
diff --git a/libcxx/include/stack b/libcxx/include/stack
index e45bfba5587ed..6a4214b0fe555 100644
--- a/libcxx/include/stack
+++ b/libcxx/include/stack
@@ -276,10 +276,16 @@ public:
   _LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI const _Container& __get_container() const { return c; }
 
   template <class _T1, class _OtherContainer>
-  friend bool operator==(const stack<_T1, _OtherContainer>& __x, const stack<_T1, _OtherContainer>& __y);
+  friend _LIBCPP_HIDE_FROM_ABI bool
+  operator==(const stack<_T1, _OtherContainer>& __x, const stack<_T1, _OtherContainer>& __y);
 
   template <class _T1, class _OtherContainer>
-  friend bool operator<(const stack<_T1, _OtherContainer>& __x, const stack<_T1, _OtherContainer>& __y);
+  friend _LIBCPP_HIDE_FROM_ABI bool
+  operator<(const stack<_T1, _OtherContainer>& __x, const stack<_T1, _OtherContainer>& __y);
+
+  template <class _T1, three_way_comparable _OtherContainer>
+  friend _LIBCPP_HIDE_FROM_ABI compare_three_way_result_t<_OtherContainer>
+  operator<=>(const stack<_T1, _OtherContainer>& __x, const stack<_T1, _OtherContainer>& __y);
 };
 
 #if _LIBCPP_STD_VER >= 17
@@ -348,8 +354,7 @@ inline _LIBCPP_HIDE_FROM_ABI bool operator<=(const stack<_Tp, _Container>& __x,
 template <class _Tp, three_way_comparable _Container>
 _LIBCPP_HIDE_FROM_ABI compare_three_way_result_t<_Container>
 operator<=>(const stack<_Tp, _Container>& __x, const stack<_Tp, _Container>& __y) {
-  // clang 16 bug: declaring `friend operator<=>` causes "use of overloaded operator '*' is ambiguous" errors
-  return __x.__get_container() <=> __y.__get_container();
+  return __x.c <=> __y.c;
 }
 
 #endif

This was noticed while reviewing the implementation status of
  P1614R2 The Mothership has Landed

Drive-by: Add some missing _LIBCPP_HIDE_FROM_ABIs.
@mordante mordante force-pushed the review/removes_clang_16_work_arounds branch from 13b0d14 to 83486d9 Compare May 9, 2024 18:44
@@ -276,10 +276,18 @@ public:
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI const _Container& __get_container() const { return c; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_LIBCPP_NODISCARD _LIBCPP_HIDE_FROM_ABI const _Container& __get_container() const { return c; }

This is probably no longer necessary (above also). I didn't verify that claim it though.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Zingam
Copy link
Contributor

Zingam commented May 10, 2024

Until Android NDK and Xcode get a Clang 18/17 based compiler this is probably not going to work. AFAIK they use interim versions which are missing features from final LLVM releases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants