-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: Revise notes for example std_global_allocator.rs
#36
base: master
Are you sure you want to change the base?
Conversation
Group all notes together, revision pass on phrasing.
std_global_allocator.rs
std_global_allocator.rs
// note: | ||
// - Miri thinks this violates stacked borrows upon program termination. | ||
// - This only occurs with `#[global_allocator]`. | ||
// - Use the allocator API if you can't have that. Perhaps check out `examples/stable_allocator_api.rs`? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternative, although it'd be good to know what "if you can't have that" is referring to.
// - Use the allocator API if you can't have that. Perhaps check out `examples/stable_allocator_api.rs`? | |
// - Consider using the allocator API if you can't have that (see: `examples/stable_allocator_api.rs`) |
#[global_allocator] | ||
static ALLOCATOR: Talck<spin::Mutex<()>, ClaimOnOom> = Talc::new(unsafe { | ||
ClaimOnOom::new(Span::from_const_array(std::ptr::addr_of!(START_ARENA))) | ||
}).lock(); | ||
ClaimOnOom::new(Span::from_const_array(std::ptr::addr_of!(START_ARENA))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore no_std
compatibility, matches README equivalent:
ClaimOnOom::new(Span::from_const_array(std::ptr::addr_of!(START_ARENA))) | |
ClaimOnOom::new(Span::from_const_array(core::ptr::addr_of!(START_ARENA))) |
Hey Brennan, thanks for the contribution! Got a packed week but I'll try get around to this within a few days ^_^ |
I noticed a recent commit hoisted the two type comments from their contextual proximity to relevant code, while the
START_ARENA
line seems to have no relevance separating the two comment sections. Grouped them together as bullet point notes.The two type focused comments are more explicit about communicating that early now, followed by the description for the choice (it's not really communicated why
spin::Mutex
is sensible to the user, but I suppose that's better context than none). Was vaguely touched on here, I could add a link to that for reference if considered helpful?It's not clear what the Miri comment refers to by "this", other than the relation to the global allocator attribute. Nor what "if you can't have that" is referring to (the violation being potentially valid, or a false-positive from Miri?). Additionally corrected a typo + corrected the file name that was changed since the comment was written.
The shorter README equivalent uses
ARENA
instead ofSTART_ARENA
, wasSTART_
meant to communicate any additional context?START_
is an initial arena.