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

Conversation

Projects
None yet
@SimonSapin
Copy link
Contributor

SimonSapin commented Jun 19, 2018

Existing tracking issue: rust-lang/rust#27783

HTML view

@SimonSapin SimonSapin force-pushed the SimonSapin:liballoc branch from be28193 to eca6b28 Jun 19, 2018

@KodrAus

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Jun 20, 2018

@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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Jun 20, 2018

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

This comment has been minimized.

Copy link
Contributor

Ericson2314 commented Jun 21, 2018

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

This comment has been minimized.

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

This comment has been minimized.

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:

  • Those APIs are still unstable (tracked at rust-lang/rust#51245), all that PR really commits to is that alloc has some "default" OOM handling when used without std and users don’t need to provide it.
  • Fallible collection allocation is also being pursued (rust-lang/rust#48043), and not at all in opposition to OOM hooks. We can have both.
  • And hooks can still be useful for e.g. crash reporting in Firefox for all those other allocations that do not have a dedicated failure code path.

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

This comment has been minimized.

Copy link
Member

joshtriplett commented Jun 21, 2018

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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Member

joshtriplett commented Jun 21, 2018

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

@SimonSapin

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

Ericson2314 commented Jun 22, 2018

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

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Jun 22, 2018

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

SimonSapin 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.

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

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Jun 22, 2018

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

I pushed a commit that rewrite that paragraph.

@Nemo157

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Member

eddyb commented Jun 22, 2018

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

@johnthagen

This comment has been minimized.

Copy link

johnthagen commented Mar 15, 2019

Would this feature allow the use of Vec<T> on platforms like bare-metal ARM?

When evaluating using Rust for an embedded project at work, not having dynamic containers was a big negative at the time compared with C++11/14 where the gcc-arm-embedded toolchain had versions of std::string and std::vector<T>. (C++ has it's own set of drawbacks, like exception-based error handling APIs, but in this one, fundamental case, it had a leg up).

If this feature enables that kind of subset of std to be available, it would make Rust much more competitive in that space (beyond all of the other great reasons to pick Rust in the first place).


Note how the guide currently uses heapless::Vec, which is fixed size.

...
use heapless::{consts, Vec};

#[entry]
fn main() -> ! {
    let (usart1, mono_timer, itm) = aux11::init();

    // A buffer with 32 bytes of capacity
    let mut buffer: Vec<u8, consts::U32> = Vec::new();
    ...
@tarcieri

This comment has been minimized.

Copy link

tarcieri commented Mar 15, 2019

@johnthagen yes! And more than that, it allows critical infrastructure crates which are built on Vec to support no_std platforms, e.g. the bytes crate. As soon as this lands on stable (🤞), I hope to revive this PR to make the bytes crate work on no_std:

carllerche/bytes#153

This is particularly interesting for things like RTOS environments which provide a heap but little else.

@SimonSapin

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Mar 15, 2019

@johnthagen Yes, if the application picks an allocator library and configures it with the #[global_allocator] attribute. The Embedded Working Group might have library recommendations.

@Ericson2314

This comment has been minimized.

Copy link
Contributor

Ericson2314 commented Mar 15, 2019

I commented in carllerche/bytes#153 on what I see is the the circular logic behind needing stabilization. Let me reiterate here. It is stated that alloc hasn't changed much so we should stabilize it. But then it is complained that alloc changes a lot so it's too hard to support cargo features. The truth is the items in alloc do not change a lot in incompatible ways---even I believe that---so there should be no practical burden to crates supporting "alloc" vs "std" features today. [It's not like after this RFC those features could be combined or something, there's reasons to opt into std support other than avoiding unstable code.]

Even the specific case I put forth as reason not to stabilize (as alloc params, use of hashbrown, fallibility, #2492 (now also a long-open RFC...), etc shake out, we should break apart crates to separate concerns) shouldn't cause library maintainers much pain. Most people make a fake mod std { pub use ...; } anyways, and all you have to do is change a handful of reexports inside it.

@BurntSushi

This comment has been minimized.

Copy link
Member

BurntSushi commented Mar 15, 2019

I personally won't be adding alloc-only features to any of the more popular crates I maintain until it stabilizes. I've dealt with nightly only features in the past, and generally my experience there is a net negative due to (reasonable!) breaking changes and otherwise targeting stable Rust. I make the occasional exception or two, depending, but alloc-only stuff can be a fairly invasive change to the code and can also require API design. I only want to do that once.

@tarcieri

This comment has been minimized.

Copy link

tarcieri commented Mar 15, 2019

@Ericson2314 regarding the reasons for not merging carllerche/bytes#153

so there should be no practical burden to crates supporting "alloc" vs "std" features today.

no_std support needs to be tested in CI (many contributors probably won't even realize it's there) as it's very easy to break if the tests for it aren't being run locally. However, testing the alloc-related no_std features requires nightly.

The question is: do you block on failed nightly builds or not? The problem is there will be a lot of them, and if you do choose to block on nightly, now the project maintainer is tasked with determining whether the nightly failures are related to the change or not.

If you don't block on the (nightly-dependent) tests for the alloc-related features, in my experience these sorts of tricky no_std bindings will bitrot quite quickly, not because the alloc API is changing (of course that's about to happen with alloc::prelude::v1), but because other contributions weren't properly testing that no_std support still works.

I entirely sympathize with project maintainers who don't want to deal with nightly, especially in core infrastructure projects like bytes.

So what was my solution? I forked bytes and then forked prost. These were both things that were originally PRs I wanted to get upstream. The Prost author was willing, but bytes was the dependency.

I'd like to make it clear this is a significant impediment to the sorts of projects I work on. I'm a bit frustrated in that the counterargument seems to be bikeshedding the details of specific proposed solutions, when really it seems like the point is that there are any number of ways extern crate alloc could be eliminated in the future, even if no one has put forward a perfect proposal to do so yet.

kennytm added a commit to kennytm/rust that referenced this pull request Mar 16, 2019

Rollup merge of rust-lang#58933 - SimonSapin:alloc-prelude-v1, r=Amanieu
Move alloc::prelude::* to alloc::prelude::v1, make alloc a subset of std

This was one of the unresolved questions of rust-lang/rfcs#2480. As the RFC says this is maybe not useful in the sense that we are unlikely to ever have a second version, but making the crate a true subset makes one less issue to think about if we stabilize it and later want to merge standard library crates and have Cargo feature flags to enable or disable parts of the `std` crate.

See also discussion in rust-lang#58175.

Also rename the feature gate and point to a dedicated tracking issue: rust-lang#58935

@phil-opp phil-opp referenced this pull request Mar 17, 2019

Open

`std` Aware Cargo #2663

@rfcbot

This comment has been minimized.

Copy link

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.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 21, 2019

@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

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Mar 21, 2019

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

@eddyb

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Mar 21, 2019

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

This comment has been minimized.

Copy link
Contributor Author

SimonSapin commented Apr 3, 2019

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 SimonSapin:liballoc branch Apr 3, 2019

SimonSapin added a commit to SimonSapin/rust that referenced this pull request Apr 3, 2019

SimonSapin added a commit to SimonSapin/rust that referenced this pull request Apr 4, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.