Re: [scope] some more thoughts

From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2023-12-03 23:28:39

On 12/3/23 19:26, Andrzej Krzemienski wrote:
> niedz., 3 gru 2023 o 15:56Â Andrey Semashev via Boost
> <boost_at_[hidden] <mailto:boost_at_[hidden]>> napisał(a):
> 
>     > Why do we have scope_success? An alternative to using scope
>     success is to
>     > just put your instructions at the end of the function. Do you have
>     multiple
>     > return statements in your function and at the same time you need
>     the same
>     > sequence of operations to be performed at each end? This is a good
>     > motivation to restructure your function to have a single return.
>     Multiple
>     > returns are generally OK, but needing the same sequence warrants a
>     single
>     > return.
> 
>     What if having a single return is difficult to achieve? Sprinkling `goto
>     finish` doesn't work well sometimes, e.g. when you have variable
>     definitions in the middle of the function. It is also more prone to
>     errors.
> 
> Further discussion would be easier if you were able to provide an
> example of such a function.
> It may be just my lacking imagination, but in my experience any
> complicated function could be
> refactored, e.g. some parts split to separate functions or lambdas, so
> that a single return could be achieved, or multiple returns without
> identical sequence of operations.
> A good example could prove me wrong.

Ok, here's a candidate that could use a scope_success. The method
handles an incoming RTSP request. It is supposed to analyze the request
validity, parse its common headers, find a session which the request
belongs to and invoke a handler in that session. In any case, the method
is supposed to send a response to that request - either a positive 200
or an error. Naturally, depending on the outcome, there may be different
additional headers that need to be added to the response.

Extremely simplified, it currently looks something like this:

  void receiver::on_request(request const& req)
  {
    response resp;

    {
      auto it = req.headers().find(cseq);
      if (req.start_line().version == protocol_version::unknown)
      {
        resp.start_line().version = m_protocol_version;
        resp.start_line().set_status(version_not_supported);

        if (it != req.headers().end())
          resp.headers().set(cseq, it->value(), it->is_sensitive());

        resp.set_body(text_plain, rtsp_version_not_supported_body);

        goto send_response;
      }

      resp.start_line().version = req.start_line().version;

      if (it == req.headers().end())
      {
        resp.start_line().set_status(bad_request);
        goto send_response;
      }

      // ... and so on, continue to validate the request, filling
      // the response as needed, based on the request, and jumping
      // to send_response below in case of errors
    }

    {
      rtsp_session* rtsp_sess = find_session(req);
      if (!rtsp_sess)
      {
        resp.start_line().set_status(session_not_found);
        goto send_response;
      }

      switch (req.start_line().method)
      {
        // For each supported method, analyze request headers
        // that pertain to that method, invoke a handler in the
        // session or otherwise handle it. Then either break or
        // goto send_response.
      }

      // At this point the request is handled successfully
      resp.start_line().set_status(ok);
    }

  send_response:
    send_message(resp, [this](error_code const& error)
    {
      // Handle sending errors, if any
    });
  }

You can see the pattern here. resp gets populated according to the
request and the processing results, then it gets sent in the end.

This code could be simplified by wrapping the send_message call in a
scope_success at the beginning of the function. This would remove the
need to introduce additional scopes to make the goto work, and would
allow to replace all gotos with returns. I could also accompany it with
a scope_fail to send a special response in case of exception.
(Currently, exceptions are handled elsewhere.)

  void receiver::on_request(request const& req)
  {
    response resp;

    scope_success success_guard{[&]
    {
      send_message(resp, [this](error_code const& error)
      {
        // Handle sending errors, if any
      });
    }};

    scope_fail failure_guard{[&]
    {
      send_fatal_response_and_disconnect(resp); // doesn't throw
    }};

    auto it = req.headers().find(cseq);
    if (req.start_line().version == protocol_version::unknown)
    {
      // ...
      return;
    }

    // etc.
  }

Also, separate from the example above, a few other use cases. Although
not quite the same as scope_success, it appears I'm using a similar
concept in Boost.Log in a couple of places:

https://github.com/boostorg/log/blob/8b921b6352e76ff54f5674c864d1f4f749c9158b/include/boost/log/sources/record_ostream.hpp#L460-L539

https://github.com/boostorg/log/blob/8b921b6352e76ff54f5674c864d1f4f749c9158b/include/boost/log/detail/format.hpp#L246-L331

In both cases, the classes work a "pumps". I.e. they initiate data
collection on construction, then the user provides the data (by calling
operator<< or operator% to format the log message), then on destruction,
if no exception was thrown, the pump pushes the collected data to the
logger. So, for example, in the following expression:

  BOOST_LOG(lg) << "Hello " << "world!";

BOOST_LOG macro creates the record_pump object, then the operator<<
calls compose the log message, then the record_pump object is destroyed
and, if the operator<< calls didn't throw, pushes the log record into
the logger. If an exception is thrown then the message is discarded.

This design is influenced by the required syntax - I need to push the
record *after* the message is formatted. But I believe, such a pattern
is not uncommon and could use scope_success for this purpose. In a way,
the previous example with RTSP request handling also fits this pattern -
you create a scope guard, then you populate some data structure, then,
if everything is ok, you send that data structure for further processing.

I will also note that there are a few examples in D documentation
regarding scope guards:

https://dlang.org/articles/exception-safe.html