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

Rollup of 6 pull requests #41129

Closed
wants to merge 23 commits into from
Closed

Rollup of 6 pull requests #41129

wants to merge 23 commits into from

Conversation

GAJaloyan and others added 23 commits March 24, 2017 17:47
Correcting the two mistakes in the README.md
copy_pointer -> copy_borrowed_ptr
With `-mcpu=power4`, code might use instructions like `fcfid`, excluding
older CPUs like the PowerPC G4, which apparently some users would like
to use.  The generic `-mcpu=powerpc` should stick to pure 32-bit PowerPC
instructions.

Fixes rust-lang/cargo#3852.
similar to GCC's __attribute((used))__. This attribute prevents LLVM from
optimizing away a non-exported symbol, within a compilation unit (object file),
when there are no references to it.

This is better explained with an example:

```
#[used]
static LIVE: i32 = 0;

static REFERENCED: i32 = 0;

static DEAD: i32 = 0;

fn internal() {}

pub fn exported() -> &'static i32 {
    &REFERENCED
}
```

Without optimizations, LLVM pretty much preserves all the static variables and
functions within the compilation unit.

```
$ rustc --crate-type=lib --emit=obj symbols.rs && nm -C symbols.o
0000000000000000 t drop::h1be0f8f27a2ba94a
0000000000000000 r symbols::REFERENCED::hb3bdfd46050bc84c
0000000000000000 r symbols::DEAD::hc2ea8f9bd06f380b
0000000000000000 r symbols::LIVE::h0970cf9889edb56e
0000000000000000 T symbols::exported::h6f096c2b1fc292b2
0000000000000000 t symbols::internal::h0ac1aadbc1e3a494
```

With optimizations, LLVM will drop dead code. Here `internal` is dropped because
it's not a exported function/symbol (i.e. not `pub`lic). `DEAD` is dropped for
the same reason. `REFERENCED` is preserved, even though it's not exported,
because it's referenced by the `exported` function. Finally, `LIVE` survives
because of the `#[used]` attribute even though it's not exported or referenced.

```
$ rustc --crate-type=lib -C opt-level=3 --emit=obj symbols.rs && nm -C symbols.o
0000000000000000 r symbols::REFERENCED::hb3bdfd46050bc84c
0000000000000000 r symbols::LIVE::h0970cf9889edb56e
0000000000000000 T symbols::exported::h6f096c2b1fc292b2
```

Note that the linker knows nothing about `#[used]` and will drop `LIVE`
because no other object references to it.

```
$ echo 'fn main() {}' >> symbols.rs
$ rustc symbols.rs && nm -C symbols | grep LIVE
```

At this time, `#[used]` only works on `static` variables.
to match the type signature of the llvm.used variable
it's not related to this feature
as it's specific to ELF and won't pass on macOS / Windows
(For an explanation of what this feature does, read the commit message)

I'd like to propose landing this as an experimental feature (experimental as in:
no clear stabilization path -- like `asm!`, `#[linkage]`) as it's low
maintenance (I think) and relevant to the "Usage in resource-constrained
environments" exploration area.

The main use case I see is running code before `main`. This could be used, for
instance, to cheaply initialize an allocator before `main` where the alternative
is to use `lazy_static` to initialize the allocator on its first use which it's
more expensive (atomics) and doesn't work on ARM Cortex-M0 microcontrollers (no
`AtomicUsize` on that platform)

Here's a `std` example of that:

``` rust

unsafe extern "C" fn before_main_1() {
    println!("Hello");
}

unsafe extern "C" fn before_main_2() {
    println!("World");
}

static INIT_ARRAY: [unsafe extern "C" fn(); 2] = [before_main_1, before_main_2];

fn main() {
    println!("Goodbye");
}
```

```
$ rustc -C lto -C opt-level=3 before_main.rs
$ ./before_main
Hello
World
Goodbye
```

In general, this pattern could be used to let *dependencies* run code before
`main` (which sounds like it could go very wrong in some cases). There are
probably other use cases; I hope that the people I have cc-ed can comment on
those.

Note that I'm personally unsure if the above pattern is something we want to
promote / allow and that's why I'm proposing this feature as experimental. If
this leads to more footguns than benefits then we can just axe the feature.

cc @nikomatsakis ^ I know you have some thoughts on having a process for
experimental features though I'm fine with writing an RFC before landing this.

- `dead_code` lint will have to be updated to special case `#[used]` symbols.

- Should we extend `#[used]` to work on non-generic functions?

cc rust-lang/rfcs#1002
cc rust-lang/rfcs#1459
cc @dpc @JinShil
Correcting mistakes in the README.md

Correcting the two mistakes in the README.md (issue rust-lang#40793)
Only use cargo-vendor if building from git sources

The only time we need to vendor sources is when building from git.  If one is
building from a rustc source tarball, everything should already be in place.
This also matters for distros which do offline builds, as they can't install
cargo-vendor this way.

This adds a common `Build::src_is_git` flag, and then uses it in the dist-src
target to decide whether to install or use `cargo-vendor` at all.

Fixes rust-lang#41042.
cstore: return an immutable borrow from `visible_parent_map`

This prevents an ICE when `visible_parent_map` is called multiple times, for example when an item referenced in an impl signature is imported from an  `extern crate` statement occurs within an impl.

Fixes rust-lang#41053.

r? @eddyb
… r=alexcrichton

Re-enable appveyor cache

After breaking the queue last time, I'm cautiously back with a PR to re-enable caching on appveyor.

If you look at https://ci.appveyor.com/project/rust-lang/rust/build/1.0.2623/job/46o90by4ari6gege (one of the multiple runs that started failed, there are actually two errors - one for restoring the cache, one right at the bottom for creating a directory. I only noticed the restore error at the time as I was a bit rushed to revert and didn't stop to wonder why it continued - turns out appveyor [does not abort on cache restore failure](appveyor/ci#723).

Turns out the cause of the build failures was the cache directory existing and me being thinking that because mkdir on windows is [recursive by default](http://stackoverflow.com/a/905239/2352259), it ignores the error if the directory already exists. Apparently this is not true, so now it checks if the directory exists before attempting to create.

In addition, I've added some more paranoia to double check everything is sane.
…hton

dist-powerpc-linux: use a pure 32-bit CPU profile

With `-mcpu=power4`, code might use instructions like `fcfid`, excluding
older CPUs like the PowerPC G4, which apparently some users would like
to use.  The generic `-mcpu=powerpc` should stick to pure 32-bit PowerPC
instructions.

Fixes rust-lang/cargo#3852.
@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@frewsxcv
Copy link
Member Author

frewsxcv commented Apr 7, 2017

@bors r+ p=10

@bors
Copy link
Contributor

bors commented Apr 7, 2017

📌 Commit 9b8e7f0 has been approved by frewsxcv

@bors
Copy link
Contributor

bors commented Apr 7, 2017

⌛ Testing commit 9b8e7f0 with merge f4e43d8...

@bors
Copy link
Contributor

bors commented Apr 7, 2017

💔 Test failed - status-travis

@frewsxcv
Copy link
Member Author

frewsxcv commented Apr 7, 2017

@bors retry #40417

@bors
Copy link
Contributor

bors commented Apr 7, 2017

⌛ Testing commit 9b8e7f0 with merge 88ba18e...

@frewsxcv
Copy link
Member Author

frewsxcv commented Apr 7, 2017

@bors retry stopping because it will fall

@bors
Copy link
Contributor

bors commented Apr 7, 2017

⌛ Testing commit 9b8e7f0 with merge 4c038c0...

@frewsxcv
Copy link
Member Author

frewsxcv commented Apr 7, 2017

@bors r-

@frewsxcv
Copy link
Member Author

frewsxcv commented Apr 7, 2017

@bors retry

@frewsxcv frewsxcv closed this Apr 7, 2017
@Centril Centril added the rollup A PR which is a rollup label Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants