Making user-defined constructors of view iterators/sentinels private

• • Abstract
  • Revision history
  • Discussion
  • Proposed change

Abstract

The exposure of user-defined constructors for iterators/sentinels in <ranges> currently does not follow consistent rules, which is reflected in the fact that some of them are public and some are private. This paper disables their visibility to comply with best practices.

Note that this may be a potential break, but after consulting the opinions of the various library implementers, this break is extremely unlikely. If so, it mostly indicates a user bug.

Revision history

R1

Added discussion for breakage.

R0

Initial revision.

Discussion

What's the issue?

Currently in <ranges>, to access the meow_view's member variables, meow_view::iterator/sentinel saves a pointer pointing to meow_view and provides a constructor that accepts a reference to it for initializing the pointer.

However, some of these user-defined constructors are declared as public, which allows users to construct them by passing in a specific view base:

    auto base = std::views::iota(0);
    auto filter = base | std::views::filter([](int) { return true; });
    auto begin = decltype(filter.begin())(filter, base.begin()); // ok
    auto end   = decltype(filter.end()  )(filter);               // ok
  

The author believes that we should prohibit providing these constructors to users. As the example above shows, this doesn't make much sense and provides no observable value.

It is worth noting that the above example fails to compile in libstdc++, because in libstdc++ the constructor is implemented to accept a pointer to filter_view, which allows it to directly pass in this pointer in begin() to save one addressof call.

Although this is a bug in libstdc++ according to the current wording, it is clearly a side effect of the constructor being accidentally exposed. Implementers are absolutely allowed to construct the iterator by just passing this to initialize the member, which is what chunk_view::iterator's user-defined constructor does.

Since both methods exist in <ranges>, it shows that this is implementation-related and should not be perceived by users.

The author believes that apart from the default constructor and the conversion constructor which has an obvious intent, there is no reason for constructors that are only used for implementation purposes to be exposed to the user.

What are the potential breakages?

During the Tokyo meeting, SG9 was concerned about the potential breakage that would result from declaring these constructors private. However, the actual damage caused by such change is negligible.

For libstdc++, to work around the issue that instantiating the begin()/end() requires computing the satisfaction of the range concept, r11-4584 changes several user-defined constructors mentioned in the paper from taking a reference to taking a pointer, indicating no breakages for GCC users in this paper because those constructors do not exist in the first place. Although later r11-5954 optimized the frontend making the former workaround redundant, rechanging these constructors to take a reference to conform to the current wording seems to be a bad idea, since no user is actually using them.

MSVC-STL doesn't seem to have much concern about this either. In the SG9 mailing list, Mr. Stephan believes that the paper has made a great change and expressed strong support for the direction.

Although libc++ has a test specifically for the user-defined constructor of basic_istream_view::iterator, Mr. Louis acknowledged that this was simply a matter of maximizing coverage, and he thinks that it is no big deal to turn these constructors into private. Quoted from the SG9 mailing list: "I'd be surprised if we found any breakage, and if we did I wouldn't be surprised if there was something wrong with that code in the first place."

In summary, the potential breakages do not cause any concern for the implementers of three major standard libraries. The author believes that this paper highlights the better design direction of C++23 range adaptors. It is worthwhile to bring these changes back to C++20 ranges adaptors, just as we did in P2711: Making multi-param constructors of views explicit.

Proposed change

This wording is relative to N4958.

[Drafting note: The proposed resolution does not touch user-defined constructors of iota_view::iterator/sentinel because it does make sense to expose them.]

1. Modify 26.6.6.3 [range.istream.iterator], class basic_istream_view::iterator synopsis, as indicated:

       namespace std::ranges {
         template<movable Val, class CharT, class Traits>
           requires default_initializable<Val> &&
                    stream-extractable<Val, CharT, Traits>
         class basic_istream_view<Val, CharT, Traits>::iterator {
         public:
           using iterator_concept = input_iterator_tag;
           using difference_type = ptrdiff_t;
           using value_type = Val;
       
           constexpr explicit iterator(basic_istream_view& parent) noexcept;
       
           […]
         
         private:  
           basic_istream_view* parent_;                                        // exposition only
       
           constexpr explicit iterator(basic_istream_view& parent) noexcept;   // exposition only
         };
       }
       

2. Modify 26.7.8.3 [range.filter.iterator], class filter_view::iterator synopsis, as indicated:

       namespace std::ranges {
         template<input_range V, indirect_unary_predicate<iterator_t<V>> Pred>
           requires view<V> && is_object_v<Pred>
         class filter_view<V, Pred>::iterator {
         private:
           iterator_t<V> current_ = iterator_t<V>();   // exposition only
           filter_view* parent_ = nullptr;             // exposition only
       
           constexpr iterator(filter_view& parent, iterator_t<V> current);   // exposition only
       
         public:
           […]
       
           iterator() requires default_initializable<iterator_t<V>> = default;
           constexpr iterator(filter_view& parent, iterator_t<V> current);
       
           […]
         };
       }
       

3. Modify 26.7.8.4 [range.filter.sentinel], class filter_view::sentinel synopsis, as indicated:

       namespace std::ranges {
         template<input_range V, indirect_unary_predicate<iterator_t<V>> Pred>
           requires view<V> && is_object_v<Pred>
         class filter_view<V, Pred>::sentinel {
         private:
           sentinel_t<V> end_ = sentinel_t<V>();       // exposition only
       
           constexpr explicit sentinel(filter_view& parent);   // exposition only
       
         public:
           sentinel() = default;
           constexpr explicit sentinel(filter_view& parent);
       
           […]
         };
       }
       

4. Modify 26.7.9.3 [range.transform.iterator], class template transform_view::iterator synopsis, as indicated:

       namespace std::ranges {
         template<input_range V, move_constructible F>
           requires view<V> && is_object_v<F> &&
                    regular_invocable<F&, range_reference_t<V>> &&
                    can-reference<invoke_result_t<F&, range_reference_t<V>>>
         template<bool Const>
         class transform_view<V, F>::iterator {
         private:
           […]
       
           constexpr iterator(Parent& parent, iterator_t<Base> current);  // exposition only
       
         public:
           […]
       
           iterator() requires default_initializable<iterator_t<Base>> = default;
           constexpr iterator(Parent& parent, iterator_t<Base> current);
           […]
         };
       }
       

5. Modify 26.7.9.4 [range.transform.sentinel], class template transform_view::sentinel synopsis, as indicated:

       namespace std::ranges {
         template<input_range V, move_constructible F>
           requires view<V> && is_object_v<F> &&
                     regular_invocable<F&, range_reference_t<V>> &&
                     can-reference<invoke_result_t<F&, range_reference_t<V>>>
         template<bool Const>
         class transform_view<V, F>::sentinel {
         private:
           […]
       
           constexpr explicit sentinel(sentinel_t<Base> end);    // exposition only
       
         public:
           sentinel() = default;
           constexpr explicit sentinel(sentinel_t<Base> end);
           […]
         };
       }
       

6. Modify 26.7.10.3 [range.take.sentinel], class template take_view::sentinel synopsis, as indicated:

       namespace std::ranges {
         template<view V>
         template<bool Const>
         class take_view<V>::sentinel {
         private:
           […]
       
           constexpr explicit sentinel(sentinel_t<Base> end);    // exposition only
       
         public:
           sentinel() = default;
           constexpr explicit sentinel(sentinel_t<Base> end);
           […]
         };
       }
       

7. Modify 26.7.11.3 [range.take.while.sentinel], class template take_while_view::sentinel synopsis, as indicated:

       namespace std::ranges {
         template<view V, class Pred>
           requires input_range<V> && is_object_v<Pred> &&
                    indirect_unary_predicate<const Pred, iterator_t<V>>
         template<bool Const>
         class take_while_view<V, Pred>::sentinel {
         private:
           […]
       
           constexpr explicit sentinel(sentinel_t<Base> end, const Pred* pred);    // exposition only
       
         public:
           sentinel() = default;
           constexpr explicit sentinel(sentinel_t<Base> end, const Pred* pred);
           […]
         };
       }
       

8. Modify 26.7.14.4 [range.join.sentinel], class template join_view::sentinel synopsis, as indicated:

       namespace std::ranges {
         template<input_range V>
           requires view<V> && input_range<range_reference_t<V>>
         template<bool Const>
         struct join_view<V>::sentinel {
         private:
           […]
       
           constexpr explicit sentinel(Parent& parent);    // exposition only
       
         public:
           sentinel() = default;
       
           constexpr explicit sentinel(Parent& parent);
           […]
         };
       }
       

9. Modify 26.7.16.3 [range.lazy.split.outer], class template lazy_split_view::outer-iterator synopsis, as indicated:

       namespace std::ranges {
         template<input_range V, forward_range Pattern>
           requires view<V> && view<Pattern> &&
                    indirectly_comparable<iterator_t<V>, iterator_t<Pattern>, ranges::equal_to> &&
                    (forward_range<V> || tiny-range<Pattern>)
         template<bool Const>
         struct lazy_split_view<V, Pattern>::outer-iterator {
         private:
           […]
       
           constexpr explicit outer-iterator(Parent& parent)                     // exposition only
             requires (!forward_range<Base>);
           constexpr outer-iterator(Parent& parent, iterator_t<Base> current)    // exposition only
             requires forward_range<Base>;
       
         public:
           […]
           constexpr explicit outer-iterator(Parent& parent)
             requires (!forward_range<Base>);
           constexpr outer-iterator(Parent& parent, iterator_t<Base> current)
             requires forward_range<Base>;
           […]
         };
       }
       

10. Modify 26.7.16.5 [range.lazy.split.inner], class template lazy_split_view::inner-iterator synopsis, as indicated:

        namespace std::ranges {
          template<input_range V, forward_range Pattern>
            requires view<V> && view<Pattern> &&
                      indirectly_comparable<iterator_t<V>, iterator_t<Pattern>, ranges::equal_to> &&
                      (forward_range<V> || tiny-range<Pattern>)
          template<bool Const>
          struct lazy_split_view<V, Pattern>::inner-iterator {
          private:
            […]
        
            constexpr explicit inner-iterator(outer-iterator<Const> i);    // exposition only
        
          public:
            […]
        
            inner-iterator() = default;
            constexpr explicit inner-iterator(outer-iterator<Const> i);
            […]
          };
        }
        

11. Modify 26.7.17.3 [range.split.iterator], class split_view::iterator synopsis, as indicated:

        namespace std::ranges {
          template<forward_range V, forward_range Pattern>
            requires view<V> && view<Pattern> &&
                     indirectly_comparable<iterator_t<V>, iterator_t<Pattern>, ranges::equal_to>
          class split_view<V, Pattern>::iterator {
          private:
            […]
        
            constexpr iterator(split_view& parent, iterator_t<V> current, subrange<iterator_t<V>> next);    // exposition only
        
          public:
            […]
        
            iterator() = default;
            constexpr iterator(split_view& parent, iterator_t<V> current, subrange<iterator_t<V>> next);
            […]
          };
        }
        

12. Modify 26.7.17.4 [range.split.sentinel], class split_view::sentinel synopsis, as indicated:

        namespace std::ranges {
          template<forward_range V, forward_range Pattern>
            requires view<V> && view<Pattern> &&
                     indirectly_comparable<iterator_t<V>, iterator_t<Pattern>, ranges::equal_to>
          struct split_view<V, Pattern>::sentinel {
          private:
            sentinel_t<V> end_ = sentinel_t<V>();               // exposition only
        
            constexpr explicit sentinel(split_view& parent);    // exposition only
        
          public:
            sentinel() = default;
            constexpr explicit sentinel(split_view& parent);
            […]
          };
        }
          

13. Modify 26.7.22.3 [range.elements.iterator], class template elements_view::iterator synopsis, as indicated:

        namespace std::ranges {
          template<input_range V, size_t N>
            requires view<V> && has-tuple-element<range_value_t<V>, N> &&
                     has-tuple-element<remove_reference_t<range_reference_t<V>>, N> &&
                     returnable-element<range_reference_t<V>, N>
          template<bool Const>
          class elements_view<V, N>::iterator {
          private:
            […]
        
            constexpr explicit iterator(iterator_t<Base> current);    // exposition only
        
          public:
            […]
        
            iterator() requires default_initializable<iterator_t<Base>> = default;
            constexpr explicit iterator(iterator_t<Base> current);
            […]
          };
        }
        

14. Modify 26.7.22.4 [range.elements.sentinel], class template elements_view::sentinel synopsis, as indicated:

        namespace std::ranges {
          template<input_range V, size_t N>
            requires view<V> && has-tuple-element<range_value_t<V>, N> &&
                      has-tuple-element<remove_reference_t<range_reference_t<V>>, N> &&
                      returnable-element<range_reference_t<V>, N>
          template<bool Const>
          class elements_view<V, N>::sentinel {
          private:
            […]
        
            constexpr explicit sentinel(sentinel_t<Base> end);    // exposition only
        
          public:
            sentinel() = default;
            constexpr explicit sentinel(sentinel_t<Base> end);
            […]
          };
        }