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

Feature: defer #31

Merged
merged 5 commits into from
Jan 31, 2024
Merged

Feature: defer #31

merged 5 commits into from
Jan 31, 2024

Conversation

liss-h
Copy link
Contributor

@liss-h liss-h commented Jan 24, 2024

Implements a mechanism like GO's defer or C23's defer proposal or Andrei Alexandrescu's SCOPE_EXIT mechanism.

Open question:

  • Do we want to keep the macro naming as is or do we name them DICE_DEFER to avoid conflicts?
    • Pro keeping names: short, readable
    • Pro changing names: more unlikely to conflict with macros from other people

@liss-h liss-h requested a review from bigerl January 24, 2024 10:44
@liss-h liss-h linked an issue Jan 24, 2024 that may be closed by this pull request
Copy link
Member

@bigerl bigerl left a comment

Choose a reason for hiding this comment

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

Thx for the PR. Beautifully built. thx. two minor things.
Regarding your question: I would change the names to DICE_DEFER, etc.

include/dice/template-library/defer.hpp Show resolved Hide resolved
include/dice/template-library/defer.hpp Show resolved Hide resolved
Copy link
Member

@bigerl bigerl left a comment

Choose a reason for hiding this comment

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

docstring comments ScopeGuardOnExit not the namespace at the moment. To document the namespace, it's name should be changed. Please also change the name of the namespace where it is used.

PS: The changes look good otherwise and the explanations are easy to follow. 👍🏻

include/dice/template-library/defer.hpp Outdated Show resolved Hide resolved
@liss-h liss-h requested a review from bigerl January 30, 2024 13:22
Copy link
Member

@bigerl bigerl left a comment

Choose a reason for hiding this comment

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

Sorry for the review dribbling in one thing after another.

I just had another look at the branch and realized that a mentioning of the new library feature in https://github.com/dice-group/dice-template-library/tree/feature/defer?tab=readme-ov-file#dice-template-library and a short description in https://github.com/dice-group/dice-template-library/tree/feature/defer?tab=readme-ov-file#usage is missing.

Copy link
Member

@bigerl bigerl left a comment

Choose a reason for hiding this comment

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

The change DEFER* -> DICE_DEFER* is still missing

see: #31 (review)

@liss-h
Copy link
Contributor Author

liss-h commented Jan 30, 2024

The change DEFER* -> DICE_DEFER* is still missing

see: #31 (review)

Ups, I didn't see that

@liss-h liss-h requested a review from bigerl January 31, 2024 09:59
Copy link
Member

@bigerl bigerl left a comment

Choose a reason for hiding this comment

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

pefect. thx :)

@liss-h liss-h merged commit 391795f into develop Jan 31, 2024
6 checks passed
@liss-h liss-h mentioned this pull request Jan 31, 2024
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.

Add scope guard/defer
2 participants