Skip to content

Comments

Make IntoPyObject for Bound & Borrowed more generic#5831

Merged
davidhewitt merged 1 commit intoPyO3:mainfrom
bschoenmaeckers:independent-bound
Feb 18, 2026
Merged

Make IntoPyObject for Bound & Borrowed more generic#5831
davidhewitt merged 1 commit intoPyO3:mainfrom
bschoenmaeckers:independent-bound

Conversation

@bschoenmaeckers
Copy link
Member

Split out of #5822 because it is not needed there anymore, but might be useful for other generic code.

@bschoenmaeckers bschoenmaeckers force-pushed the independent-bound branch 2 times, most recently from c77f5ea to 114d4b0 Compare February 18, 2026 10:29
Copy link
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.

I can see it being reasonable to relax these bounds 👍

@davidhewitt davidhewitt added this pull request to the merge queue Feb 18, 2026
Merged via the queue into PyO3:main with commit ee66c21 Feb 18, 2026
45 checks passed
Comment on lines +1082 to +1085
pub fn as_unbound(&self) -> &'a Py<T> {
// Safety: NonNull<ffi::PyObject> is layout-compatible with Py<T>
unsafe { NonNull::from(&self.0).cast().as_ref() }
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this might be unsound. I think the &Py can only live as long as the reference to the Borrowed (&self) and not for 'a, as the pointer we get from self is only valid until the borrow ends at the end of the scope.

Copy link
Member

Choose a reason for hiding this comment

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

You're absolutely right, thanks for catching this and sorry I missed it review!

Copy link
Member

Choose a reason for hiding this comment

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

Glad I could catch it. Opened #5832 to revert this for now.

Copy link
Member Author

@bschoenmaeckers bschoenmaeckers Feb 18, 2026

Choose a reason for hiding this comment

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

Sorry for my mistake, I will take a closer look tomorrow so I understand the relationship between these lifetimes better.

Copy link
Member

Choose a reason for hiding this comment

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

No worries, I made a similar form of exactly this mistake many times when we were trying to implement the Bound API originally!

Icxolu added a commit that referenced this pull request Feb 18, 2026
github-merge-queue bot pushed a commit that referenced this pull request Feb 19, 2026
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.

3 participants