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

Fix double std::move on objects in 'lenses::getset' on set call #165

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Commits on Nov 21, 2022

  1. Fix extra call to 'getter' in 'lenses::getset' on set call

    Basically, we shouldn't calculate the getter when we know that the
    value will be dropped.
    
    The patch partially fixes arximboldi#160, because now the getter is not called
    at all during the set operation. But the double-move can still
    happen when both, getter and setter calls are performed.
    dimula73 committed Nov 21, 2022
    Configuration menu
    Copy the full SHA
    b863b87 View commit details
    Browse the repository at this point in the history

Commits on Nov 22, 2022

  1. Fix copilation with gcc9

    error: 'auto' not allowed in function prototype
    dimula73 committed Nov 22, 2022
    Configuration menu
    Copy the full SHA
    596983f View commit details
    Browse the repository at this point in the history

Commits on Dec 22, 2022

  1. 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 arximboldi#160
    dimula73 committed Dec 22, 2022
    Configuration menu
    Copy the full SHA
    51e3c0e View commit details
    Browse the repository at this point in the history

Commits on Dec 23, 2022

  1. Fix the move-from verification code

    When the moved-from object is assigned, it becomes valid again. None
    of our code seems to use it though.
    dimula73 committed Dec 23, 2022
    Configuration menu
    Copy the full SHA
    3ea51d9 View commit details
    Browse the repository at this point in the history
  2. Add tests for nested attr|getset lens

    Such lens causes:
    
    1) Compilaion failure due to the missing const functor in getset_t
       (also fixed in the patch)
    
    2) A crash caused by double-move in the attr implementation
       (cannot be fixed without rewriting attr into getset)
    dimula73 committed Dec 23, 2022
    Configuration menu
    Copy the full SHA
    af07b69 View commit details
    Browse the repository at this point in the history
  3. Rewrite attr into using getset instead of raw lambda functions

    The previous implementation cased severe issues:
    
    1) Double-move of `p` argument when passing a prvalue into a
       nested chain of two attr lenses, which usually causes a
       crash.
    
    2) `f = LAGER_FWD(f)` creates a `const decltype(f) f` object
       inside the lambda, which prevents the value to be perfect-
       forwarded with `identity_functor` and `const_functor` when
       setting/fetching it.
    dimula73 committed Dec 23, 2022
    Configuration menu
    Copy the full SHA
    ce03ff3 View commit details
    Browse the repository at this point in the history
  4. Fix compilation of lenses/unbox.hpp

    The functor should be forwarded into the lens.
    dimula73 committed Dec 23, 2022
    Configuration menu
    Copy the full SHA
    0b6ab3e View commit details
    Browse the repository at this point in the history