Skip to content

Conversation

Centril
Copy link
Contributor

@Centril Centril commented May 9, 2019

@pietroalbini Remember to look at #60672 (comment).


Successful merges:

Failed merges:

r? @ghost

SimonSapin and others added 10 commits May 7, 2019 13:57
This is similar to `NonNull::cast`.

Compared to the `as` operator (which has a wide range of meanings
depending on the input and output types), a call to this method:

* Can only go from a raw pointer to a raw pointer
* Cannot change the pointer’s `const`ness

… even when the pointed types are inferred based on context.
Add a `cast` method to raw pointers.

This is similar to `NonNull::cast`.

Compared to the `as` operator (which has a wide range of meanings depending on the input and output types), a call to this method:

* Can only go from a raw pointer to a raw pointer
* Cannot change the pointer’s `const`ness

… even when the pointed types are inferred based on context.
pin: make the to-module link more visible

Cc @gnzlbg
…ister

cleanup: Remove `DefIndexAddressSpace`

The scheme with two address spaces for `DefIndex` was needed in the past, but apparently not needed anymore (after removing `DefId`s from locals and `HirId`-ification).
…lice, r=sfackler

Inline some Cursor calls for slices

(Partially) brings back rust-lang#33921

I've noticed in some serialization code I was writing that writes to slices produce much, much, worse code than you'd expect even with optimizations turned on. For example, you'd expect something like this to be zero cost:

```
use std::io::{self, Cursor, Write};

pub fn serialize((a, b): (u64, u64)) -> [u8;8+8] {
    let mut r = [0u8;16];
    {
        let mut w = Cursor::new(&mut r[..]);

        w.write(&a.to_le_bytes()).unwrap();
        w.write(&b.to_le_bytes()).unwrap();
    }
    r
}
```

...but it compiles down to [dozens of instructions](https://rust.godbolt.org/z/bdwDzb) because the `slice_write()` calls aren't inlined, which in turn means `unwrap()` can't be optimized away, and so on.

To be clear, this pull-req isn't sufficient by itself: if we want to go down that path we also need to add `#[inline]`'s to the default implementations for functions like `write_all()` in the `Write` trait and so on, or implement them separately in the `Cursor` impls. But I figured I'd start a conversation about what tradeoffs we're expecting here.
…apin

Stabilize and re-export core::array in std

Fixes rust-lang#60014
@Centril
Copy link
Contributor Author

Centril commented May 9, 2019

@bors r+ p=5 rollup=never

@bors
Copy link
Collaborator

bors commented May 9, 2019

📌 Commit e40f9a6 has been approved by Centril

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label May 9, 2019
@bors
Copy link
Collaborator

bors commented May 9, 2019

⌛ Testing commit e40f9a6 with merge a784a80...

bors added a commit that referenced this pull request May 9, 2019
Rollup of 5 pull requests

Successful merges:

 - #60601 (Add a `cast` method to raw pointers.)
 - #60638 (pin: make the to-module link more visible)
 - #60647 (cleanup: Remove `DefIndexAddressSpace`)
 - #60656 (Inline some Cursor calls for slices)
 - #60657 (Stabilize and re-export core::array in std)

Failed merges:

r? @ghost
@fortanix-bot
Copy link

Build failed for target x86_64-fortanix-unknown-sgx - status

cc @jethrogb

@jethrogb
Copy link
Contributor

jethrogb commented May 9, 2019

Due to #60657, looks like main CI is going to fail as well.

@jethrogb
Copy link
Contributor

jethrogb commented May 9, 2019

Except dist-various-2 just passed...?

@jethrogb
Copy link
Contributor

jethrogb commented May 9, 2019

Oh the use is only in cfg(test) code

@bors
Copy link
Collaborator

bors commented May 9, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Centril
Pushing a784a80 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 9, 2019
@bors bors merged commit e40f9a6 into rust-lang:master May 9, 2019
@rust-highfive
Copy link
Contributor

📣 Toolstate changed by #60672!

Tested on commit a784a80.
Direct link to PR: #60672

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request May 9, 2019
Tested on commit rust-lang/rust@a784a80.
Direct link to PR: <rust-lang/rust#60672>

💔 rls on windows: test-pass → test-fail (cc @Xanewok, @rust-lang/infra).
@Centril Centril deleted the rollup-fhcx463 branch May 9, 2019 21:52
@Centril Centril added beta-accepted Accepted for backporting to the compiler in the beta channel. beta-nominated Nominated for backporting to the compiler in the beta channel. labels May 9, 2019
@Centril
Copy link
Contributor Author

Centril commented May 9, 2019

Beta accepted specifically 028e78d.

@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 14, 2019
@Centril Centril added the rollup A PR which is a rollup label Oct 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.