Skip to content

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