-
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
Coherent design of GlobalAlloc and Alloc #21
Comments
I think that Now that we making progress with For example, right now one has to implement both traits for most allocators, which is inconvenient, and the type signatures aren't quite the same. It would be better to be able to avoid that in the future, if possible, because the amount of context that one has to keep in mind and document about these traits is quite large, and having to duplicate it, or even worse, diverge in subtle ways, would be bad for users. Ideally, at some point, one would just implement For this to work we might need to keep Even if we are OK with always having two traits, and with these traits diverging, this isn't necessarily something that we want to commit to in an MVP for I don't know how much it would cost to avoid being in a situation where |
I don't expect that there will come a point where So the only thing implementors of both traits have in common is the ability to request and release memory. They cannot and should not be merged IMO. |
I think we have a couple of hard constraints:
Beyond this, while it would be nice to avoid inconsistencies between the two traits (which are otherwise similar) other goals might take priority. For example, I don’t understand what “an MVP for |
AFAICT we can add new trait methods, associated types, consts, etc. in a backwards-compatible way by giving them defaults. I don't think it's worth it to block So for me an MVP is the smallest possible stabilization we could make that delivers the most value, and is forward-compatible with adding more things later. This requires knowing in which direction things should go, as opposed to having all details figured out before stabilizing anything.
I am not sure how this would work. For example, I would prefer if, instead, we either added an I don't know why we would want, by design, to have two 99% identical traits, with slightly different types in the APIs but otherwise semantically identical modulo documentation bugs, or design oversights or errors. |
Ok, so an actual blanket impl may not be desirable. But my point remains that we need |
We have When a user sets it as the The problem is that the implementation of I don't think there is a way for users to override the impl of Are there any other solutions to this problem? I worry that we'd end up with two traits that are 99.999% identical, except for some subtle differences, and that most users need to end up having to know both traits well and watch out for their subtle differences, because most users want to make their |
I also worry that some default methods exposed by |
Independently of Back on topic, I’m starting to see your point. I’m still undecided on whether if would be worth the transition, but I think it would be possible to change the language rules from:
To:
…and have a blanket In this world you would implement only The way
|
(Err, I didn’t mean to send this just yet.) The way (The current implementation of that last injection step also uses LLVM’s API directly and so the signature of all of these symbols is limited to types that are primitive to LLVM. For example, callers of |
IIRC this was the main reason we don't use "fancier" types (e.g.
Here, if That is, the For this to work, we need to make sure when designing |
No, this was unrelated to implementation details: rust-lang/rust#51160 (comment)
Yes, and we already have a mechanism for such conversions here, here, and here. We technically can carry |
I think we could also require conversion from/to trait Alloc {
type Err: From<std::alloc::AllocErr> = std::alloc::AllocErr;
} or similar bounds, and have the compiler invoke the conversions in the shim, but this essentially means that one can only propagate as much information as |
I think that wouldn’t work. If |
Do we already have a concrete proposal at this time? Taking into consideration the latest progress I guess we want
|
I think many concerns regarding this has been resolved as we decided not to have an associated error type and drop the |
I discovered, that we cannot implement |
i wouldn't allow two implementations and pick a favorite if you have both. Rather, just say that |
I think the main goal is to provide backwards compatibility with a stable type ( |
As |
I feel we should first come to a consensus on what we want to do about |
Is this related to |
Why not?
That is not possible. Compiling with #[global_allocator]
static ALLOCATOR: std::alloc::System = std::alloc::System; becomes: static ALLOCATOR: std::alloc::System = std::alloc::System;
const _: () = {
#[rustc_std_internal_symbol]
unsafe fn __rg_alloc(arg0: usize, arg1: usize) -> *mut u8 {
::core::alloc::GlobalAlloc::alloc(
&ALLOCATOR,
::core::alloc::Layout::from_size_align_unchecked(arg0, arg1),
) as *mut u8
}
#[rustc_std_internal_symbol]
unsafe fn __rg_dealloc(arg0: *mut u8, arg1: usize, arg2: usize) -> () {
::core::alloc::GlobalAlloc::dealloc(
&ALLOCATOR,
arg0 as *mut u8,
::core::alloc::Layout::from_size_align_unchecked(arg1, arg2),
)
}
#[rustc_std_internal_symbol]
unsafe fn __rg_realloc(arg0: *mut u8, arg1: usize, arg2: usize, arg3: usize) -> *mut u8 {
::core::alloc::GlobalAlloc::realloc(
&ALLOCATOR,
arg0 as *mut u8,
::core::alloc::Layout::from_size_align_unchecked(arg1, arg2),
arg3,
) as *mut u8
}
#[rustc_std_internal_symbol]
unsafe fn __rg_alloc_zeroed(arg0: usize, arg1: usize) -> *mut u8 {
::core::alloc::GlobalAlloc::alloc_zeroed(
&ALLOCATOR,
::core::alloc::Layout::from_size_align_unchecked(arg0, arg1),
) as *mut u8
}
}; |
Ok, I tried it and got errors like |
Right now,
Alloc
andGlobalAlloc
are similar, but not quite, and depending on how we evolveAlloc
, they could diverge even more. This is not necessarily bad, but it should happen by design, and not by accident. In particular, forGlobalAlloc
, how we currently do linking imposes some constraints on, e.g., whether the trait can have generic methods, etc.So I think it is important to have at least some level on consensus on where do we want to go with these two traits, and how do they fit with each other, such that we get a cohesive design.
I'm opening this issue to track that, and will leave my opinion as a comment.
The text was updated successfully, but these errors were encountered: