Skip to content

Change GILGuard to be able to represent a GIL that was already held#4187

Merged
alex merged 1 commit intoPyO3:mainfrom
alex:gil-guard-simplification
May 16, 2024
Merged

Change GILGuard to be able to represent a GIL that was already held#4187
alex merged 1 commit intoPyO3:mainfrom
alex:gil-guard-simplification

Conversation

@alex
Copy link
Copy Markdown
Member

@alex alex commented May 16, 2024

See #4181

@alex alex added the CI-skip-changelog Skip checking changelog entry label May 16, 2024
@alex alex force-pushed the gil-guard-simplification branch 2 times, most recently from 01fc4ec to 0367f2d Compare May 16, 2024 03:28
Copy link
Copy Markdown
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

One thought, otherwise LGTM and feel free to merge!

src/gil.rs Outdated
/// Indicates the GIL was already held with this GILGuard was acquired.
Assumed,
/// Indicates that we actually acquired the GIL when this GILGuard was acquired
Held {
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.

I like the name suggested by @adamreichold

Suggested change
Held {
Ensured {

src/gil.rs Outdated
Comment on lines 165 to 166
/// If the GIL was already acquired via PyO3, this returns `None`. Otherwise,
/// the GIL will be acquired and a new `GILPool` created.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Need a change to the docs of the return value here.

@alex alex force-pushed the gil-guard-simplification branch from 0367f2d to 0535563 Compare May 16, 2024 21:35
@alex alex enabled auto-merge May 16, 2024 21:36
@alex alex added this pull request to the merge queue May 16, 2024
Merged via the queue into PyO3:main with commit 8de1787 May 16, 2024
@alex alex deleted the gil-guard-simplification branch May 16, 2024 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI-skip-changelog Skip checking changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants