Skip to content

[libc++] Remove unused defaulted template arg from __rewrap_range. #67733

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

Merged
merged 1 commit into from
Oct 4, 2023

Conversation

AMP999
Copy link
Contributor

@AMP999 AMP999 commented Sep 28, 2023

Template argument _Unwrapped is always deduced from the type of _Unwrapped __iter.

@AMP999 AMP999 requested a review from a team as a code owner September 28, 2023 20:33
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 28, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 28, 2023

@llvm/pr-subscribers-libcxx

Changes

Template argument _Unwrapped is always deduced from the type of _Unwrapped __iter.


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

1 Files Affected:

  • (modified) libcxx/include/__algorithm/unwrap_range.h (+2-2)
diff --git a/libcxx/include/__algorithm/unwrap_range.h b/libcxx/include/__algorithm/unwrap_range.h
index 2c75c8f49de938e..a1e0989570bdb2d 100644
--- a/libcxx/include/__algorithm/unwrap_range.h
+++ b/libcxx/include/__algorithm/unwrap_range.h
@@ -76,7 +76,7 @@ _LIBCPP_HIDE_FROM_ABI constexpr auto __unwrap_range(_Iter __first, _Sent __last)
 template <
     class _Sent,
     class _Iter,
-    class _Unwrapped = decltype(std::__unwrap_range(std::declval<_Iter>(), std::declval<_Sent>()))>
+    class _Unwrapped>
 _LIBCPP_HIDE_FROM_ABI constexpr _Iter __rewrap_range(_Iter __orig_iter, _Unwrapped __iter) {
   return __unwrap_range_impl<_Iter, _Sent>::__rewrap(std::move(__orig_iter), std::move(__iter));
 }
@@ -86,7 +86,7 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR pair<_Unwrapped, _Unwrapped> __unwrap_ra
   return std::make_pair(std::__unwrap_iter(std::move(__first)), std::__unwrap_iter(std::move(__last)));
 }
 
-template <class _Iter, class _Unwrapped = decltype(std::__unwrap_iter(std::declval<_Iter>()))>
+template <class _Iter, class _Unwrapped>
 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR _Iter __rewrap_range(_Iter __orig_iter, _Unwrapped __iter) {
   return std::__rewrap_iter(std::move(__orig_iter), std::move(__iter));
 }

@AMP999
Copy link
Contributor Author

AMP999 commented Sep 29, 2023

The CI is failing in https://buildkite.com/llvm-project/libcxx-ci/builds/30092#018add7f-bf00-44ec-9959-2a1ac111d99c because the benchmarks already committed to main are trying to use ranges::ends_with in C++20 mode. What should I do about that?
@var-const

@ldionne
Copy link
Member

ldionne commented Sep 29, 2023

The CI is failing in https://buildkite.com/llvm-project/libcxx-ci/builds/30092#018add7f-bf00-44ec-9959-2a1ac111d99c because the benchmarks already committed to main are trying to use ranges::ends_with in C++20 mode. What should I do about that? @var-const

This should be fixed on main now.

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

This makes sense to me, but I'd like @philnik777 to have a quick look as well to make sure this was a copy-paste error (what it looks like).

@philnik777 Did you write it that way to make it possible to deduce from something like {} passed as an argument?

@AMP999 Please wait for @philnik777 to validate before merging.

Copy link
Contributor

@philnik777 philnik777 left a comment

Choose a reason for hiding this comment

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

I think my initial intent was to ensure that we only take the unwrapped iterator. Basically, the argument should have been type_identity_t<_Unwrapped>. Given that this is already enforced by __unwrap_iter_impl, i don't think there is anything wrong with just removing the default (especially given that it's broken already).

LGTM with a clang-format run. (The file is formatted % the changed lines and a whitespace change above)

@AMP999
Copy link
Contributor Author

AMP999 commented Sep 30, 2023

@philnik777 The CI is green with respect to clang-format. Can you help me land this? Amirreza Ashouri <[email protected]>

@philnik777
Copy link
Contributor

You have to remove libcxx/include/__algorithm/unwrap_range.h from libcxx/utils/data/ignore_format.txt to check formatting.

@AMP999
Copy link
Contributor Author

AMP999 commented Oct 1, 2023

You have to remove libcxx/include/__algorithm/unwrap_range.h from libcxx/utils/data/ignore_format.txt to check formatting.

Thank you! I've fixed that.

Template argument `_Unwrapped` is always deduced from the type of `_Unwrapped __iter`.
@philnik777 philnik777 merged commit b6f6fe9 into llvm:main Oct 4, 2023
@AMP999 AMP999 deleted the bugFix branch October 4, 2023 09:25
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.

4 participants