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

Remove the `alloc_system` crate #55660

Merged
merged 2 commits into from Nov 11, 2018

Conversation

Projects
None yet
10 participants
@alexcrichton
Member

alexcrichton commented Nov 3, 2018

In what's hopefully one of the final nails in the coffin of the "old allocator story of yore" this PR deletes the alloc_system crate and all traces of it from the compiler. The compiler no longer needs to inject allocator crates anywhere and the alloc_system crate has no real reason to exist outside the standard library.

The unstable alloc_system crate is folded directly into the standard library where its stable interface, the System type, remains the same. All unstable traces of alloc_system are removed, however.

@rust-highfive

This comment has been minimized.

Collaborator

rust-highfive commented Nov 3, 2018

r? @dtolnay

(rust_highfive has picked a reviewer for you, use r? to override)

use alloc_system::System;
#[global_allocator]
static ALLOC: System = System;

This comment has been minimized.

@alexcrichton

alexcrichton Nov 3, 2018

Member

Note that this and the other sanitizer crates no longer link to System as it's not an available type to use. While a regression I think this is minor enough to allow because the default allocator is now System (not jemalloc) which all the sanitizers are compatible that. That, plus the fact that the sanitizers are unstable

/// This type can be used in a `static` item
/// with the `#[global_allocator]` attribute
/// to force the global allocator to be the system’s one.
/// (The default is jemalloc for executables, on some platforms.)

This comment has been minimized.

@kevinmehall

kevinmehall Nov 3, 2018

Contributor

Do you want to update this doc comment while you're moving it?

@dtolnay

This comment has been minimized.

Member

dtolnay commented Nov 3, 2018

Looks good to me but let's run this by @SimonSapin as well:

r? @SimonSapin

@rust-highfive rust-highfive assigned SimonSapin and unassigned dtolnay Nov 3, 2018

@SimonSapin

This comment has been minimized.

Contributor

SimonSapin commented Nov 4, 2018

Looks good to me as well, with the doc-comment for System updated as @kevinmehall pointed out.

It probably doesn’t make sense anymore to show a #[global_allocator] static A: System = System; example. Maybe replace it with an example that uses GlobalAlloc::alloc?

self.sess.allocator_kind.set(None);
return
}
// At this point we've determined that we need an allocator. Let's see
// if our compilation session actually needs an allocator based on what
// we're emitting.
let mut need_lib_alloc = false;
let mut need_exe_alloc = false;
let mut all_rlib = true;
for ct in self.sess.crate_types.borrow().iter() {

This comment has been minimized.

@mattico

mattico Nov 4, 2018

Contributor

Nit: this could be let all_rlib = self.sess.crate_types.borrow().iter().all(|ct| *ct == config::CrateType::Rlib)

@alexcrichton alexcrichton force-pushed the alexcrichton:cleanup-alloc-system branch from 80e88cf to 7fa5adf Nov 5, 2018

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Nov 5, 2018

@bors: r=dtolnay,SimonSapin

@bors

This comment has been minimized.

Contributor

bors commented Nov 5, 2018

📌 Commit 7fa5adf has been approved by dtolnay,SimonSapin

@bors

This comment has been minimized.

Contributor

bors commented Nov 6, 2018

☔️ The latest upstream changes (presumably #55518) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton alexcrichton force-pushed the alexcrichton:cleanup-alloc-system branch from 7fa5adf to 1116d6e Nov 6, 2018

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Nov 6, 2018

@bors: r=dtolnay,SimonSapin

@bors

This comment has been minimized.

Contributor

bors commented Nov 6, 2018

📌 Commit 1116d6e has been approved by dtolnay,SimonSapin

@sfackler

This comment has been minimized.

Member

sfackler commented Nov 6, 2018

cc @frewsxcv for the sanitizer implications

@frewsxcv

This comment has been minimized.

Member

frewsxcv commented Nov 10, 2018

cc @frewsxcv for the sanitizer implications

What are the sanitizer implications of this?

@bors

This comment has been minimized.

Contributor

bors commented Nov 11, 2018

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

  1. git checkout cleanup-alloc-system (switch to your branch)
  2. git fetch upstream master (retrieve the latest master)
  3. git rebase upstream/master -p (rebase on top of it)
  4. Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
  5. git push self cleanup-alloc-system --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message
warning: Cannot merge binary files: src/Cargo.lock (HEAD vs. heads/homu-tmp)
Auto-merging src/libstd/lib.rs
Auto-merging src/librustc/session/mod.rs
Removing src/liballoc_system/lib.rs
Removing src/liballoc_system/Cargo.toml
Auto-merging src/Cargo.lock
CONFLICT (content): Merge conflict in src/Cargo.lock
Automatic merge failed; fix conflicts and then commit the result.

alexcrichton added some commits Nov 3, 2018

rustc: Clean up allocator injection logic
This commit cleans up allocator injection logic found in the compiler
around selecting the global allocator. It turns out that now that
jemalloc is gone the compiler never actually injects anything! This
means that basically everything around loading crates here and there can
be easily pruned.

This also removes the `exe_allocation_crate` option from custom target
specs as it's no longer used by the compiler anywhere.
std: Delete the `alloc_system` crate
This commit deletes the `alloc_system` crate from the standard
distribution. This unstable crate is no longer needed in the modern
stable global allocator world, but rather its functionality is folded
directly into the standard library. The standard library was already the
only stable location to access this crate, and as a result this should
not affect any stable code.

@alexcrichton alexcrichton force-pushed the alexcrichton:cleanup-alloc-system branch from 1116d6e to cc75903 Nov 11, 2018

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Nov 11, 2018

@bors: r=dtolnay,SimonSapin

@bors

This comment has been minimized.

Contributor

bors commented Nov 11, 2018

📌 Commit cc75903 has been approved by dtolnay,SimonSapin

@bors

This comment has been minimized.

Contributor

bors commented Nov 11, 2018

⌛️ Testing commit cc75903 with merge ca79ecd...

bors added a commit that referenced this pull request Nov 11, 2018

Auto merge of #55660 - alexcrichton:cleanup-alloc-system, r=dtolnay,S…
…imonSapin

Remove the `alloc_system` crate

In what's hopefully one of the final nails in the coffin of the "old allocator story of yore" this PR deletes the `alloc_system` crate and all traces of it from the compiler. The compiler no longer needs to inject allocator crates anywhere and the `alloc_system` crate has no real reason to exist outside the standard library.

The unstable `alloc_system` crate is folded directly into the standard library where its stable interface, the `System` type, remains the same. All unstable traces of `alloc_system` are removed, however.
@bors

This comment has been minimized.

Contributor

bors commented Nov 11, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: dtolnay,SimonSapin
Pushing ca79ecd to master...

@bors bors merged commit cc75903 into rust-lang:master Nov 11, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details

@alexcrichton alexcrichton deleted the alexcrichton:cleanup-alloc-system branch Nov 11, 2018

/// This is based on `malloc` on Unix platforms and `HeapAlloc` on Windows,
/// plus related functions.
///
/// This type implements the `GlobalAlloc` trait and Rust programs by deafult

This comment has been minimized.

@bjorn3

bjorn3 Nov 12, 2018

Contributor

*default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment