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 #121491

Merged
merged 18 commits into from
Feb 23, 2024
Merged

Rollup of 8 pull requests #121491

merged 18 commits into from
Feb 23, 2024

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

estebank and others added 18 commits February 22, 2024 18:01
Having this set to true disrupts compiler development workflows for people who use `llvm.download-ci-llvm = true`
because we don't provide ci-llvm on the `rustc-alt-builds` server. Therefore, it is kept off by default.

Signed-off-by: onur-ozkan <[email protected]>
I ended up getting confused while trying to flip the
variances when flipping the order. Should be
all right now
This was caused by 72b172b in rust-lang#121206. That commit removed an early
return from `analysis` when there are stashed errors. As a result, it's
possible to reach privacy analysis when there are stashed errors, which
means more code paths can be reached. One such code path was handled in
that commit, where a `span_bug` was changed to a `span_delayed_bug`.

This commit handles another such code path uncovered by fuzzing, in much
the same way.

Fixes rust-lang#121455.
This makes it easier to see that we're manipulating the instance that is being
constructed, and is a lot less verbose than `basic_coverage_blocks`.
Commit 72b172b in rust-lang#121206 changed things so that
`emit_stashed_diagnostics` is only called from `run_compiler`. But
rustfmt doesn't use `run_compiler`, so it needs to call
`emit_stashed_diagnostics` itself to avoid an abort in
`DiagCtxtInner::drop` when stashed diagnostics occur.

Fixes rust-lang#121450.
…lcnr

Fix rust-lang#121208 fallout

rust-lang#121208 converted lots of delayed bugs to bugs. Unsurprisingly, there were a few invalid conversion found via fuzzing.

r? `@lcnr`
When encountering `<&T as Clone>::clone(x)` because `T: Clone`, suggest `#[derive(Clone)]`

CC rust-lang#40699.

```
warning: call to `.clone()` on a reference in this situation does nothing
  --> $DIR/noop-method-call.rs:23:71
   |
LL |     let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.clone();
   |                                                                       ^^^^^^^^
   |
   = note: the type `PlainType<u32>` does not implement `Clone`, so calling `clone` on `&PlainType<u32>` copies the reference, which does not do anything and can be removed
help: remove this redundant call
   |
LL -     let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref.clone();
LL +     let non_clone_type_ref_clone: &PlainType<u32> = non_clone_type_ref;
   |
help: if you meant to clone `PlainType<u32>`, implement `Clone` for it
   |
LL + #[derive(Clone)]
LL | struct PlainType<T>(T);
   |
```
…, r=compiler-errors

remove `llvm.assertions=true` in compiler profile

Having this set to true disrupts compiler development workflows for people who use `llvm.download-ci-llvm = true` because we don't provide ci-llvm on the `rustc-alt-builds` server. Therefore, it is kept off by default.

cc `@Nilstrieb` `@compiler-errors`

For more context, see https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/CI.20LLVM.20for.20aarch64
…rors

fix generalizer unsoundness

I ended up getting confused while trying to flip the variances when flipping the order. Should be all right now.

This is only exploitable when generalizing if the `ambient_variance` of the relation is `Contravariant`. This can currently only be the case in the NLL generalizer which only rarely generalizes, causing us to miss this regression. Very much an issue with rust-lang#121462 however.
…t, r=lcnr

Fix more rust-lang#121208 fallout

rust-lang#121208 converted lots of delayed bugs to bugs. Unsurprisingly, there were a few invalid conversion found via fuzzing.

r? `@lcnr`
Allow for a missing `adt_def` in `NamePrivacyVisitor`.

This was caused by 72b172b in rust-lang#121206. That commit removed an early return from `analysis` when there are stashed errors. As a result, it's possible to reach privacy analysis when there are stashed errors, which means more code paths can be reached. One such code path was handled in that commit, where a `span_bug` was changed to a `span_delayed_bug`.

This commit handles another such code path uncovered by fuzzing, in much the same way.

Fixes rust-lang#121455.

r? `@oli-obk`
coverage: Use variable name `this` in `CoverageGraph::from_mir`

A tiny little improvement, extracted from rust-lang#120013.

This makes it easier to see that we're manipulating the instance that is being constructed, and is a lot less verbose than `basic_coverage_blocks`.
Explicitly call `emit_stashed_diagnostics`.

Commit 72b172b in rust-lang#121206 changed things so that
`emit_stashed_diagnostics` is only called from `run_compiler`. But rustfmt doesn't use `run_compiler`, so it needs to call `emit_stashed_diagnostics` itself to avoid an abort in `DiagCtxtInner::drop` when stashed diagnostics occur.

Fixes rust-lang#121450.

r? `@oli-obk`
@rustbot rustbot added 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 Feb 23, 2024
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=8

@bors
Copy link
Contributor

bors commented Feb 23, 2024

📌 Commit 6ee43bc 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 Feb 23, 2024
@bors
Copy link
Contributor

bors commented Feb 23, 2024

⌛ Testing commit 6ee43bc with merge fdf8b29...

bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 23, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121434 (Fix rust-lang#121208 fallout)
 - rust-lang#121471 (When encountering `<&T as Clone>::clone(x)` because `T: Clone`, suggest `#[derive(Clone)]`)
 - rust-lang#121476 (remove `llvm.assertions=true` in compiler profile)
 - rust-lang#121479 (fix generalizer unsoundness)
 - rust-lang#121480 (Fix more rust-lang#121208 fallout)
 - rust-lang#121482 (Allow for a missing `adt_def` in `NamePrivacyVisitor`.)
 - rust-lang#121484 (coverage: Use variable name `this` in `CoverageGraph::from_mir`)
 - rust-lang#121487 (Explicitly call `emit_stashed_diagnostics`.)

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

The job armhf-gnu failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
---- [ui] tests/ui/macros/macro-quote-test.rs stdout ----

error: test run failed!
status: exit status: 101
command: RUST_TEST_THREADS="8" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/remote-test-client" "run" "1" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/macros/macro-quote-test/a" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/macros/macro-quote-test/auxiliary/libhello_macro.so"
--- stderr -------------------------------
thread 'main' panicked at src/tools/remote-test-client/src/main.rs:352:5:
thread 'main' panicked at src/tools/remote-test-client/src/main.rs:352:5:
io::copy(&mut file, dst) failed with Connection reset by peer (os error 104)
------------------------------------------



@bors
Copy link
Contributor

bors commented Feb 23, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 23, 2024
@matthiaskrgr
Copy link
Member Author

@bors retry

@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 Feb 23, 2024
@bors
Copy link
Contributor

bors commented Feb 23, 2024

⌛ Testing commit 6ee43bc with merge 52cea08...

@bors
Copy link
Contributor

bors commented Feb 23, 2024

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 52cea08 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 23, 2024
@bors bors merged commit 52cea08 into rust-lang:master Feb 23, 2024
12 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 23, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#121434 Fix #121208 fallout 51a6cf1b95bdcd5ff758f6bbae94261b0d0bbd32 (link)
#121471 When encountering <&T as Clone>::clone(x) because `T: Clo… 52f223a8e71b7ea46b35a757a4636d2a523d2ade (link)
#121476 remove llvm.assertions=true in compiler profile adb69567383a0d65157bf500d84f393cdb6d8ea3 (link)
#121479 fix generalizer unsoundness 51ce4d1bc1d827d077c3d1459813651e1c32c9ba (link)
#121480 Fix more #121208 fallout e9bf9a52438c229a5ed23de46de99e3e04f198e2 (link)
#121482 Allow for a missing adt_def in NamePrivacyVisitor. a72b2363e12c3c3b030271a0e205adb0c57759a9 (link)
#121484 coverage: Use variable name this in `CoverageGraph::from_… 809c145b548dd5373f3382927111ac91b0f28861 (link)
#121487 Explicitly call emit_stashed_diagnostics. c57c44221d43a332a564ba6b9ca3c92d3eb75162 (link)

previous master: ea2cc4368e

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 (52cea08): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

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)
- - 0
Regressions ❌
(secondary)
1.7% [1.7%, 1.7%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

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

Binary size

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

Bootstrap: 649.932s -> 649.924s (-0.00%)
Artifact size: 310.98 MiB -> 311.02 MiB (0.01%)

@matthiaskrgr matthiaskrgr deleted the rollup-wkzqawy branch March 16, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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. 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.

10 participants