Re: [scope] Scope review starts on November 26th

From: Janko Dedic (jankodedic2_at_[hidden])
Date: 2023-11-30 20:39:51

On Thu, Nov 30, 2023 at 4:52 PM Andrey Semashev via Boost <
boost_at_[hidden]> wrote:

> On 11/30/23 00:35, Janko Dedic via Boost wrote:
> > On Wed, Nov 29, 2023 at 4:56 PM Andrey Semashev via Boost <
> > boost_at_[hidden]> wrote:
> >
> >> I don't see the current definition of BOOST_SCOPE_FINAL as a deficiency
> >> of the interface compared to passing the capture list in the macro
> >> arguments. The latter doesn't offer any advantages compared to the
> >> current definition, and in fact is more limiting as it only allows for
> >> lambda functions for the scope guard action.
> >
> > It offers better syntax.
>
> Better in what way, other than your personal preference? Can you
> describe objective advantages of your proposed syntax and why the
> current BOOST_SCOPE_FINAL doesn't work for you?
>

Better because the user is not forced to type the capture list everywhere.
Ideally we would not even require a semicolon at the end, but we can't get
rid of that. The ideal is a core language construct like Go's defer or D's
scope(exit).

> >> Besides, note that the lambda definition syntax has evolved over time
> >> and may now include additional elements such as template parameters,
> >> noexcept and mutable qualifiers and the trailing return type. There may
> >> be new syntax additions in the future, which could be incompatible with
> >> the macro definition. Although some of the syntax elements are useless
> >> for the purpose of scope guards, at least noexcept and mutable are
> >> meaningful and would look awkward if BOOST_SCOPE_FINAL accepted the
> >> capture list in its parameters.
> >
> > I only suggested adding [&] to the macro, or adding a macro that includes
> > the [&].
>
> Sorry, but no. That would limit user's options for no reason, and
> binding variables by value is not uncommon. Users of Boost.Scope should
> have the full range of function object definition ways that is permitted
> by the language.
>

And why is having these options useful? Note that the user can still use
the constructor or the factory function directly to fully customize the
behavior (if they want to for whatever reason). I can understand your
motivation behind this macro (declaring an anonymous scope guard variable
only), but in 99% of cases I would personally end up using
BOOST_SCOPE_FINAL [&], and I don't understand why the library wouldn't
provide such a macro (Alexandrescu's original SCOPE_EXIT works like this).

> >> If you want to enforce that your action never throws, you can declare
> >> its operator() as noexcept:
> >>
> >>   BOOST_SCOPE_FINAL []() noexcept { /* I guarantee I won't throw */ };
> >
> > Does anyone really want to write code like this?
>
> I see nothing wrong with this code. I actually wrote code like this,
> where the no-throw guarantee was important.
>

It just looks like a weird macro incantation to me personally. SCOPE_EXIT {
run(); }; at least tried to model a control flow construct, even though
there's the unfortunate semicolon at the end. I strive to have the least
amount of clever macros possible in my code.

> >> Movability is necessary for the factory functions, at the very least.
> >
> > I can see why. C++17 fixes this "defect". You can technically rely on
> > reference lifetime extension, but that doesn't look as good.
> >
> > auto&& guard = make_scope_exit(...);
>
> Reference lifetime extension has nothing to do with it. Without RVO, in
> order to return the scope guard from the factory function by value, its
> copy or move constructor needs to be invoked.
>

Move constructor can be private to achieve this. Then, lifetime extension
works and the user can't move on their side.

> >>> I did not like the locked_write_string example. Reading this code
> >> locally:
> >>>
> >>>     // Lock the file
> >>>     while (flock(fd, LOCK_EX) < 0)
> >>>     {
> >>>         err = errno;
> >>>         if (err != EINTR)
> >>>             return;
> >>>     }
> >>>
> >>> it's hard to get what's happening, why we're setting err and how big of
> >> an
> >>> impact that has on function behavior. A better way to write this code
> >> would
> >>> be to simply extract this piece into its own function:
> >>>
> >>>         err = errno;
> >>>         if (err != EINTR)
> >>>             return;
> >>>
> >>> together with the ec = std::error_code(err, std::generic_category()),
> >> which
> >>> doesn't require a scope guard.
> >>
> >> Sorry, I don't understand your suggestion. How would this help with
> >> other return points of the function?
> >
> > It helps with all the return points in the example function.
>
> I don't see how. Can you provide a code example?
>
> Note that the checks for EINTR are made in different contexts. Some of
> them cause return, other - restart loop iteration, third - affect
> error_code initialization.
>

As far as I can tell, they're all the same. The last loop can be rewritten
to be like the rest.

> >>> I think a similar utility would be more useful in form of a class
> >> template,
> >>> for example:
> >>>
> >>> resource<HANDLE, INVALID_HANDLE_VALUE, ::CloseHandle> windows_handle;
> >>> resource<SDL_Window*, nullptr, ::SDL_DestroyWindow> sdl_window_handle;
> >>>
> >>> This would be very useful for concisely implementing RAII wrappers
> over C
> >>> handles (you don't have to manually write destructors, move
> constructors
> >>> and move assignment operators).
> >>
> >> This would require C++17, if I'm not mistaken (or when were the auto
> >> non-type template parameters introduced?). I don't think it offers much
> >> advantage compared to the current unique_resource with resource traits.
> >
> >  The advantage is doing the same thing in 1 line instead of 20 lines of
> > code, at which point writing the destructor and moves manually has a
> better
> > ROI than implementing traits classes.
>
> This would also prohibit resource types with non-constexpr constructors,
> as well as non-default-constructible resources. I don't like this tradeoff.
>

 It's not necessarily a "trade-off." I personally don't find the full
flexibility of unique_resource that useful, but I can understand why you'd
want to support it. I'm just saying that the work you have to do by
providing custom resource traits basically ends up being more than writing
destructors and moves manually. I'm also suggesting that perhaps having a
type like I've described that solves 99% of cases in one line might be a
valuable addition to the library.