[libc++] Make common_iterator's proxy types into aggregates

[• Quuxplusone]

Summary

This is an alternative to one part of D120998; @ldionne, do you like this diff better than the corresponding diff in D120998?

This saves one move in each case, which is basically nothing perf-wise; this is more about simplifying the code.

[ldionne]

Yes, I like this better, if I understood the logic in my comment properly.

libcxx/include/__iterator/common_iterator.h

123

Right, so we know that _VSTD::__unchecked_get<_Iter>(__hold_) (aka the iterator) has a non-reference iter_reference_t, which is a complicated way of saying *iter is a temporary. So when we construct the aggregate now, we're even eliding a move operation. That sounds right?

[• Quuxplusone]

libcxx/include/__iterator/common_iterator.h

123

We're replacing a conversion iter_reference_t&& -> iter_value_t with a conversion iter_reference_t -> iter_value_t. In the case that iter_reference_t and iter_value_t are the same type, this is a savings. Otherwise, AIUI, it's really no different. (But I imagine that "they're the same type" is the common case.)

Also in the __postfix_proxy case, we're replacing iter_reference_t&& -> iter_reference_t with iter_reference_t -> iter_reference_t, which definitely saves one move-ctor of whatever type iter_reference_t is.

libcxx/include/__iterator/common_iterator.h

 template<class _Iter>
 concept __can_use_postfix_proxy =
   constructible_from<iter_value_t<_Iter>, iter_reference_t<_Iter>> &&
   move_constructible<iter_value_t<_Iter>>;
 
 template<input_or_output_iterator _Iter, sentinel_for<_Iter> _Sent>
   requires (!same_as<_Iter, _Sent> && copyable<_Iter>)
 class common_iterator {
-  class __proxy {
-    friend common_iterator;
-
-    iter_value_t<_Iter> __value;
-    // We can move __x because the only caller verifies that __x is not a reference.
-    constexpr explicit __proxy(iter_reference_t<_Iter>&& __x)
-      : __value(_VSTD::move(__x)) {}
-
-  public:
+  struct __proxy {
     constexpr const iter_value_t<_Iter>* operator->() const noexcept {
-      return _VSTD::addressof(__value);
+      return _VSTD::addressof(__value_);
     }
+    iter_value_t<_Iter> __value_;
   };
 
-  class __postfix_proxy {
-    friend common_iterator;
-
-    iter_value_t<_Iter> __value;
-    constexpr explicit __postfix_proxy(iter_reference_t<_Iter>&& __x)
-      : __value(_VSTD::forward<iter_reference_t<_Iter>>(__x)) {}
-
-  public:
+  struct __postfix_proxy {
     constexpr const iter_value_t<_Iter>& operator*() const noexcept {
-      return __value;
+      return __value_;
     }
+    iter_value_t<_Iter> __value_;
   };
 
 public:
   variant<_Iter, _Sent> __hold_;
 
   common_iterator() requires default_initializable<_Iter> = default;
 
   constexpr common_iterator(_Iter __i) : __hold_(in_place_type<_Iter>, _VSTD::move(__i)) {}
public:
   {
     _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_), "Attempted to dereference a non-dereferenceable common_iterator");
     if constexpr (is_pointer_v<_Iter> || requires(const _Iter& __i) { __i.operator->(); })    {
       return _VSTD::__unchecked_get<_Iter>(__hold_);
     } else if constexpr (is_reference_v<iter_reference_t<_Iter>>) {
       auto&& __tmp = *_VSTD::__unchecked_get<_Iter>(__hold_);
       return _VSTD::addressof(__tmp);
     } else {
-      return __proxy(*_VSTD::__unchecked_get<_Iter>(__hold_));
+      return __proxy{*_VSTD::__unchecked_get<_Iter>(__hold_)};
     }
   }
 
   common_iterator& operator++() {
     _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_), "Attempted to increment a non-dereferenceable common_iterator");
     ++_VSTD::__unchecked_get<_Iter>(__hold_); return *this;
   }
 
   decltype(auto) operator++(int) {
     _LIBCPP_ASSERT(holds_alternative<_Iter>(__hold_), "Attempted to increment a non-dereferenceable common_iterator");
     if constexpr (forward_iterator<_Iter>) {
       auto __tmp = *this;
       ++*this;
       return __tmp;
     } else if constexpr (requires (_Iter& __i) { { *__i++ } -> __can_reference; } ||
                          !__can_use_postfix_proxy<_Iter>) {
       return _VSTD::__unchecked_get<_Iter>(__hold_)++;
     } else {
-      __postfix_proxy __p(**this);
+      auto __p = __postfix_proxy{**this};
       ++*this;
       return __p;
     }
   }
 
   template<class _I2, sentinel_for<_Iter> _S2>
     requires sentinel_for<_Sent, _I2>
   friend constexpr bool operator==(const common_iterator& __x, const common_iterator<_I2, _S2>& __y) {