Skip to content
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

single_view should not copy the value #817

Closed
ilyapopov opened this issue Apr 8, 2018 · 10 comments
Closed

single_view should not copy the value #817

ilyapopov opened this issue Apr 8, 2018 · 10 comments

Comments

@ilyapopov
Copy link

Currently, single_view requires value type to be copy constructible, and copies it into itself. This, in my view, goes against the concept of view, which is not supposed to own values, but to refer to the original values. Thus I believe single_view should store a reference to original value.

@ericniebler
Copy link
Owner

I disagree. Many views own state. They must have O(1) copy and move, and single_view satisfies that requirement.

@ilyapopov
Copy link
Author

But this places unnecessary requirements on the element type (one can't use it with non-copiable types). It is also a clear performance pessimization (copy can be expensive). If not changed, it should probably be renamed, since in the current form it is more of a container, rather than a view.

Use case I had is: I have a generic function that takes a range of objects and processes them. But in one place I want to call this function with only one object. I considered passing a single_view, and was surprised by behavior, which is different from what I expect from a view, hence the issue.

@ilyapopov
Copy link
Author

Additionally, there is the following text in the Ranges TS:

Otherwise, if both T and const T satisfy Range and reference_t <iterator_t> is not the same type as reference_t <iterator_t> , false; [Note: Deep const-ness implies element ownership, hereas shallow const-ness implies reference semantics. — end note]

Which implies that the range that owns its elements should not be considered a view.

@ericniebler
Copy link
Owner

That's merely a heuristic for guessing at the view-ness of a range. That heuristic cannot know that the single_view only holds one element.

@apmccartney
Copy link
Contributor

apmccartney commented May 2, 2018

@ilyapopov Assuming this is a requirement in your work, as a work around, how about

#include <type_traits>
#include <range/v3/view/single.hpp>
#include <range/v3/view/indirect.hpp>

struct reference_single_view_fn {
  template<typename T, std::enable_if_t<std::is_lvalue_reference<T>::value, bool> = true>
  auto operator(T&& value) const {
    return ranges::view::single(&value) | ranges::view::indirect;
  }
};

static constexpr reference_single_view_fn reference_single_view{};

@ericniebler
Copy link
Owner

@CaseyCarter can you remind me why the single_view returns by value instead of by (const?) lvalue reference? It prevents view::single from working with move-only types.

@CaseyCarter
Copy link
Collaborator

CaseyCarter commented Oct 11, 2018

No idea. single_view's iterators are pointers in P0896:

template<CopyConstructible T>
  requires is_object_v<T>
class single_view : public view_interface<single_view<T>> {
private:
  semiregular<T> value_; // exposition only
public:
  single_view() = default;
  constexpr explicit single_view(const T& t);
  constexpr explicit single_view(T&& t);
  template<class... Args>
    requires Constructible<T, Args...>
  constexpr single_view(in_place_t, Args&&... args);

  constexpr T* begin() noexcept;
  constexpr const T* begin() const noexcept;
  constexpr T* end() noexcept;
  constexpr const T* end() const noexcept;
  constexpr static ptrdiff_t size() noexcept;
  constexpr T* data() noexcept;
  constexpr const T* data() const noexcept;
};

template<class T>
  explicit single_view(T&&) -> single_view<decay_t<T>>;

It's roughly std::array<std::optional<T>, 1>.

@ericniebler
Copy link
Owner

I like P0896's formulation better.

@CaseyCarter
Copy link
Collaborator

FWIW: span<T, 1> provides a good "reference to a single element" View, albeit one that is awkward to construct.

@ericniebler
Copy link
Owner

#918 makes single_view work with move-only types by returning by reference, as in P0896.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants