Skip to content

Rollup of 4 pull requests #85335

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 9 commits into from
May 15, 2021
Merged

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Soveu and others added 9 commits April 30, 2021 16:13
str::is_char_boundary - slight optimization

Current `str::is_char_boundary` implementation emits slightly more instructions, because it includes an additional branch for `index == s.len()`
```rust
pub fn is_char_boundary(s: &str, index: usize) -> bool {
    if index == 0 || index == s.len() {
        return true;
    }
    match s.as_bytes().get(index) {
        None => false,
        Some(&b) => (b as i8) >= -0x40,
    }
}
```
Just changing the place of `index == s.len()` merges it with `index < s.len()` from `s.as_bytes().get(index)`
```rust
pub fn is_char_boundary2(s: &str, index: usize) -> bool {
    if index == 0 {
        return true;
    }

    match s.as_bytes().get(index) {
        // For some reason, LLVM likes this comparison here more
        None => index == s.len(),
        // This is bit magic equivalent to: b < 128 || b >= 192
        Some(&b) => (b as i8) >= -0x40,
    }
}
```
This one has better codegen on every platform, except powerpc
<details><summary>x86 codegen</summary>
<p>

```nasm
example::is_char_boundary:
        mov     al, 1
        test    rdx, rdx
        je      .LBB0_5
        cmp     rsi, rdx
        je      .LBB0_5
        cmp     rsi, rdx
        jbe     .LBB0_3
        cmp     byte ptr [rdi + rdx], -65
        setg    al
.LBB0_5:
        ret
.LBB0_3:
        xor     eax, eax
        ret

example::is_char_boundary2:
        test    rdx, rdx
        je      .LBB1_1
        cmp     rsi, rdx
        jbe     .LBB1_4
        cmp     byte ptr [rdi + rdx], -65
        setg    al
        ret
.LBB1_1:  ; technically this branch is the same as LBB1_4
        mov     al, 1
        ret
.LBB1_4:
        sete    al
        ret
 ```
</p>
</details>

<details><summary>aarch64 codegen</summary>
<p>

```as
example::is_char_boundary:
        mov     x8, x0
        mov     w0, #1
        cbz     x2, .LBB0_4
        cmp     x1, x2
        b.eq    .LBB0_4
        b.ls    .LBB0_5
        ldrsb   w8, [x8, x2]
        cmn     w8, rust-lang#65
        cset    w0, gt
.LBB0_4:
        ret
.LBB0_5:
        mov     w0, wzr
        ret

example::is_char_boundary2:
        cbz     x2, .LBB1_3
        cmp     x1, x2
        b.ls    .LBB1_4
        ldrsb   w8, [x0, x2]
        cmn     w8, rust-lang#65
        cset    w0, gt
        ret
.LBB1_3:
        mov     w0, #1
        ret
.LBB1_4:
        cset    w0, eq
        ret
```

</p>
</details>

<details><summary>riscv64gc codegen</summary>
<p>

example::is_char_boundary:
        seqz    a3, a2
        xor     a4, a1, a2
        seqz    a4, a4
        or      a4, a4, a3
        addi    a3, zero, 1
        bnez    a4, .LBB0_3
        bgeu    a2, a1, .LBB0_4
        add     a0, a0, a2
        lb      a0, 0(a0)
        addi    a1, zero, -65
        slt     a3, a1, a0
.LBB0_3:
        mv      a0, a3
        ret
.LBB0_4:
        mv      a0, zero
        ret

example::is_char_boundary2:
        beqz    a2, .LBB1_3
        bgeu    a2, a1, .LBB1_4
        add     a0, a0, a2
        lb      a0, 0(a0)
        addi    a1, zero, -65
        slt     a0, a1, a0
        ret
.LBB1_3:
        addi    a0, zero, 1
        ret
.LBB1_4:
        xor     a0, a1, a2
        seqz    a0, a0
        ret

</p>
</details>

[Link to godbolt](https://godbolt.org/z/K8avEz8Gr)

`@rustbot` label: A-codegen
…cs-than-necessary, r=Mark-Simulacrum

Generate not more docs than necessary

This is something that `@Nemo157` was talking about: they wanted that when using `x.py doc std`, it only generated `std` (and the crates "before" it).

r? `@Mark-Simulacrum`
Warn about unused `pub` fields in non-`pub` structs

This pull request fixes rust-lang#85255. The current implementation of dead code analysis is too prudent because it marks all `pub` fields of structs as live, even though they cannot be accessed from outside of the current crate if the struct itself only has restricted or private visibility.

I have changed this behavior to take the containing struct's visibility into account when looking at field visibility and liveness. This also makes dead code warnings more consistent; consider the example given in rust-lang#85255:
```rust
struct Foo {
    a: i32,
    pub b: i32,
}

struct Bar;

impl Bar {
    fn a(&self) -> i32 { 5 }
    pub fn b(&self) -> i32 { 6 }
}

fn main() {
    let _ = Foo { a: 1, b: 2 };
    let _ = Bar;
}
```
Current nightly already warns about `Bar::b()`, even though it is `pub` (but `Bar` is not). It should therefore also warn about `Foo::b`, which it does with the changes in this PR.
@rustbot rustbot added the rollup A PR which is a rollup label May 15, 2021
@GuillaumeGomez
Copy link
Member Author

@bors: r+ p=4 rollup=never

@bors
Copy link
Collaborator

bors commented May 15, 2021

📌 Commit 8ea8252 has been approved by GuillaumeGomez

@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 15, 2021
@bors
Copy link
Collaborator

bors commented May 15, 2021

⌛ Testing commit 8ea8252 with merge 50f2bf6...

@bors
Copy link
Collaborator

bors commented May 15, 2021

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 50f2bf6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 15, 2021
@bors bors merged commit 50f2bf6 into rust-lang:master May 15, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 15, 2021
@GuillaumeGomez GuillaumeGomez deleted the rollup-0tvc14g branch May 15, 2021 20:58
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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.

6 participants