Skip to content

[libc++][test] Fix overload_compare_iterator::iterator_category #112165

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

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Oct 14, 2024

overload_compare_iterator only supports operations required for forward iterators. On the other hand, it is used for output iterators of uninitialized memory algorithms, which requires it to be forward iterator.

As a result, overload_compare_iterator<I>::iterator_category should always be std::forward_iterator_tag if we don't extend its ability. The correct iterator_category can prevent standard library implementations like MSVC STL attempting random access operations on overload_compare_iterator.

Fixes #74756.

`overload_compare_iterator` only supports operations required for
forward iterators. On the other hand, it is used for output iterators
of uninitialized memory algorithms, which requires it to be forward
iterator.

As a result, `overload_compare_iterator<I>::iterator_category` should
always be `std::forward_iterator_tag` if we don't extends its ability.
The correct `iterator_category` can prevent standard library
implementations like MSVC STL to attempt random access operations on it.
@frederick-vs-ja frederick-vs-ja requested a review from a team as a code owner October 14, 2024 08:01
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Oct 14, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 14, 2024

@llvm/pr-subscribers-libcxx

Author: A. Jiang (frederick-vs-ja)

Changes

overload_compare_iterator only supports operations required for forward iterators. On the other hand, it is used for output iterators of uninitialized memory algorithms, which requires it to be forward iterator.

As a result, overload_compare_iterator&lt;I&gt;::iterator_category should always be std::forward_iterator_tag if we don't extends its ability. The correct iterator_category can prevent standard library implementations like MSVC STL to attempt random access operations on it.

Fixes #74756.


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

1 Files Affected:

  • (modified) libcxx/test/std/utilities/memory/specialized.algorithms/overload_compare_iterator.h (+6-1)
diff --git a/libcxx/test/std/utilities/memory/specialized.algorithms/overload_compare_iterator.h b/libcxx/test/std/utilities/memory/specialized.algorithms/overload_compare_iterator.h
index d5dcb08c37ed6f..f3b37292e717a1 100644
--- a/libcxx/test/std/utilities/memory/specialized.algorithms/overload_compare_iterator.h
+++ b/libcxx/test/std/utilities/memory/specialized.algorithms/overload_compare_iterator.h
@@ -12,6 +12,7 @@
 
 #include <iterator>
 #include <memory>
+#include <type_traits>
 
 #include "test_macros.h"
 
@@ -21,11 +22,15 @@
 // See https://github.com/llvm/llvm-project/issues/69334 for details.
 template <class Iterator>
 struct overload_compare_iterator {
+  static_assert(
+      std::is_base_of<std::forward_iterator_tag, typename std::iterator_traits<Iterator>::iterator_category>::value,
+      "overload_compare_iterator can only adapt forward iterators");
+
   using value_type        = typename std::iterator_traits<Iterator>::value_type;
   using difference_type   = typename std::iterator_traits<Iterator>::difference_type;
   using reference         = typename std::iterator_traits<Iterator>::reference;
   using pointer           = typename std::iterator_traits<Iterator>::pointer;
-  using iterator_category = typename std::iterator_traits<Iterator>::iterator_category;
+  using iterator_category = std::forward_iterator_tag;
 
   overload_compare_iterator() = default;
 

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.

This shouldn't be marked [NFC], since it fixes a bug in the test suite. Other than that LGTM.

@frederick-vs-ja frederick-vs-ja changed the title [libc++][test][NFC] Fix overload_compare_iterator::iterator_category [libc++][test] Fix overload_compare_iterator::iterator_category Oct 14, 2024
@frederick-vs-ja frederick-vs-ja merged commit d9c2256 into llvm:main Oct 15, 2024
65 checks passed
@frederick-vs-ja frederick-vs-ja deleted the fwd-overload_compare_iterator branch October 15, 2024 14:35
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
…vm#112165)

`overload_compare_iterator` only supports operations required for
forward iterators. On the other hand, it is used for output iterators of
uninitialized memory algorithms, which requires it to be forward
iterator.

As a result, `overload_compare_iterator<I>::iterator_category` should
always be `std::forward_iterator_tag` if we don't extend its ability.
The correct `iterator_category` can prevent standard library
implementations like MSVC STL attempting random access operations on
`overload_compare_iterator`.

Fixes llvm#74756.
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. test-suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc++][test] overload_compare_iterator doesn't support its claimed iterator_category
4 participants