C++ Logo

std-discussion

Advanced search

Re: Some feedback on scope guards

From: Jason McKesson <jmckesson_at_[hidden]>
Date: Sun, 16 Apr 2023 10:29:11 -0400
On Sun, Apr 16, 2023 at 7:34 AM Edward Catmur via Std-Discussion
<std-discussion_at_[hidden]> wrote:
>
>
>
> On Sun, 16 Apr 2023 at 08:11, Ville Voutilainen <ville.voutilainen_at_[hidden]> wrote:
>>
>> On Sun, 16 Apr 2023 at 13:40, Edward Catmur <ecatmur_at_[hidden]> wrote:
>> >> > Fine, and how do we specify the facility that such moves have undefined/unspecified behavior but the cases we want to allow do? I can't see a way.
>> >>
>> >> We don't. If I want to store these things in a vector and move the
>> >> vector around, that's my business. I'll need to disable the elements
>> >> if the vector
>> >> has been moved. If I want to wrap these things in a shared_ptr after
>> >> allocating them dynamically, and then store those shared_ptrs into a
>> >> vector,
>> >> that's my business, and none of the library's to prevent.
>> >
>> >
>> > And we need to know what happens when you do that, or know that it is unspecified. What I'm saying is that the behavior needs to be specified, or explicitly unspecified. If the behavior is specified in terms of uncaught_exceptions(), that takes care of it, but leaves the coroutine case broken.
>>
>> I fail to see the problem. It's as if you're suggesting that there's
>> some unknown specification challenge here, but we have the
>> specification
>> you speak of already, in the TS.
>
>
> And that specification breaks with coroutines.
>
>> >> No library type prevents using it as a subobject, or prevents itself
>> >> completely from working with other library types. It sure as hell
>> >> matters.
>> >
>> >
>> > std::typeinfo? std::chrono::time_zone?
>>
>> Yes? Neither of those needs other than pointers to them to work. Scope
>> guards are not like them.
>> You found two cases that presumably prove my statement wrong, but
>> (pointers to) those types can
>> be used as subobjects, that works fine, and they work with other
>> library types without problems, since
>> for them it's truly so that you don't need by-value subobjects of them.
>>
>> Try harder.
>
>
> And you can take pointers to scope guards. Consider also immovable types like scoped_lock - those won't work with most library components.
>
>> >> > Working well can be dealt with by the vendor since these are library types. Wrong - maybe, but this is a scope guard! The name indicates it should be used in a scope.
>> >> Yeah, which does include packaging them any which way I please and
>> >> passing them down to helper functions for further processing,
>> >> or just having a collection-bunch of them in a scope, rather than
>> >> having to have individual objects of the scope guard type directly,
>> >> without
>> >> the possibility to bunch them.
>> > I've never seen code where that would actually be useful.
>>
>> Uhh.. you've never seen code that packages multiple individual objects
>> into a bunch, to pass them as one object to be processed
>> by a helper function? You can't fathom code where you'd take a pack of
>> callbacks and wrap scope guards on top of them, variadically?
>
>
> No, since you may as well wrap the lot into a single scope guard.
>
>> Yeah, sure, I can show you that example you asked for. In my copious free time.
>>
>> >> The current design in the TS is just fine. It specifies how the
>> >> destructor of these things works, and tells you clearly enough what
>> >> happens if you move a container of these things around into a scope
>> >> that has a different value for uncaught_exceptions() than
>> >> the original scope had.
>> > It's not fine, because it breaks with coroutines.
>>
>> Well, gee whizz, that's an acceptable cost, compared to preventing the
>> use of scope guards as subobjects, which is not an
>> acceptable cost.
>
>
> Using scope_guard outside block scope is a code smell, though it can be justified in some rare cases. Using scope_success or scope_failure outside block scope would not pass code review where I work.

Essentially, you're declaring that there is a class of types which
should always be block scoped. I think this is a reasonable thought.
However, if you are writing such a type, then... doesn't it make sense
that this type can *itself* contain members of similar
block-scoped-only types? It's a transitive property: if type X must
always be block scoped, and type Y must always be block scoped, then
type Y can contain a member of type X and the invariant will remain.

So why is it a code smell?

It'd be great if we had some kind of attribute that would make it a
warning to use such a type in a non-block scoped way, or outside of
other block-scoped types. But otherwise, the concept is valid. And so
long as that concept is valid, there is no reason to a priori forbid
their use as members of other block-scoped types.

> Meanwhile, using scope_success and scope_failure in coroutine block scope across suspension points is completely natural; the only problem is that it can break. We should not be adding footguns like that to the standard.

Coroutines already break `thread_local` (details:
https://stackoverflow.com/questions/75448478/thread-local-variables-and-coroutines/75449918#75449918).
So I submit that coroutines already added footguns to the language.

The avalanche has already started; it is too late for the pebbles to vote.

Received on 2023-04-16 14:29:22