Re: [boost] [scope] review

On 12/6/23 14:58, Julien Blanc wrote:

Le 2023-12-05 16:54, Andrey Semashev via Boost a écrit :

...

On 12/5/23 17:34, Julien Blanc via Boost wrote:

...

Regarding why scope_fail doesn't enforce noexcept-ness of the action, it is because the "failure" may be indicated by other means than an exception. The library explicitly supports error codes as an alternative.

In general, I think that marking scope guards' destructors noexcept is pointless. If the action throws while there is another exception in flight, your program will terminate either way. If the action throws while there is no other exception then why should the scope guard terminate the program? Throwing in this case might as well be the intended behavior. Because if it isn't intended then it is the user who must communicate this by marking his operator() as noexcept.

I think there's a misunderstanding here. My point was exactly about asserting is_nothrow_invocable for the scope_fail constructor argument, not for its destructor.

It doesn't matter where the assert is. I disagree with the requirement itself.

[unique_resource]

...

I'd like to note that the way resource traits are currently defined, they allow multiple unallocated states of the resource. For example, for POSIX file descriptors, any negative value is unallocated while -1 is just the default. As far as I understand, `markable` doesn't support this.

It does, and in a way that is very similar to unique_resource: https://github.com/akrzemi1/markable/blob/master/documentation/overview.adoc...

...

From the implementation standpoint, I don't think that relying on `markable` for detecting unallocated state would simplify the code much, if at all. You'd still have to effectively duplicate the implementation of unique_resource.

A very basic implementation of unique_resource could ressemble this:

template class unique_resource {     std::optional<Resource> r_;     Deleter d_; public:     unique_resource(unique_resource const&) = delete;     unique_resource& operator=(unique_resource const&) = delete;     ~unique_resource() noexcept() {         if (r_.has_value()) d_(r_.value());     }     // ...

Most of the code in the proposed unique_resource is implementing yet-another-optional.

unique_resource doesn't work like optional, the resource object stays constructed for the whole lifetime of the owner unique_resource object. For example, the TS does not require the resource to be in allocated state to be able to call unique_resource::get().

The invalid value detection does look a lot like what markable does. So i'm wondering to which extent the proposed unique_resource could not just be rewritten by:

template class unique_resource {    compressed_pair p_;    ... };

and using unique_fd = unique_resource >;

Am i missing something obvious here?

A significant part of the unique_resource implementation is duplicated to support resource types with unallocated values. See the unique_resource_data class. That part could be changed to rely on `markable`, but that would not eliminate the need to specialize for `markable` instead of for resource traits. This is because unique_resource provides an interface that is different from `markable`, and there has to be some part of the implementation that provides that interface and knows how to interact with `markable`. A lot of implementation complexity is also because of the exception safety requirements. For example, unique_resource is supposed to call the deleter on the resource, if its construction fails with an exception. Note that both the resource and the deleter may fail to construct, which is the reason why resource_holder and deleter_holder exist in the current implementation. I don't see that part of the implementation going away with `markable` either. So in the end, what `markable` would eliminate is the need in a couple of type traits (has_custom_default and has_deallocated_state), which is a very minor part of the code. The rest would stay more-or-less the same. And let's not forget that `markable` itself has some level of complexity. But more importantly, there are design issues with this approach. What would `unique_resource::get()` return? And what would be passed to the deleter? If it's `markable<T> const&` then that would be difficult to use in practice, as you would need to call `fd.get().value()` to access the actual file descriptor. And for deleters, it would make it impossible to specify raw functions like `close` as deleters. So it looks like `unique_resource` would have to unwrap `markable` internally, which adds another level of complexity. And it wouldn't work anyway because `markable` requires that the stored value is not in the marked state to be able to call `value()`. Same problem as with `optional`. So, with all things considered, it looks like `markable` is incompatible with `unique_resource` interface. And even if it was, it would only make thinks more complicated.