C++ Logo

std-discussion

Advanced search

Re: Some feedback on scope guards

From: Andrey Semashev <andrey.semashev_at_[hidden]>
Date: Sun, 16 Apr 2023 18:36:49 +0300
On April 16, 2023 5:29:27 PM Jason McKesson via Std-Discussion
<std-discussion_at_[hidden]> wrote:

> 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.

No, the concept doesn't look valid to me. What about having a block-scoped
unique_ptr pointing to such type? Presumably, unique_ptr won't be marked as
block-scoped, so should this be forbidden? Why, other than "because it
breaks the concept?" And unique_ptr is just one example.



>
>
>> 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.
> --
> Std-Discussion mailing list
> Std-Discussion_at_[hidden]
> https://lists.isocpp.org/mailman/listinfo.cgi/std-discussion

Received on 2023-04-16 15:36:54