Skip to content

Conversation

mkj
Copy link

@mkj mkj commented May 16, 2025

I've made linear_map public as a namespace for linear_map::Entry. There could be a bit of confusion with the crate::Entry re-export of index_map::Entry, not sure if there's any better option.

Closes #471

@mkj mkj force-pushed the dev/linearentry branch 3 times, most recently from 419d010 to 20b3b69 Compare May 16, 2025 11:39
Copy link
Contributor

@sgued sgued left a comment

Choose a reason for hiding this comment

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

Looking mostly good. Some quick nits and it should be fine.

Comment on lines +686 to +693
// SAFETY: `idx` must not be modified after construction, and
// the size of `map` must not be changed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// SAFETY: `idx` must not be modified after construction, and
// the size of `map` must not be changed.
// SAFETY: `idx` must point to an existing item of the buffer

I don't think that there is any safety issue as long as idx < buffer.len(), only potential logic issues.

Copy link
Author

Choose a reason for hiding this comment

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

It's using get_unchecked() so if self.map.buffer shrank it'd be UB. Maybe I've missed something though?

I wasn't sure whether it was worth using _unchecked() functions, was following the convention of other bits of heapless.

@mkj mkj force-pushed the dev/linearentry branch from 20b3b69 to ad89973 Compare August 25, 2025 04:49
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.

LinearMap entry API
2 participants