Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow final action to invoke throwing actions #1192

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

apenn-msft
Copy link

where an action is provided to final action to execute at end of scope and that action throws, because final action's destructor is marked noexcept, the program will terminate. Instead, allow final action's destructor to be noexcept dependent upon whether the action throws.

where an action is provided to final action to execute at end of scope and that action throws, because final action's destructor is marked noexcept, the program will terminate. Instead, allow final action's destructor to be noexcept dependent upon whether the action throws.
@apenn-msft
Copy link
Author

Note when discussing this with a few others, there was some pushback regarding adherence to guidelines that destructors must not throw. the spirit of the guideline is to prevent half-baked objects, and throwing during stack unwind.
In this case, the final action destructor does not constitute destruction of an object, and is a mechanism purely used to invoke an arbitrary action on scope exit, so it should be considered a legitimate exception to the rule:
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c36-a-destructor-must-not-fail

As well, while the destructor being noexcept may achieve stack-unwind termination in a more direct manner, even if final action should invoke a throwing action during stack-unwind, the runtime is still required to terminate:
https://en.cppreference.com/w/cpp/language/throw

It is arguable whether a termination from an explicit noexcept or stack-unwind throw is better or worse; however I believe a minor nit during already-terminating scenario should not come at the expense/prevention of legitimate code being executed and instead terminating due to an artificial constraint.

In other words, with this change, a final action which throws during stack unwind will still terminate as before; but a final action which throws during normal destruction will now be allowed to work, improving the usefulness of a gsl::finally as a general purpose utility to invoke code at scope exit.

@apenn-msft
Copy link
Author

@apenn-msft please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="Microsoft"

@apenn-msft apenn-msft marked this pull request as ready for review February 3, 2025 23:19
@apenn-msft
Copy link
Author

Note I have companion PRs here:
#1191
#1190

these are not additive to this PR but rather alternatives to this PR to determine which the community best likes. In all 3 PRs the same problem we want to address is that gsl finally makes it too easy to write code that would cause surprise crashes at runtime.

@apenn-msft
Copy link
Author

pending discussion of #1193

@apenn-msft
Copy link
Author

also as a safer existing implementation, see fundamentals TS scope_success:

https://cplusplus.github.io/fundamentals-ts/v3.html#scopeguard.exit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant