-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[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
Conversation
@llvm/pr-subscribers-libcxx ChangesTemplate argument Full diff: https://github.com/llvm/llvm-project/pull/67733.diff 1 Files Affected:
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));
}
|
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 |
This should be fixed on |
There was a problem hiding this 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.
There was a problem hiding this 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)
@philnik777 The CI is green with respect to clang-format. Can you help me land this? |
You have to remove |
Thank you! I've fixed that. |
Template argument `_Unwrapped` is always deduced from the type of `_Unwrapped __iter`.
Template argument
_Unwrapped
is always deduced from the type of_Unwrapped __iter
.