Skip to content

Change MemoryMapper::get_mut to take a &mut self#3636

Merged
stevenengler merged 1 commit intoshadow:mainfrom
stevenengler:rust-1.88
Aug 6, 2025
Merged

Change MemoryMapper::get_mut to take a &mut self#3636
stevenengler merged 1 commit intoshadow:mainfrom
stevenengler:rust-1.88

Conversation

@stevenengler
Copy link
Copy Markdown
Contributor

@stevenengler stevenengler commented Jul 24, 2025 •
edited
Loading

This clippy error was added in rust 1.88. In #3632 I added an #[allow(...)], but I think it would be better using a &mut self for now.

See #3632 (comment) for context.

error: mutable borrow from immutable input(s)
    --> main/host/memory_manager/memory_mapper.rs:1079:77
     |
1079 |     pub unsafe fn get_mut<T: Pod>(&self, src: ForeignArrayPtr<T>) -> Option<&mut [T]> {
     |                                                                             ^^^^^^^^
     |
note: immutable borrow here
    --> main/host/memory_manager/memory_mapper.rs:1079:35
     |
1079 |     pub unsafe fn get_mut<T: Pod>(&self, src: ForeignArrayPtr<T>) -> Option<&mut [T]> {
     |                                   ^^^^^
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#mut_from_ref
     = note: `#[deny(clippy::mut_from_ref)]` on by default

We previously used the mutable reference in a higher-layer type (MemoryManager::mapped_mut) to ensure there would be only one mutable reference to plugin memory. This PR changes MemoryMapper::get_mut to also take a mutable reference to ensure we don't accidentally introduce a bug where we allow multiple mutable references.

@stevenengler stevenengler self-assigned this Jul 24, 2025
@stevenengler stevenengler added the Type: Maintenance Refactoring, cleanup, documenation, or process improvements label Jul 24, 2025
@github-actions github-actions bot added Component: Testing Unit and integration tests and frameworks Component: Main Composing the core Shadow executable labels Jul 24, 2025
@stevenengler stevenengler changed the title Changed MemoryMapper::get_mut to take a &mut self Change MemoryMapper::get_mut to take a &mut self Jul 24, 2025
@stevenengler stevenengler mentioned this pull request Jul 24, 2025
Merged
@stevenengler stevenengler requested a review from sporksmith July 24, 2025 00:08
@stevenengler
Changed MemoryMapper::get_mut to take a &mut self
1e218b6 
We previously used the mutable reference in a higher-layer type
(`MemoryManager::mapped_mut`) to ensure there would be only one mutable
reference to plugin memory. This commit changes `MemoryMapper::get_mut`
to also take a mutable reference to ensure we don't accidentally
introduce a bug where we allow multiple mutable references.
sporksmith
sporksmith approved these changes Aug 5, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@sporksmith sporksmith left a comment

Choose a reason for hiding this comment

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

Yup, makes sense

@stevenengler stevenengler merged commit e6055d3 into shadow:main Aug 6, 2025
24 checks passed
@stevenengler stevenengler deleted the rust-1.88 branch August 6, 2025 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sporksmith sporksmith sporksmith approved these changes

Assignees

@stevenengler stevenengler

Labels

Component: Main Composing the core Shadow executable Component: Testing Unit and integration tests and frameworks Type: Maintenance Refactoring, cleanup, documenation, or process improvements

Projects

None yet

Milestone

Code health and maintenance

Development

Successfully merging this pull request may close these issues.

2 participants

@stevenengler @sporksmith