Skip to content

[Work in progress] version zero - scope_guard - #20

Merged
JeffGarland merged 13 commits into
mainfrom
scope-guard-ver-zero
May 25, 2025
Merged

[Work in progress] version zero - scope_guard#20
JeffGarland merged 13 commits into
mainfrom
scope-guard-ver-zero

Conversation

@robert-andrzejuk

Copy link
Copy Markdown
Collaborator

No description provided.

@robert-andrzejuk
robert-andrzejuk requested a review from a team April 23, 2025 23:43
github-actions[bot]

This comment was marked as resolved.

Comment thread include/beman/scope/scope.hpp
Comment thread include/beman/scope/scope.hpp Outdated
Comment thread include/beman/scope/scope.hpp
Comment thread tests/scope_fail.test.cpp
@JeffGarland

This comment was marked as outdated.

Comment thread include/beman/scope/scope.hpp
Comment thread include/beman/scope/scope.hpp Outdated
Comment thread include/beman/scope/scope.hpp Outdated
if constexpr (ConstructionExceptionBehavior == exception_during_constuction_behaviour::invoke_exit_func) {
exit_func();

// To throw? or not to throw?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

joke?

Comment thread include/beman/scope/scope.hpp Outdated
Comment thread include/beman/scope/scope.hpp Outdated
std::is_nothrow_move_constructible_v<InvokeChecker>)
requires(HasRelease<InvokeChecker> || HasStaticRelease<InvokeChecker>)
: m_exit_func{std::move(rhs.m_exit_func)}, m_invoke_checker{std::move(rhs.m_invoke_checker)} {
// TODO: This does not work corectly for a shared invoke checker

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to add a test case?

@JeffGarland

This comment was marked as outdated.

@JeffGarland

This comment was marked as outdated.

@robert-andrzejuk

Copy link
Copy Markdown
Collaborator Author

I'll implement the suggested changes soon and update the pull request.

Is there a coding or naming guideline for the project?

The experimental implementations are not supported in MSVC.
So I've made a basic unique_resource stub.
I still have some uncertainties about it's usage and nitty details.

@JeffGarland

Copy link
Copy Markdown
Member

I'll implement the suggested changes soon and update the pull request.

Ok.

Is there a coding or naming guideline for the project?

No, we probably should have that though -- I'm just wanting to stick to what the standard does since that's how the wording needs to be written. Also, it minimizes the change for users when they update to the std version.

The experimental implementations are not supported in MSVC. So I've made a basic unique_resource stub. I still have some uncertainties about it's usage and nitty details.

For the moment MSVC is completely shut off in the CI -- so it's perfectly fine to limit the scope of this PR and add unique_resource real impl in another PR.

Note that my current goal is to have a working version by the end of the month. The 'mailing deadline' for the June c++ meeting is the 19th -- so there's a small possibility of getting the initial submission there.

Comment thread include/beman/scope/scope.hpp Outdated
JeffGarland and others added 4 commits May 19, 2025 05:11
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@JeffGarland

Copy link
Copy Markdown
Member

@robert-andrzejuk - unblocking this. I've implemented most of my suggestions and resolved comments. Also made current with current main branch. This will now hopefully pass all the CI

My current thinking is we should create a separate issue for the remaining items - including unique_resource. My personal development preference is to do small improvements and update main frequently (especially now that main is being mirrored in godbolt).

@JeffGarland

Copy link
Copy Markdown
Member

I've created issues for the remaining work/discussion here. I'm merging this to main so we have the initial implementation to work from.

@JeffGarland
JeffGarland merged commit d74d6e0 into main May 25, 2025
@ClausKlein
ClausKlein deleted the scope-guard-ver-zero branch February 27, 2026 20:48
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.

2 participants