Skip to content
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

RFC: Stabilize the alloc crate #2480

Merged
merged 9 commits into from
Apr 3, 2019
Merged

Conversation

SimonSapin
Copy link
Contributor

@SimonSapin SimonSapin commented Jun 19, 2018

Existing tracking issue: rust-lang/rust#27783

HTML view

@SimonSapin SimonSapin added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Jun 19, 2018
@KodrAus
Copy link
Contributor

KodrAus commented Jun 20, 2018

I think having a stable alloc crate to target will be great for the ecosystem over the immediate future. A single standard library that selectively enables/disables features sounds nice over the long term as a more granular platform for describing the capabilities of a platform, but I imagine that comes with its own challenges too.

I'm definitely in favour of not blocking a reasonable concrete solution to making fewer assumptions about the runtime environment on a perfect hypothetical vision of the future 👍

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jun 20, 2018

rust-lang/rust#42774 is so close unless we do this, because of issues I don't at this moment fully recall (having mentally paged them out over the last few months) adding default type variables.

off-topic rant:

Honestly more stabilization-as-is proposals kind of annoy me; I had a fairly detailed steps on how to make progress on the more pie-in-sky goals without blocking on research-level stuff from the portability working group, but I was never able to sit down with a quorum of people that mattered and fully explain things, so nothing could be prioritized and coordinated. Instead we keep on drifting by default towards "screw facade, stick it all back in std" despite the portability group leaning in the opposite direction. And the big-std approach does require way more language-level work before portability goals have any hope of being achieved, so I consider it a non-starter. With a stalling of the portability group (and 2nd-half-of-year delay for std-aware Cargo) I just went and worked on other things, and now I poke my head back in to see stuff like this.

@SimonSapin
Copy link
Contributor Author

@Ericson2314 Could you explain that “unless”? I don’t see how having an alloc crate separate from core or std hinders in any way having an Alloc trait for generic collections.

As to your self-described off-topic rant, maybe you’re reacting to the discussion of a single-crate standard library in the RFC? That’s a possibility that may or may not happen, it is only discussed to show that this RFC doesn’t prevent it.

It is not what this RFC is proposing. In fact this RFC is moving in the opposite direction, committing to having more standard library crates.

Instead we keep on drifting by default towards "screw facade, stick it all back in std"

Beyond not factually describing this proposal, you’re making an accusation of attitude that I feel is not fair at all. I’m trying to argue for a way to make incremental improvement and to discuss alternatives, while I have yet to extract any concrete proposal from your many words written on this topic.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jun 20, 2018

[a single-crate standard library] is not what this RFC is proposing. In fact this RFC is moving in the opposite direction, committing to having more standard library crates.

Indeed we agree that alloc should be its own crate and I am glad that we do.

Could you explain that “unless”? I don’t see how having an alloc crate separate from core or std hinders in any way having an Alloc trait for generic collections.

A separate alloc is not the problem; The problem is stabilizing alloc now prevents evolution of the collections in alloc.

If I recall correctly, there are difficulties going from Collection<T> to Collection<T, A=Heap>. At the very least it requires stabilization of defaulted type parameters on nominal types, which is @eddyb's plan. Also, with that plan we only have default params on nomimal types and not aliases. Taken together, these mean:

  1. We are blocked on language work, even if the road map is there
  2. Unless road map changes, no defining collections prior to global allocator

C.f with alloc unstable, we can freely add the extra parameter now. std can just do type Foo<T> = alloc::collections::Foo<T, Heap>; or even struct Foo<T>(alloc::collections::Foo<T, Heap>);, avoiding any reliance on new language features.

Beyond not factually describing this proposal, you’re making an accusation of attitude that I feel is not fair at all.

You're right that I was not being fair. I had the hardest time explaining my preference for many little crates which is based on many separate individually small and future benefits, which is hard to string together into a single argument. We thankfully do agree that alloc should be separate, and hopefully those compatibility and blockage-avoiding concerns above are clearer / less opinionated.

while I have yet to extract any concrete proposal from your many words written on this topic.

The conversation is indeed woefully decentralized, inevitably leading to the difficulties in coordination. rust-lang/rust#42774 (comment) I mention doing the alias. rust-lang/rust#42774 (comment) I link to a (now-deleted?) post where I linked to the branch where I started generalized the collections. rust-lang-nursery/portability-wg#1 (comment), a still-existing post, I link that branch, and a preparatory PR to to rustc by @eddyb, as the 2nd of 3 easier portability initiatives to pursue in parallel.

@SimonSapin
Copy link
Contributor Author

I think there is value in alloc::foo::Bar and std::foo::Bar being the same item, if they both exist. They’re just two different paths to access the same thing, and the reason for both alloc and std to exist is orthogonal to what foo::Bar is.

So if the plan for allocator-generic collections is to make new types, I would prefer these types to have different names. Or at the very least to be in a dedicated module whose name indicates what the difference is, but not overloading the alloc/std split with this additional meaning.

We thankfully do agree that alloc should be separate

It sounds like you have no objection to this RFC as proposed, so further discussion of the Alloc trait and how it’s used in collections is probably better kept in the relevant tracking issue to avoid dispersing that discussion even more, and keep this thread on topic.

@Ericson2314
Copy link
Contributor

It sounds like you have no objection to this RFC as proposed, so f

My objection is until we actually start making allocator-polymorphic and fallible collections, we shouldn't stabilize the alloc crate.

For example rust-lang/rust#51607 (comment) I think registering a hook is a needlessly awkward interface I rather if we have ergonomic fallible collections. I also think that ergonomic fallible collections are way easier to implement than everyone else does.

I think there is value in alloc::foo::Bar and std::foo::Bar being the same item, if they both exist.

OK so the thing we really want is for the allocator polymorphic types and std types to unify. I.e. there exists some allocator such that poly_a_collection::<T, GlobalAlloc> = std::a_collection<T>; Initially, this won't be possible without the language work, but after the language work it could well be.

The library work I'm proposing is really easy if we could only get some consensus it's worth doing. [I don't really want to just crank out PRs until there's some consensus on the path should persued, but I made the linked branch to convert a few collections and show how easy it is]. I think all the collections could be converted in a month. The language work could take 2. We could definitely stabilize alloc by the end of the year, but alloc might look quite different by then. If, after we try that there is some sort of more difficult hurdle unifying the types, then and only then let's move the polymorphic allocators to a different namespace and make the alloc::collections ones specialized ones std reexports as-is.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jun 21, 2018

I guess I'm afraid this goes back to our little-libraries philosophy debate in that I want the crates beneath std to sort of look nice and stand on their own, and I deem alloc too ugly compared to where it could be with just a little effort to warrant stabilization at this time.

@SimonSapin
Copy link
Contributor Author

SimonSapin commented Jun 21, 2018

For example rust-lang/rust#51607 (comment) I think registering a hook is a needlessly awkward interface I rather if we have ergonomic fallible collections.

Again I don’t see how this is related to this RFC (stabilizing the alloc crate), and this really feels like derailing the conversation. That said:

OK so the thing we really want is for the allocator polymorphic types and std types to unify.

Also off-topic.

I think all the collections could be converted in a month.

Still off-topic, but: are you volunteering to do this work? Consider working with glandium in rust-lang/rust#50882. (Not in this thread!)

We could definitely stabilize alloc by the end of the year, but alloc might look quite different by then.
[…]
I deem alloc too ugly compared to where it could be with just a little effort to warrant stabilization at this time.

As mentioned in the RFC, everything being stabilized here already exists (re-exported) as stable under std::. The only new thing proposed by this RFC is making a subset of those already-stable std API available when std as a whole is not available for unrelated reasons. We cannot modify those APIs, so the only thing that can be “quite different” is new APIs added in the future. And that’s still totally possible with this RFC! Stabilizing a crate doesn’t mean we can’t add stuff to it later.

If you’d like to object to the RFC please concentrate on what it actually proposes compared to the current situation, and keep other things you’d also like to happen to the relevant threads.

@joshtriplett
Copy link
Member

This seems reasonable to me.

One request: in the RFC, could you add a recommendation for conventions that crates supporting builds with or without the use of alloc should follow? (Either a feature named no_alloc, or a feature named alloc, depending on defaults.)

@Nemo157
Copy link
Member

Nemo157 commented Jun 21, 2018

Either a feature named no_alloc, or a feature named alloc, depending on defaults

Please no more negative features, no_std features are a real pain to work with. Either an alloc feature, or a default-enabled alloc feature, depending on defaults. What could be really useful is a good example of how to have a crate supporting both an alloc and a std feature, where each enables slightly more functionality (at first glance having std = ["alloc"] and just always referring to items using their most “fundamental” path would cover most of what you need to do).

@SimonSapin
Copy link
Contributor Author

SimonSapin commented Jun 21, 2018

@joshtriplett The RFC already gives this example:

#![no_std]

extern crate alloc;

use alloc::vec::Vec;

One of the motivation points for the RFC is to allow this on stable Rust without using a Cargo feature at all. (Is that not clear? I can try to rephrase it.)

@Nemo157 The example I had in mind was a no_std feature that does not change the public API at all, it only opts into using the (before this RFC) unstable alloc crate in order to avoid std. In a sense, it enables the "feature" of being compatible with targets where std does not exist. But regardless, this RFC enables such libraries to use #![no_std] unconditionally.

For other libraries that can work with only core and alloc but that extend their own public API or functionality when they can also depend on the std crate, I agree that a Cargo feature should only add stuff, not remove it. (Unfortunately default features are not great because they need to be explicitly disabled by ever dependent in order to not be included, but this is getting off-topic.)

@Centril
Copy link
Contributor

Centril commented Jun 21, 2018

@SimonSapin

#![no_std]
extern crate alloc;
use alloc::vec::Vec;

I only want to say: Thank you so much for this! This will be awesome for anyone who has gone through the pain of conditional compilation of alloc feature gates.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jun 21, 2018

Still off-topic, but: are you volunteering to do this work? Consider working with glandium in rust-lang/rust#50882. (Not in this thread!)

Thanks for the link. I commented there.

The only new thing proposed by this RFC is making a subset of those already-stable std API available when std as a whole is not available for unrelated reasons.

I disagree these stabile APIs are good enough for alloc. They might be good enough for the narrower set of situations std supports, but the no_std situation is broader, and for me that raises the bar.

For example I want to be able to look a library's dependencies and know that it may use allocation but it doesn't have any weird ambient authority. Being able to set hooks and use a global allocator constitute that ambient authority. Yes the hook interface isn't stable yet, but the global alloc stuff would be: I'd want to move it out to alloc and into another create prior to stabilization. Then I know a library that just depends on alloc and not that other create can't do those other things.

Taking a quote from rust-lang/rust#50882

Ideally, all inherent methods of Box<T> could be applied to
Box<T, A: Alloc + Default>, but, as of now, that would be backwards
incompatible because it would require type annotations in places where
they currently aren't required, per #50822 (comment)

If we make std do a type alias of it, then that problem might be avoided. But it would be breaking change for that same inference reason if alloc was already stable.

Overall, the portable work requires some realm for experimenting (not even for too long!), and stabilizing every crate behind facade ties are hands. (we can make more, but the good names will be taken). I'll never understand this rush to stabilize concurrent with no clear path (to me at least) for the working group to coalesce around proposals for an official roadmap.

@joshtriplett
Copy link
Member

@Nemo157 I completely agree, I'd rather see this always done as a positive lint rather than a negative one.

@SimonSapin
Copy link
Contributor Author

SimonSapin commented Jun 21, 2018

Being able to set hooks and use a global allocator constitute that ambient authority.

Having a global allocator (and possibly OOM hooks) is everything that the alloc crate is about, in the current core v.s. alloc v.s. std split. The Alloc trait is available in core::alloc.

If/when we have something similar to Vec or Box that doesn’t (even through a default type parameter) assume the existence of a global allocator, then such types are "pure" library code that don’t assume any dependency and therefore can live in libcore. (Or whatever other crate if we decide that core should be split up.)

@SimonSapin
Copy link
Contributor Author

SimonSapin commented Jun 21, 2018

stabilizing every crate behind facade ties are hands. (we can make more, but the good names will be taken).

Is that what this is about? You’re advocating to block real use cases for months or years over a name?

@Ericson2314
Copy link
Contributor

I sort of presumed there was zero appetite to split core up and make a second facade. But I'd love to be wrong—that's a big reassurance! My mental model for stabilization was sort or bottom-up: working up carve out all the pure code and then deal with the lang items. But maybe this is better thought of as top down: strip things out of std leaving self-supporting subsets, while opening the door to future dividing-and-conquering down to pure bits.

If the preceding paragraph sounds good to you, then yes that does just leave the name. If that is "everything" this create is about, can we call it global_alloc?

@SimonSapin
Copy link
Contributor Author

I am not in favor of renaming the alloc crate − which has existed in pretty much this form for several years with no sign of something else concrete happening − to a longer name now just because of unspecific plans that we might do something else in this area some day.

In what order you would like to focus your attention on other things doesn’t seem relevant. This thread is about this RFC making this proposal at this time.


Off-topic:

My earlier comment was not in support of nor against splitting up libcore. Whether that is possible or desirable is a discussion for another thread. It was only a statement of possibility not prevented by our stability policy. I only included it in an attempt to preemptively defuse an off-topic objection I suspected would be coming from you.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Jun 22, 2018

Forget about my plans for a moment. You pointed out core::alloc exists, so allocation is already a cross-crate concern. (c.f. collections and sync were basically the sole source of their corresponding std::* modules.) Likewise alloc::alloc is kind of a weird path, but follows from the interface subset policy you espouse with the alloc:: collection PR. Renaming alloc clarifies the purpose of this crate, and avoids that odd-looking path while retaining the subset interface policy.

which has existed in pretty much this form for several year

Let's no pretend these intermediate crates haven't changed because people have been broadly happy with their existance. collections and std-unicode also hadn't changed much, and then were merged, so there's plenty of precedent of shaking things up right before the merge.

@dhardy
Copy link
Contributor

dhardy commented Jun 22, 2018

The alloc crate does not have a prelude (items that are implicitly in scope).

Would it not be possible to add a prelude and use it automatically when extern crate alloc; is specified? This would make it easier to use alloc.

PR #51569 moves them around so that the module structure matches that of std, and the public APIs become a subset

which implies that it would also be possible to make alloc a superset of core. This would allow writing extern crate alloc as std; and pretending that one is in fact using std (except that many parts of it are unavailable). I am not convinced this is a good idea however; aside from maintenance overhead, the current structure of core/alloc/std encourages users wanting to target no_std with partial functionality (like we have in Rand, like Regex is trying to do now, etc.) to import from the most appropriate module, and thus makes it clearer which parts need to be cfg-ed on feature flags.

@SimonSapin
Copy link
Contributor Author

The alloc crate does not have a prelude (items that are implicitly in scope).

Would it not be possible to add a prelude and use it automatically when extern crate alloc; is specified? This would make it easier to use alloc.

The quoted part of the RFC might be giving a misleading framing. I think it’s better to think of current prelude(s) as the default one and the #![no_std] one, rather than associated to crates. If we want to have more preludes I’d prefer a language proposal like #890 that also works with crates.io rather than something ad-hoc for alloc.

Until such a proposal is accepted, I think #![no_std] crates can live with writing use alloc::vec::Vec; explicitly.

@SimonSapin
Copy link
Contributor Author

The quoted part of the RFC might be giving a misleading framing.

I pushed a commit that rewrite that paragraph.

@Nemo157
Copy link
Member

Nemo157 commented Jun 22, 2018

Could the alloc crate have an explicit prelude. Looking at the std prelude it could be appropriate to have

pub mod prelude {
    pub mod v1 {
        pub use crate::boxed::Box;
        pub use crate::borrow::ToOwned;
        pub use crate::string::String;
        pub use crate::string::ToString;
        pub use crate::vec::Vec;
    }
}

This would hopefully be forward compatible with any implicit prelude proposal since it's a pattern used in a lot of crates now.

@eddyb
Copy link
Member

eddyb commented Jun 22, 2018

Minor note: we should probably be naming the module rust2015 cf. rust-lang/rust#51418.

@rfcbot
Copy link
Collaborator

rfcbot commented Mar 21, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Mar 21, 2019
@nikomatsakis
Copy link
Contributor

@SimonSapin I have a quick question -- perhaps @eddyb can elaborate -- but during the all hands it came up that we might want the compiler to be able to treat allocation functions as a "pure function" i.e., something the compiler could remove etc. Is there any "text" of this kind planned? (Sorry for being vague, I don't recall the details off hand.)

@nikomatsakis
Copy link
Contributor

Also I might be totally wrong and this has nothing to do with defining one's own allocators. =) In that case ignore me.

@Centril Centril removed I-nominated T-lang Relevant to the language team, which will review and decide on the RFC. labels Mar 21, 2019
@eddyb
Copy link
Member

eddyb commented Mar 21, 2019

@nikomatsakis Those were specifically for GlobalAlloc (which is already stable, regardless of what we do with the alloc crate) but we might want to also apply them to Alloc itself?

I think @alexcrichton is familiar with all the guarantees we tell LLVM about, and I suppose the question for him is "how many of those do you think we could reasonably add to Alloc as well, and how backwards-incompatible can we even be here?".

The extra things we wanted, aside from the LLVM ones, were:

  • in-place optimization, by hoisting alloc calls (i.e. doing them earlier)
    • we can't guarantee alloc will actually remain ordered with any side-effects (as it could be called sooner - or even later, but we have no optimizations planned for "sinking" alloc calls)
    • could be useful, but harder to do on non-global allocators
  • compile-time "symbolic heap" (in miri)
    • we can't guarantee alloc/dealloc pairs will actually be called (very much like what LLVM wants, in order to be able to elide allocations)
    • for non-global allocators, their implementation could be interpreted by miri directly, so there's probably no point in doing anything for them

The "we can't guarantee X" language can also be replaced with "the allocator should accept/remain safe in face of X not being upheld by the compiler".

@SimonSapin
Copy link
Contributor Author

SimonSapin commented Mar 21, 2019

@nikomatsakis I believe these questions are only tangentially relevant to this RFC (they remain the same if liballoc doesn’t exist and its content lives in libstd) but I’ll try to answer anyway.

I believe the current status is that we patch LLVM (src/llvm-project/llvm/lib/Analysis/MemoryBuiltins.cpp) to make it treat a few symbols like rust_alloc, rust_realloc, and rust_dealloc specially. Like with malloc and free, LLVM’s optimizer will sometimes elide some allocations and possibly move some calls (assume they have no "significant" side effect). We have a codegen test (src/test/codegen/alloc-optimisation.rs) to ensure that this optimization does not accidentally regress. However as far as I know we make no publicly documented promise about it.

We probably should document in the GlobalAlloc trait that calls may be moved or elided in such a way, and so impls should not rely on side effects always happening when one would expect. However I’m not sure how to phrase that exactly. Could you file a docs issue about this?

For the Alloc trait, other than its impl for the std::alloc::Global type (which goes to the same code paths as the above) as far as I know there is no such optimization today. Whether we want to add one and how it would work is a matter to discuss in rust-lang/rust#32838 or rust-lang/rust#42774.

@SimonSapin
Copy link
Contributor Author

It looks like miri intercepts and special-cases (src/tools/miri/src/fn_call.rs) calls to __rust_alloc and a few other symbols. So any #![global_allocator] is ignored for the purpose of const-eval.

@SimonSapin
Copy link
Contributor Author

The @rust-lang/libs team has decided to accept this RFC, after discussion in today’s triage meeting.

To track further progress, subscribe to the repurposed tracking issue here: rust-lang/rust#27783

Consensus was that if and when extern crate syntax is to be actively deprecated in a future edition, that deprecation should be blocked on having a replacement for the purpose of opting in or out of standard library functionality. The design of that mechanism is for another RFC to propose.

@SimonSapin SimonSapin merged commit 8884ad3 into rust-lang:master Apr 3, 2019
@SimonSapin SimonSapin deleted the liballoc branch April 3, 2019 18:14
SimonSapin added a commit to SimonSapin/rust that referenced this pull request Apr 12, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 12, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
Centril added a commit to Centril/rust that referenced this pull request Apr 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.