Skip to content

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 <ar.ashouri999@gmail.com>

@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