Page MenuHomePhabricator

[libc++] Fix `_IterOps::__iter_move` to support proxy iterators.
ClosedPublic

Authored by var-const on Jul 20 2022, 12:58 PM.

Details

Summary

The return type was specified incorrectly for proxy iterators that
define reference to be a class that implicitly converts to
value_type. __iter_move would end up returning an object of type
reference which would then implicitly convert to value_type; thus,
the function will return a value_type&& rvalue reference to the local
temporary.

Diff Detail

Event Timeline

var-const created this revision.Jul 20 2022, 12:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2022, 12:58 PM
Herald added a subscriber: mgrang. · View Herald Transcript
var-const requested review of this revision.Jul 20 2022, 12:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2022, 12:58 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.Jul 20 2022, 1:00 PM
ldionne added a subscriber: ldionne.

LGTM, please ship this if it passes locally under --param std=X for all standards.

libcxx/include/__algorithm/iterator_operations.h
68–73

In particular, I think the comment about dangling references is not super useful here since we have a test for that.

This revision is now accepted and ready to land.Jul 20 2022, 1:00 PM
var-const updated this revision to Diff 446246.Jul 20 2022, 1:13 PM

Fix the test in C++03 mode.

huixie90 added inline comments.
libcxx/include/__algorithm/iterator_operations.h
70–73

I am not sure if this solves the problem.

std::vector<bool>::iterator::operator* returns a prvalue Proxy. Even the return type is now Proxy&&, it is still a dangling reference to a local temporary Proxy created by operator*

I think we might have to return by value without std::move if the reference type is a prvalue. (similar to c++17 move_iterator https://timsong-cpp.github.io/cppwp/n4659/move.iterators#move.iterator-1)