offa / scope-guard

Implementation of Scoped Guards and Unique Resource as proposed in P0052.
MIT License
46 stars 6 forks source link

unique_resource::operator= without any noexcept member assignment operations #171

Closed hoosierc closed 3 years ago

hoosierc commented 3 years ago

Hi again, and thank you for the quick response to my previous query. :)

I've been trying to track down the origin of the std::is_nothrow_copyassignable_v\<RR> and std::is\nothrow_copy_assignable_v\<DD> requirements for the move-assignment implementation of unique_resource.

I've found the requirements listed in R6 of P0052 as:

2 Requires: (is_nothrow_move_assignable_v<R> || is_copy_assignable_v<R>) is true and (is_nothrow_move_assignable_v<D> || is_copy_assignable_v<D>)is true.

and I'm wondering if the current state was just a misinterpretation of those as is_nothrow_copy_assignable_v instead of is_copy_assignable_v or if there's something else that contributed which I haven't tracked down yet.

From what I can tell in R10, the only requirement for the members if they're not no-throw move assignable is Cpp17CopyAssignable which doesn't seem to me to have any specification regarding exception handling, but I might just be misinterpreting the requirements.

For some context, the reason this is coming up for me is that I was attempting to use a std::function as the deleter, and doing so resulted in losing the ability to assign the unique_resource since, as far as I can tell, std::function assignment is not required to be noexcept (but, according to P0771, it is in some library implementations anyway). I'm not able to reproduce this issue with std::function using the latest toolchains available on Compiler Explorer, but https://godbolt.org/z/a3EjYK illustrates the sort of error I've encountered with older toolchains where swapping out which definition is used on lines 640-641 allows compilation.

I've also tried to create a crude std::function stand-in that is explicitly not noexcept for the move/copy operations, and I'm seeing a similar compiler error with trunk GCC in that case. This one also has the definitions on line 640-641 that can be swapped out. https://godbolt.org/z/T5ParP

I'm guessing this may be a case where I'm misunderstanding the spec and/or there's a reason that unique_resource can't be implemented according to the spec in R10, but I wanted to reach out in case it was intended to work.

offa commented 3 years ago

Thanks for providing such a detailed issue :smiley:! You are right, the current behaviour is not correct.

Requires: If is_nothrow_move_assignable_v is true, R1 shall meet the Cpp17MoveAssignable requirements (Table 28); otherwise R1 shall meet the Cpp17CopyAssignable (Table 29) require- ments. If is_nothrow_move_assignable_v is true, D shall meet the Cpp17MoveAssignable requirements (Table 28); otherwise D shall meet the Cpp17CopyAssignable requirements (Table 29).

(§ 7.6.3/1)

Cpp17CopyAssignable doesn't mention anything regarding noexcept (C++20 draft).

offa commented 3 years ago

I have changed the type requirement, feel free to reopen if there are any issues left. Thanks for reporting! :+1:

hoosierc commented 3 years ago

I figure a detailed issue is the least I can do. You're doing the hard work here. :) Thanks again for the quick update!