Skip to content

[libc++] Move check for contiguous iterator to static_assert #115322

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

ldionne
Copy link
Member

@ldionne ldionne commented Nov 7, 2024

I don't think there was any reason to use enable_if to enforce that bounded_iter and static_bounded_iter are instantiated with contiguous iterators. Using a static_assert removes that detail from the ABI and allows providing a slightly cleaner error message.

Fixes #115002

I don't think there was any reason to use enable_if to enforce that
bounded_iter and static_bounded_iter are instantiated with contiguous
iterators. Using a static_assert removes that detail from the ABI and
allows providing a slightly cleaner error message.

Fixes llvm#115002
@ldionne ldionne requested a review from philnik777 November 7, 2024 14:25
@ldionne ldionne requested a review from a team as a code owner November 7, 2024 14:25
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

I don't think there was any reason to use enable_if to enforce that bounded_iter and static_bounded_iter are instantiated with contiguous iterators. Using a static_assert removes that detail from the ABI and allows providing a slightly cleaner error message.

Fixes #115002


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

2 Files Affected:

  • (modified) libcxx/include/__iterator/bounded_iter.h (+5-2)
  • (modified) libcxx/include/__iterator/static_bounded_iter.h (+4-1)
diff --git a/libcxx/include/__iterator/bounded_iter.h b/libcxx/include/__iterator/bounded_iter.h
index 4811b0dde7f777..9b8532d55a331d 100644
--- a/libcxx/include/__iterator/bounded_iter.h
+++ b/libcxx/include/__iterator/bounded_iter.h
@@ -47,8 +47,11 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 //    pointer, it is undefined at the language level (see [expr.add]). If
 //    bounded iterators exhibited this undefined behavior, we risk compiler
 //    optimizations deleting non-redundant bounds checks.
-template <class _Iterator, class = __enable_if_t< __libcpp_is_contiguous_iterator<_Iterator>::value > >
+template <class _Iterator>
 struct __bounded_iter {
+  static_assert(__libcpp_is_contiguous_iterator<_Iterator>::value,
+                "__bounded_iter<It> requires It to be a contiguous iterator.");
+
   using value_type        = typename iterator_traits<_Iterator>::value_type;
   using difference_type   = typename iterator_traits<_Iterator>::difference_type;
   using pointer           = typename iterator_traits<_Iterator>::pointer;
@@ -67,7 +70,7 @@ struct __bounded_iter {
   _LIBCPP_HIDE_FROM_ABI __bounded_iter(__bounded_iter const&) = default;
   _LIBCPP_HIDE_FROM_ABI __bounded_iter(__bounded_iter&&)      = default;
 
-  template <class _OtherIterator, __enable_if_t< is_convertible<_OtherIterator, _Iterator>::value, int> = 0>
+  template <class _OtherIterator, __enable_if_t<is_convertible<_OtherIterator, _Iterator>::value, int> = 0>
   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR __bounded_iter(__bounded_iter<_OtherIterator> const& __other) _NOEXCEPT
       : __current_(__other.__current_),
         __begin_(__other.__begin_),
diff --git a/libcxx/include/__iterator/static_bounded_iter.h b/libcxx/include/__iterator/static_bounded_iter.h
index 2b80507cf56a01..b2e9e9853673fd 100644
--- a/libcxx/include/__iterator/static_bounded_iter.h
+++ b/libcxx/include/__iterator/static_bounded_iter.h
@@ -70,8 +70,11 @@ struct __static_bounded_iter_storage<_Iterator, 0> {
 // it can be computed from the start of the range.
 //
 // The operations on which this iterator wrapper traps are the same as `__bounded_iter`.
-template <class _Iterator, size_t _Size, class = __enable_if_t<__libcpp_is_contiguous_iterator<_Iterator>::value> >
+template <class _Iterator, size_t _Size>
 struct __static_bounded_iter {
+  static_assert(__libcpp_is_contiguous_iterator<_Iterator>::value,
+                "__static_bounded_iter<It> requires It to be a contiguous iterator.");
+
   using value_type        = typename iterator_traits<_Iterator>::value_type;
   using difference_type   = typename iterator_traits<_Iterator>::difference_type;
   using pointer           = typename iterator_traits<_Iterator>::pointer;

@ldionne
Copy link
Member Author

ldionne commented Nov 7, 2024

This is redundant, I had not seen #115304 yet.

@ldionne ldionne closed this Nov 7, 2024
@ldionne ldionne deleted the review/bounded_iter-static-assert-instead-of-enable_if branch November 7, 2024 14:27
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.

[libc++] __bounded_iter should use static_assert instead of enable_if for checking contiguous_iterator
2 participants