Repository navigation
Add some docs to Global - #163332
Add some docs to Global#163332
Global#163332Conversation
74244c1 to
372d6b3
Compare
|
r? clarfonthey |
372d6b3 to
3e574ad
Compare
| /// * All instances of `Global` are [*equivalent*]. | ||
| /// * Allocations from `Global` are only invalidated by calls to de-/reallocating functions. | ||
| /// If no such call is made, then the allocation will live for the rest of the program. | ||
| /// * The global allocation functions are equivalent to the methods on `Global`, except |
There was a problem hiding this comment.
Is the implication that it's OK to Global.allocate and then std::alloc::deallocate that memory? I would expect that to not be the case. I think it definitely shouldn't be guaranteed that you can Global.allocate and System.deallocate, even if #[global_allocator] = System.
There was a problem hiding this comment.
That must be the case, since otherwise the guarantees made by Box about this are broken, unless we special case around it by saying "if you go to a Box (or Vec) first, then you can alloc::dealloc or Global.deallocate". But at that point we might as well just guarantee this.
In particular, we allow you to create Box<T, Global> using alloc::alloc() and from_raw. Likewise for its allocator obviously. OTOH, we allow you to deallocate it using into_raw and alloc::dealloc, and obviously using its allocator.
There was a problem hiding this comment.
Note this is specifically for Global, not for anything that is equivalent to Global, like that example. The goal is to deprecate these methods explicitly in favour of methods on Global, so, it's useful to say they're identical.
There was a problem hiding this comment.
Also note that using the allocator you registered as the global allocator (e.g. System) to deallocate stuff from Global/alloc::alloc(...) is never allowed. Does my text insinuate that? If so, it needs to be fixed
There was a problem hiding this comment.
I think it would be worth making sure that these lines from std::alloc::alloc is copied here (or into doc comments on allocate/deallocate):
invoking this function is not equivalent to invoking the underlying GlobalAlloc::alloc method of the registered allocator directly
and:
The allocation can only be freed by invoking dealloc or realloc. In particular, passing a pointer to such an allocation directly to the underlying method on GlobalAlloc is not permitted.
I think that last paragraph at least implies you can't std::alloc::alloc() and then <Global as GlobalAlloc>::dealloc(...) -- perhaps the right thing is to rephrase it, alongside any coping done?
It may be worth expanding the documentation at the top level (std::alloc):
The global allocator is invoked via the functions in this module (alloc, alloc_zeroed, dealloc, realloc). Note, however, that invoking those functions is not equivalent to directly invoking the underlying methods on the declared global allocator! See the documentation of those functions for details.
This saying something about the relationship of methods from:
- Global as Allocator
- std::alloc free-standing functions
- Global as GlobalAlloc
- Global as GlobalAllocator
I believe we annotate some of these functions to LLVM -- maybe just the inner functions that we eventually call though? -- and given the proliferation of ways to reach the #[global_allocator] it seems worth documenting some more details.
I'm not sure if the text here insinuates you can interchange with System, but IMO we have a bunch of documentation around this that is not as clear as it used to be (in part due to added indirection and added Allocator stabilization), and probably was never crystal clear.
There was a problem hiding this comment.
I think it would be worth making sure that these lines from
std::alloc::allocis copied here (or into doc comments on allocate/deallocate):
sure, we can do that
The allocation can only be freed by invoking dealloc or realloc. In particular, passing a pointer to such an allocation directly to the underlying method on GlobalAlloc is not permitted.
I think that last paragraph at least implies you can't
std::alloc::alloc()and then<Global as GlobalAlloc>::dealloc(...)
Global is not the underlying GlobalAlloc allocator. In fact Global does not implement GlobalAlloc. If it did, it would create an infinite loop when registered as the global allocator. Global is the allocator that invokes the registered #[global_allocator] through the free standing allocation functions in alloc, and is therefore equivalent to them. In particular, as contradictory as that might sound, Global is never a global allocator.
…larfonthey Add some docs to `Global` r? libs
…uwer Rollup of 7 pull requests Successful merges: - #162493 (Add support for -Zsanitizer-cfi-minimal-runtime) - #163133 ([rustdoc] Fix invalid jump to def link when `#[rustc_allow_incoherent_impl]` is involved) - #163215 (Fix suggestion for Option to bool with proper precedence handling) - #163266 (More deferred liveness cleanups) - #163274 (Support -Z merge-functions with gcc and add stack-protector asm tests) - #163312 (Add rustdoc regression test for glob import of a crate that re-exports) - #163332 (Add some docs to `Global`)
Rollup of 13 pull requests Successful merges: - #160695 (Detect bad number of generics caused by bad derive) - #162883 (No longer auto-inline `f16` and `f128` on well-supported platforms) - #163133 ([rustdoc] Fix invalid jump to def link when `#[rustc_allow_incoherent_impl]` is involved) - #163215 (Fix suggestion for Option to bool with proper precedence handling) - #163266 (More deferred liveness cleanups) - #163274 (Support -Z merge-functions with gcc and add stack-protector asm tests) - #163290 (Merge `ForceWarning`/`Allow`/`Expect` into `Warning`) - #163298 (Don't emit sugg for pub-reexport extern std) - #163312 (Add rustdoc regression test for glob import of a crate that re-exports) - #163357 (Borrowck cleanups part trois) - #161710 (Stabilize `mem::conjure_zst`) - #163332 (Add some docs to `Global`) - #163362 (simplify ndk compiler test)
Rollup merge of #163332 - maxdexh:global-allocator-docs, r=clarfonthey Add some docs to `Global` r? libs
Rollup of 13 pull requests Successful merges: - rust-lang/rust#160695 (Detect bad number of generics caused by bad derive) - rust-lang/rust#162883 (No longer auto-inline `f16` and `f128` on well-supported platforms) - rust-lang/rust#163133 ([rustdoc] Fix invalid jump to def link when `#[rustc_allow_incoherent_impl]` is involved) - rust-lang/rust#163215 (Fix suggestion for Option to bool with proper precedence handling) - rust-lang/rust#163266 (More deferred liveness cleanups) - rust-lang/rust#163274 (Support -Z merge-functions with gcc and add stack-protector asm tests) - rust-lang/rust#163290 (Merge `ForceWarning`/`Allow`/`Expect` into `Warning`) - rust-lang/rust#163298 (Don't emit sugg for pub-reexport extern std) - rust-lang/rust#163312 (Add rustdoc regression test for glob import of a crate that re-exports) - rust-lang/rust#163357 (Borrowck cleanups part trois) - rust-lang/rust#161710 (Stabilize `mem::conjure_zst`) - rust-lang/rust#163332 (Add some docs to `Global`) - rust-lang/rust#163362 (simplify ndk compiler test)
…=clarfonthey Clarify some things in the docs of `Global` Follow-up to clear up some remaining confusion after rust-lang#163332 cc @Mark-Simulacrum r? clarfonthey
Rollup merge of #163401 - maxdexh:global-allocator-docs-2, r=clarfonthey Clarify some things in the docs of `Global` Follow-up to clear up some remaining confusion after #163332 cc @Mark-Simulacrum r? clarfonthey
Clarify some things in the docs of `Global` Follow-up to clear up some remaining confusion after rust-lang/rust#163332 cc @Mark-Simulacrum r? clarfonthey
Rollup of 13 pull requests Successful merges: - rust-lang/rust#160695 (Detect bad number of generics caused by bad derive) - rust-lang/rust#162883 (No longer auto-inline `f16` and `f128` on well-supported platforms) - rust-lang/rust#163133 ([rustdoc] Fix invalid jump to def link when `#[rustc_allow_incoherent_impl]` is involved) - rust-lang/rust#163215 (Fix suggestion for Option to bool with proper precedence handling) - rust-lang/rust#163266 (More deferred liveness cleanups) - rust-lang/rust#163274 (Support -Z merge-functions with gcc and add stack-protector asm tests) - rust-lang/rust#163290 (Merge `ForceWarning`/`Allow`/`Expect` into `Warning`) - rust-lang/rust#163298 (Don't emit sugg for pub-reexport extern std) - rust-lang/rust#163312 (Add rustdoc regression test for glob import of a crate that re-exports) - rust-lang/rust#163357 (Borrowck cleanups part trois) - rust-lang/rust#161710 (Stabilize `mem::conjure_zst`) - rust-lang/rust#163332 (Add some docs to `Global`) - rust-lang/rust#163362 (simplify ndk compiler test)
r? libs