Skip to content

Conversation

@Wybxc
Copy link
Contributor

@Wybxc Wybxc commented Aug 19, 2025

Fix #159.

See also: #162.

@ginnyTheCat ginnyTheCat requested a review from messense August 20, 2025 16:40
@ginnyTheCat
Copy link
Collaborator

ginnyTheCat commented Aug 20, 2025

Looks great! I tested this PR in my projects that use NativeDevice and it works well. Thanks for implementing this despite the changing requirements. I appreciate it a lot

Maybe @messense can have a look to see if I'm missing sth or if this is good to go.

@messense messense requested a review from Copilot August 21, 2025 01:06
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes memory safety issues in the native device implementation by changing how trait objects are handled and improving lifetime management. The changes address the referenced issue #159 and are related to issue #162.

Key changes:

  • Added a 'static lifetime bound to the NativeDevice trait to ensure proper memory management
  • Replaced mutable reference implementation with Box<T> for better ownership semantics
  • Added Rc<RefCell<T>> implementation to enable shared ownership with interior mutability
  • Updated test code to use the new Rc<RefCell<T>> pattern

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@messense messense merged commit e418de4 into messense:main Aug 21, 2025
12 checks passed
@Wybxc Wybxc deleted the device-1 branch August 21, 2025 02:08
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.

Device::from_native is unsafe

3 participants