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 8 pull requests #118838

Merged
merged 30 commits into from
Dec 11, 2023
Merged

Rollup of 8 pull requests #118838

merged 30 commits into from
Dec 11, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

petrochenkov and others added 30 commits December 5, 2023 20:56
True || needs_par_as_let_scrutinee(...) is always true.
In all four of Break, Closure, Ret, Yeet, the needs_par_as_let_scrutinee
is guaranteed to return true because the .precedence().order() of those
expr kinds is <= AssocOp::LAnd.precedence().

The relevant functions in rustc_ast::util::parser are:

    fn needs_par_as_let_scrutinee(order: i8) -> bool {
        order <= prec_let_scrutinee_needs_par() as i8
    }

    fn prec_let_scrutinee_needs_par() -> usize {
        AssocOp::LAnd.precedence()
    }

The .precedence().order() of Closure is PREC_CLOSURE (-40) and of Break,
Ret, Yeet is PREC_JUMP (-30).

The value of AssocOp::LAnd.precedence() is 6.

So this commit causes no change in behavior, only potentially
performance by doing a redundant call to contains_exterior_struct_lit in
those four cases. This is fine because Break, Closure, Ret, Yeet should
be exceedingly rare in the position of a let scrutinee.
    tidy error: /git/rust/compiler/rustc_ast_pretty/src/pprust/state.rs:1165: unexplained "```ignore" doctest; try one:

    * make the test actually pass, by adding necessary imports and declarations, or
    * use "```text", if the code is not Rust code, or
    * use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or
    * use "```should_panic", if the code is expected to fail at run time, or
    * use "```no_run", if the code should type-check but not necessary linkable/runnable, or
    * explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided.

    tidy error: /git/rust/compiler/rustc_ast_pretty/src/pprust/state.rs:1176: unexplained "```ignore" doctest; try one:

    * make the test actually pass, by adding necessary imports and declarations, or
    * use "```text", if the code is not Rust code, or
    * use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or
    * use "```should_panic", if the code is expected to fail at run time, or
    * use "```no_run", if the code should type-check but not necessary linkable/runnable, or
    * explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided.
When making changes to the bootstrap that shouldn't change its behavior,
this feature will help developers perform comparisons to check whether the
bootstrap behavior has changed or not.

This can also be used for different purposes. For example, allowing CI to
dump the shims and upload them so that developers can download them and compare
with their local dump to see if CI affects the bootstrap unexpectedly. Or, make CI
perform comparisons on specific bootstrap tests to check for behavior changes between
the master and PR branches.

Signed-off-by: onur-ozkan <[email protected]>
Signed-off-by: onur-ozkan <[email protected]>
To ensure deterministic results we must sort the dump lines.
This is necessary because the order of rustc invocations different
almost all the time.

Signed-off-by: onur-ozkan <[email protected]>
We don't really want to communicate with target maintainers via email.
GitHub is where everything happens, people should have a GitHub account
that can be pinged on issues.

This doesn't necessarily have to be a strict rule, but edit the template
to suggest this. The previous template made it look like we care about
having an email address, which we do not.
…rrors

resolve: Use `def_kind` query to cleanup some code

Follow up to rust-lang#118188.

Similar attempts to use queries in resolver resulted in perf regressions in the past, so this needs a perf run first.
…=clubby789

dump bootstrap shims

When making changes to the bootstrap that shouldn't change its behavior, this feature will help developers perform comparisons to check whether the bootstrap behavior has changed or not. As an example, when removing Python from the bootstrap by migrating to Rust, this feature will be super useful for ensuring that the behavior remains unaffected. It will assist me in performing comparisons to verify that the bootstrap behavior and its outputs remains consistent throughout the migration process.

This can also be used for different purposes. For example, allowing CI to dump the shims and upload them so that developers can download them and compare with their local dump to see if CI affects the bootstrap unexpectedly. Or, make CI perform comparisons on specific bootstrap tests to check for behavior changes between the master and PR branches.
…errors

Do not parenthesize exterior struct lit inside match guards

Before this PR, the AST pretty-printer injects parentheses around expressions any time parens _could_ be needed depending on what else is in the code that surrounds that expression. But the pretty-printer did not pass around enough context to understand whether parentheses really _are_ needed on any particular expression. As a consequence, there are false positives where unneeded parentheses are being inserted.

Example:

```rust
#![feature(if_let_guard)]

macro_rules! pp {
    ($e:expr) => {
        stringify!($e)
    };
}

fn main() {
    println!("{}", pp!(match () { () if let _ = Struct {} => {} }));
}
```

**Before:**

```console
match () { () if let _ = (Struct {}) => {} }
```

**After:**

```console
match () { () if let _ = Struct {} => {} }
```

This PR introduces a bit of state that is passed across various expression printing methods to help understand accurately whether particular situations require parentheses injected by the pretty printer, and it fixes one such false positive involving match guards as shown above.

There are other parenthesization false positive cases not fixed by this PR. I intend to address these in follow-up PRs. For example here is one: the expression `{ let _ = match x {} + 1; }` is pretty-printed as `{ let _ = (match x {}) + 1; }` despite there being no reason for parentheses to appear there.
llvm-wrapper: adapt for LLVM API change

LLVM commit llvm/llvm-project@1d608fc renamed the pass.

r? ````@nikic````
…rors

Extract exhaustiveness into its own crate

It now makes sense to extract exhaustiveness into its own crate! This was much-requested by rust-analyzer (they currently maintain by hand a copy of the algorithm), and I hope this can serve other projects e.g. clippy.

This is the churny PR: it exclusively moves code around. It's not yet useable outside of rustc but I wanted the churny parts to be out of the way.

r? `@compiler-errors`
Edit target doc template to remove email

We don't really want to communicate with target maintainers via email. GitHub is where everything happens, people should have a GitHub account that can be pinged on issues.

This doesn't necessarily have to be a strict rule, but edit the template to suggest this. The previous template made it look like we care about having an email address, which we do not.

r? ````@davidtwco````
…askrgr

Fix again `rustc_codegen_gcc` tests

Similar to rust-lang#118706

r? `@GuillaumeGomez`
@rustbot rustbot added A-meta Area: Issues & PRs about the rust-lang/rust repository itself S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Dec 11, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=8

@bors
Copy link
Contributor

bors commented Dec 11, 2023

📌 Commit 1cf538b has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 11, 2023
@bors
Copy link
Contributor

bors commented Dec 11, 2023

⌛ Testing commit 1cf538b with merge 21cce21...

@bors
Copy link
Contributor

bors commented Dec 11, 2023

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 21cce21 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 11, 2023
@bors bors merged commit 21cce21 into rust-lang:master Dec 11, 2023
12 checks passed
@rustbot rustbot added this to the 1.76.0 milestone Dec 11, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#118620 resolve: Use def_kind query to cleanup some code 6066057efeb901f89666daf430264dcaa10f4260 (link)
#118647 dump bootstrap shims 73101b40876469df51607555baabecf95fd7f4f8 (link)
#118726 Do not parenthesize exterior struct lit inside match guards 7a5899eed37fdf03e60708751aad68232277c301 (link)
#118818 llvm-wrapper: adapt for LLVM API change d1f59fab5fd7a90b3ce463a3aef066717458d802 (link)
#118822 Extract exhaustiveness into its own crate b812d86c0e6df7edf1963cc86396cd2856752ea3 (link)
#118826 Edit target doc template to remove email 285dd4cb78ad739fea7e60a7e8bdf64c7faf445d (link)
#118827 Update table for linker-plugin-lto docs 360a29f17c177b9d9205da7494c33f7f39a09709 (link)
#118835 Fix again rustc_codegen_gcc tests c41081d74e7894b5ae5e78c3f454d26aeaa62bf5 (link)

previous master: 57010939ed

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (21cce21): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.9% [-1.0%, -0.9%] 6
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
1.1% [1.1%, 1.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.1% [1.1%, 1.1%] 1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
5.1% [5.0%, 5.2%] 3
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.7% [-3.5%, -2.1%] 5
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 670.568s -> 672.051s (0.22%)
Artifact size: 314.19 MiB -> 314.25 MiB (0.02%)

@matthiaskrgr matthiaskrgr deleted the rollup-8kwzpho branch March 16, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues & PRs about the rust-lang/rust repository itself 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. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.