Fully fix the forwarding problems in getset lens

The patch ensures the following requirements:

1) When `view(lens, whole)` is called:

   * the setter lambda is neither called nor constructed.
     It avoid unnecessary copies of the 'whole' object.

   * the `whole` object is directly forwarded into the getter lambda,
     without any copies.

3) When `set(lens, whole, part)` is called:

   * no getter is called at the toplevel lens (which
     value was previously discarded).

   * on the lower lenses, the getter receives an 'lvalue'
     reference to the `whole` object, and setter receives an
     'rvalue' reference to the `whole` object. It ensures
     that there is no double-move problem.

Fixes #160

Author: dimula73

lager/lenses.hpp

@@ -21,6 +21,15 @@
 namespace lager {
 namespace detail {
 
+template <typename T>
+struct should_move :
+    std::integral_constant<bool,
+                           !std::is_array_v<T> &&
+                               !std::is_lvalue_reference_v<T>> {};
+
+template <typename T>
+constexpr bool should_move_v = should_move<T>::value;
+
 template <typename T>
 struct const_functor;
 
@@ -42,6 +51,15 @@ struct const_functor
     }
 };
 
+template <typename T>
+struct is_const_functor : public std::false_type  {};
+
+template <typename T>
+struct is_const_functor<const_functor<T>> : public std::true_type {};
+
+template <typename T>
+constexpr bool is_const_functor_v = is_const_functor<T>::value;
+
 template <typename T>
 struct identity_functor;
 
@@ -59,40 +77,111 @@ struct identity_functor
     template <typename Fn>
     auto operator()(Fn&& f) &&
     {
-        return make_identity_functor(
-            std::forward<Fn>(f)(std::forward<T>(value)));
+
+        if constexpr (!should_move_v<T>) {
+            return make_identity_functor(
+                std::forward<Fn>(f)(value));
+        } else {
+            return make_identity_functor(
+                std::forward<Fn>(f)(std::move(value)));
+        }
     }
 };
 
 template <typename Part>
 struct identity_functor_skip_first
 {
     template <typename T>
-    auto operator() (T&&) const {
+    auto operator() (T&&) const & {
         return make_identity_functor(part);
     }
 
-    Part &part;
+    template <typename T>
+    auto operator() (T&&) && {
+        if constexpr (!should_move_v<Part>) {
+            return make_identity_functor(part);
+        } else {
+            return make_identity_functor(std::move(part));
+        }
+    }
+
+    Part part;
 };
 
 template <typename T>
-auto make_identity_functor_skip_first(T&& x) -> identity_functor_skip_first<std::remove_reference_t<T>>
+auto make_identity_functor_skip_first(T&& x) -> identity_functor_skip_first<T>
 {
     return {std::forward<T>(x)};
 }
 
-template <typename Func, typename Getter, typename Whole>
-auto call_getter_or_skip(const Func& func, Getter&& getter, Whole&& whole) {
-    return func(LAGER_FWD(getter)(LAGER_FWD(whole)));
-}
+template <typename F, typename Getter, typename Setter>
+struct getset_t
+{
+    template <typename Whole>
+    auto operator() (Whole &&w) {
+        if constexpr (is_const_functor_v<decltype(f(getter(LAGER_FWD(w))))>) {
+            /**
+             * We don't have a setter here, so it is safe to
+             * jus pass `w` as an rvalue.
+             *
+             * We also know that we are calling the const_functor,
+             * and it discards the passed argument, so just pass a
+             * noop to it.
+             *
+             * This branch is taken when viewing through the lens.
+             */
+            return f(getter(LAGER_FWD(w))) // pass `w` into the getter as an rvalue!
+                (zug::noop);
+        } else {
+            /**
+             * Here we have both, getter and setter, so we pass
+             * `w` to getter as an lvalue, and to setter as an rvalue.
+             * The setter has a chance to reuse the resources of
+             * the passed value.
+             *
+             * This branch is taken on all the levels of setting the
+             * value through except of the tompost level.
+             */
+            return f(getter(w)) // pass `w` into the getter as an lvalue!
+                ([&](auto&& x) {
+                    return setter(LAGER_FWD(w), LAGER_FWD(x));
+                });
+        }
+    }
+
+    F f;
+    Getter getter;
+    Setter setter;
+};
+
+/**
+ * This specialization is called when a set() method is called over
+ * the lens. In such a case we can skip calling the getter branch
+ * of the lens.
+ *
+ * This branch is taken on the topmost level of setting the value
+ * through the lens.
+ */
+template <typename T, typename Getter, typename Setter>
+struct getset_t<identity_functor_skip_first<T>, Getter, Setter>
+{
+    template <typename Part>
+    auto operator() (Part &&p) {
+        return std::move(f)(zug::noop)
+            ([&](auto&& x) {
+                return setter(LAGER_FWD(p), LAGER_FWD(x));
+            });
+    }
+
+    identity_functor_skip_first<T> &&f;
+    Getter getter;
+    Setter setter;
+};
 
-template <typename Getter, typename Part, typename Whole>
-auto call_getter_or_skip(const identity_functor_skip_first<Part>& func, Getter&&, Whole&&) {
-    /**
-     * When set() call is being executed, we should not execute the setter,
-     * the value will be dropped later anyway
-     */
-    return func(*static_cast<Part*>(nullptr));
+template <typename F, typename Getter, typename Setter>
+auto make_getset_t(F &&f, const Getter &getter, const Setter &setter)
+{
+    return getset_t<F, Getter, Setter>{std::forward<F>(f), getter, setter};
 }
 
 } // namespace detail
@@ -112,7 +201,7 @@ decltype(auto) view(LensT&& lens, T&& x)
 template <typename LensT, typename T, typename U>
 decltype(auto) set(LensT&& lens, T&& x, U&& v)
 {
-    return lens(detail::make_identity_functor_skip_first(v)) (
+    return lens(detail::make_identity_functor_skip_first(std::forward<decltype(v)>(v))) (
                std::forward<T>(x))
         .value;
 }
@@ -135,14 +224,10 @@ namespace lenses {
 //! @{
 
 template <typename Getter, typename Setter>
-auto getset(Getter&& getter, Setter&& setter)
+auto getset(const Getter& getter, const Setter& setter)
 {
     return zug::comp([=](auto&& f) {
-        return [&, f = LAGER_FWD(f)](auto&& p) {
-            return detail::call_getter_or_skip(f, getter, LAGER_FWD(p))([&](auto&& x) {
-                return setter(LAGER_FWD(p), LAGER_FWD(x));
-            });
-        };
+        return detail::make_getset_t(LAGER_FWD(f), getter, setter);
     });
 }