Re: [scope] A few questions

From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2023-12-01 14:42:01

On 12/1/23 13:10, Julien Blanc wrote:
> Le 2023-11-30 19:20, Andrey Semashev via Boost a écrit :
>> On 11/30/23 17:54, Julien Blanc via Boost wrote:
> 
>> I can add support for default construcion, if that is considered useful
>> by the community.
> 
> I think it does not hurt for scope_fail and scope_exit. We have
> scope_final for the simple case. I'm asking because i finally added it
> to the scope_exit-like we use in our code base. But we also have a type
> erased self-contained function object, which solves the issue below.

Ok, I will add support for default construction to scope guards.

https://github.com/Lastique/scope/issues/11

>> The example with std::function is not really the best practice, as the
>> section follows on after that example. You should generally avoid it, as
>> it may throw on construction/assignment and therefore break your
>> exception safety.
> 
> Indeed. I fear, however, that people will just come up with this code,
> use it without understanding the implications. I think Andrzej made a
> very good point
> (https://lists.boost.org/Archives/boost//2022/05/253109.php) by saying
> that examples should be exempts from anti-patterns / bad code. I try to
> keep that in mind now when doing a review.

I can't say I fully agree with Andrzej in that post. While it may look
awkward at times that library documentation contains code samples that
one shouldn't (or maybe even doesn't, in their right mind) write in
actual real projects, sometimes this is just the most efficient way to
demonstrate a library feature (such as that a function throws an
exception) or warn about certain caveats or illustrate library misuse.
Library documentation writers do rely on readers having a certain level
of knowledge and intelligence, so that they are able to understand which
examples are merely an illustration and not intended to be copy-pasted
into real code.

That said, maybe I could better articulate in text surrounding that
example that such usage is the recommended way.

> To get back on the default construction, after more thoughts on this, it
> looks like a good rationale (can't guarantee that assignment will be
> nothrow) for *not* providing default construction. But doesn't that
> apply more generally? Isn't writing code like that:
> 
> std::function<void()> getReleaseFunction(int fd)
> {
> Â Â Â Â Â Â Â  return [fd]() { ::close(fd); };
> }
> 
> auto v = boost::scope::scope_exit(getReleaseFunction(fd));
> 
> something that should be completly forbidden, or at least strongly
> discouraged?

Although using std::function has the caveats we've already discussed, I
don't think strictly forbidding it would be the right thing to do. There
may be uses where std::function doesn't affect the correctness of the
surrounding code. So I prefer to make the user aware of the caveats and
let him decide what's important to him.

Besides, there isn't an efficient way to prevent the user from this
issue. Sure, I could specialize scope guards on std::function (at the
cost of including <functional>, which would penalize every user, even
those who never wanted to use std::function to begin with), but it
doesn't prevent users from using boost::function or any other equivalent.

>> There is a guarantee that scope guard construction won't throw, unless
>> constructing one of the function objects throws. This implies that the
>> scope guard itself doesn't do anything that may throw, which includes
>> dynamic memory allocation.
> 
> Thanks, this is the kind of definitive answer i was looking for. I could
> unfortunately not find it in the documentation, though. I think it
> deserves to be added (if it is present, then it probably should be
> emphasized more).

It is part of the scope guard reference documentation, e.g.:

https://lastique.github.io/scope/libs/scope/doc/html/boost/scope/scope_exit.html#idm3835-bb

>> unique_resource is not designed to handle IO errors, it simply ensures
>> you don't leak the resource. The actual correctness of resource
>> management is still user's responsibility.
>>
>> ...
>>
>> Again, unique_resource is not about error handling, or IO in general for
>> that matter. Its only purpose is to free the resource on destruction.
>>
>> However, I can update the example the way you suggest, to avoid the
>> confusion.
> 
> I think there's indeed a dangerous confusion here, and it is exacerbated
> by the fact that both reside in the same library / namespace. scope_exit
> / scope_fail are helpers to write correct code. unique_fd is just about
> not leaking resources. That is indeed definitely not the same thing.
> Anyone who cares about file integrity won't use it when writing files (i
> have yet to see an raii design that works for file integrity).

For reference, I've added an issue about updating the docs:

https://github.com/Lastique/scope/issues/9

I wouldn't say scope guards are a tool for error handling, though. I
mean, they could be used for error handling in some cases, but (a) it's
not the only use case and (b) scope guards usually have a limited
capacity for error handling, as the action is usually not allowed to
throw. I would say, the primary use case of scope guards is implementing
transactional behavior of the code. And to some extent that may include
resource management and error handling.