Rollup of 8 pull requests #110967

matthiaskrgr · 2023-04-28T20:56:48Z

Successful merges:

Provide better type hints when a type doesn't support a binary operator #110877 (Provide better type hints when a type doesn't support a binary operator)
only error combining +whole-archive and +bundle for rlibs #110917 (only error combining +whole-archive and +bundle for rlibs)
Use NonNull::new_unchecked and NonNull::len in rustc_arena. #110921 (Use NonNull::new_unchecked and NonNull::len in rustc_arena.)
Encoder/decoder cleanups #110927 (Encoder/decoder cleanups)
share BinOp::Offset between CTFE and Miri #110944 (share BinOp::Offset between CTFE and Miri)
run-make test: using single quotes to not trigger the shell #110948 (run-make test: using single quotes to not trigger the shell)
Fix an ICE in conflict error diagnostics #110957 (Fix an ICE in conflict error diagnostics)
fix false negative for unused_mut #110960 (fix false negative for unused_mut)

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Fixes rust-lang#110912 Checking `flavor == RlibFlavor::Normal` was accidentally lost in 601fc8b rust-lang#105601 That caused combining +whole-archive and +bundle link modifiers on non-rlib crates to fail with a confusing error message saying that combination is unstable for rlibs. In particular, this caused the build to fail when +whole-archive was used on staticlib crates, even though +whole-archive effectively does nothing on non-bin crates because the final linker invocation is left to an external build system.

It's unnecessary. Note that `MemDecoder::read_raw_bytes` how has a `&'a [u8]` return type, the same as what `read_raw_bytes_inherent` had.

Because I was wondering about it, and this may save a future person from also wondering.

`rustc_arena`.

Checking that `read_raw_bytes(len)` changes the position by `len` is a reasonable thing for a test, but isn't much use in just one of the zillion `Decodable` impls.

The methods for `i8`, `bool`, `char`, `str` are the same for all impls, because they layered on top of other methods.

It's just a synonym for `read_u8`.

I was curious about how many `Encodable`/`Decodable` derives we have. Some grepping revealed that it's over 500 of each, but the number of `Encodable` ones was higher, which was weird. Most of the `Encodable`-only ones were in `hir.rs`. This commit removes them all, plus some other unnecessary derives in that file and others that I found via trial and error.

Provide better type hints when a type doesn't support a binary operator For example, when checking whether `vec![A] == vec![A]` holds, we first evaluate the LHS's ty, then probe for any `PartialEq` implementations for that. If none is found, we report an error by evaluating `Vec<A>: PartialEq<?0>` for fulfillment errors, but the RHS is not yet evaluated and remains an inference variable `?0`! To fix this, we evaluate the RHS and equate it to that RHS infer var `?0`, so that we are able to provide more detailed fulfillment errors for why `Vec<A>: PartialEq<Vec<A>>` doesn't hold (namely, the nested obligation `A: PartialEq<A>` doesn't hold). Fixes rust-lang#95285 Fixes rust-lang#110867

…b_fix, r=petrochenkov only error combining +whole-archive and +bundle for rlibs Fixes rust-lang#110912 Checking `flavor == RlibFlavor::Normal` was accidentally lost in 601fc8b rust-lang#105601 That caused combining +whole-archive and +bundle link modifiers on non-rlib crates to fail with a confusing error message saying that combination is unstable for rlibs. In particular, this caused the build to fail when +whole-archive was used on staticlib crates, even though +whole-archive effectively does nothing on non-bin crates because the final linker invocation is left to an external build system. cc ``@petrochenkov``

…=Nilstrieb Use `NonNull::new_unchecked` and `NonNull::len` in `rustc_arena`. This avoids a few extra dereferences as well as an `unwrap`. According to the docs for [`NonNull::len`](https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.len) this also ensures that: > This function is safe, even when the non-null raw slice cannot be dereferenced to a slice because the pointer does not have a valid address. I am also fairly sure that the `unwrap` is unneeded in this case, as the docs for [`Box::into_raw`](https://doc.rust-lang.org/std/boxed/struct.Box.html#method.into_raw) also state: > Consumes the Box, returning a wrapped raw pointer. **The pointer will be properly aligned and non-null.**

…ps, r=scottmcm Encoder/decoder cleanups Best reviewed one commit at a time. r? ``@scottmcm``

share BinOp::Offset between CTFE and Miri r? ``@oli-obk``

run-make test: using single quotes to not trigger the shell This test got added in rust-lang#110801. I'm no expert on Makefiles, but IIUC this command is passed to the shell, which usually tries to execute commands specified in between backticks in double-quoted strings. Using single quotes should fix this, I think. (Note: Waiting for CI to test this, since I only have a web browser available right now). r? ``@jyn514`` cc ``@WaffleLapkin`` Since this is breaking our build bot, even if it is not directly LLVM related: ``@rustbot`` label: +llvm-main

…ict_error, r=cjgillot Fix an ICE in conflict error diagnostics Fixes rust-lang#110929 r? ``@cjgillot``

…errors fix false negative for `unused_mut` fixes rust-lang#110849 We want to avoid double diagnostics for code like this, but only if an error actually occurs: ```rust fn main() { let mut x: (i32, i32); x.0 = 1; } ``` The first commit fixes the lint and the second one removes all the unused `mut`s it found.

matthiaskrgr · 2023-04-28T20:57:11Z

@bors r+ rollup=never p=8

bors · 2023-04-28T20:57:15Z

📌 Commit 34ef13b has been approved by matthiaskrgr

It is now in the queue for this repository.

bors · 2023-04-28T23:13:45Z

⌛ Testing commit 34ef13b with merge 7a96158...

bors · 2023-04-29T02:29:07Z

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 7a96158 to master...

rust-timer · 2023-04-29T02:31:23Z

📌 Perf builds for each rolled up PR:

PR#	Perf Build Sha
#110960	ab1c4582f2627102268987dfaff1f7a29985e82f
#110957	6fa8b441ab280143873ef4c8a125c11ed96aa499
#110948	4c8805833452b133d51399c3288ed4a207cff5b5
#110944	fe93e3f35bc4fc049d1c06f90ef4d35ce4c6676d
#110927	f5397a766c0ebc703f253194dbe4fb1c85e7e53a
#110921	15fa707bb915741c5a8dbfdbb6ca29990dc2bbc8
#110917	da78fa555b969cf3f93decd3b117adff1cb97b15
#110877	068e587367fa4e424eca954201fb0b90e2e4e8de

previous master: f495605381

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 · 2023-04-29T03:45:42Z

Finished benchmarking commit (7a96158): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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)	1.1%	[0.4%, 2.0%]	17
Regressions ❌ (secondary)	1.1%	[1.1%, 1.3%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.3%	[-0.3%, -0.3%]	2
All ❌✅ (primary)	1.1%	[0.4%, 2.0%]	17

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.8%	[0.7%, 1.0%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.2%	[-5.2%, -5.2%]	1
All ❌✅ (primary)	0.8%	[0.7%, 1.0%]	2

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)	3.0%	[1.2%, 5.8%]	13
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	3.0%	[1.2%, 5.8%]	13

lqd · 2023-05-02T11:59:32Z

I think there's a bit of noise here, but e.g. diesel now looks on average a bit slower than before, probably a real regression.

The wall-time on typeck seems to be the most impacted query there, though it's less clear in cachegrind (involves inlining within the hot type folding and predicate obligation processing).

So maybe some of the cleanups in #110877 could be related, doing slightly more work than before:
@rust-timer build 068e587367fa4e424eca954201fb0b90e2e4e8de

On clap, it seems like things are now back to more-or-less the same state as before, but this rollup's result look similar to diesel's in cg (some of the wall-time query diffs look more to be about borrowck, however, which also saw changes in this PR).

wg-grammar is not clear as well, seems to be on a slight upward trend starting at this PR but it looks noisy and still within the recent bimodality bumps' ranges.

rust-timer · 2023-05-02T13:15:19Z

Finished benchmarking commit (068e587367fa4e424eca954201fb0b90e2e4e8de): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

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)	1.6%	[1.4%, 2.0%]	6
Regressions ❌ (secondary)	1.1%	[1.1%, 1.2%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.6%	[1.4%, 2.0%]	6

Max RSS (memory usage)

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

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: 660.129s -> 659.944s (-0.03%)

lqd · 2023-05-02T15:02:27Z

#110877 looks involved in some of these, so let's at least let author @compiler-errors know.

compiler-errors · 2023-05-02T15:04:40Z

I have absolutely no idea how that PR could've caused regressions. It hardly touches non-diagnostic code, let alone hot code 😅

lqd · 2023-05-02T15:18:21Z

It could indeed just be some unlucky rejuggling of the folding and predicates codegen. Looking at it wrong, or even just thinking about it, is apparently enough for it to become unstable in benchmarks (just typing this sentence has probably jinxed us with more instability).

compiler-errors and others added 25 commits April 27, 2023 00:57

Clean up some builtin operator typeck logic

3d80dd9

Provide RHS type hint when reporting operator error

015acc2

Fix a bad binop error when we need a call

3125979

Remove MemDecoder::read_raw_bytes_inherent.

b51deba

It's unnecessary. Note that `MemDecoder::read_raw_bytes` how has a `&'a [u8]` return type, the same as what `read_raw_bytes_inherent` had.

Add a comment explaining the lack of Decoder::read_enum_variant.

37c9e45

Because I was wondering about it, and this may save a future person from also wondering.

Use NonNull::new_unchecked and NonNull::len in

618841b

`rustc_arena`.

Remove a low-value assertion.

fa133f5

Checking that `read_raw_bytes(len)` changes the position by `len` is a reasonable thing for a test, but isn't much use in just one of the zillion `Decodable` impls.

Add some provided methods to Encoder/Decoder.

7a16d25

The methods for `i8`, `bool`, `char`, `str` are the same for all impls, because they layered on top of other methods.

Remove MemDecoder::read_byte.

a676dfa

It's just a synonym for `read_u8`.

share BinOp::Offset between CTFE and Miri

586d17d

run-make test: using single quotes to not trigger the shell

f1b88ea

Add regression test for issue 110929

b29b56f

fix false negative for unused_mut

69c71da

Fix an ICE in conflict errors diagnostics

754a62c

remove unused muts

fc63926

Rollup merge of rust-lang#110927 - nnethercote:Encoder-Decoder-cleanu…

37076eb

…ps, r=scottmcm Encoder/decoder cleanups Best reviewed one commit at a time. r? ``@scottmcm``

Rollup merge of rust-lang#110944 - RalfJung:offset, r=compiler-errors

03da9db

share BinOp::Offset between CTFE and Miri r? ``@oli-obk``

Rollup merge of rust-lang#110957 - WaffleLapkin:reach_generator_confl…

235d088

…ict_error, r=cjgillot Fix an ICE in conflict error diagnostics Fixes rust-lang#110929 r? ``@cjgillot``

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Apr 28, 2023

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 Apr 28, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 29, 2023

bors merged commit 7a96158 into rust-lang:master Apr 29, 2023

rustbot added this to the 1.71.0 milestone Apr 29, 2023

This was referenced Apr 29, 2023

Add generic NonZero type. #100428

Closed

Remove type ascription from parser and diagnostics #109128

Merged

Deny the unsafe_op_in_unsafe_fn lint in rustc_arena. #110950

Merged

rustbot added the perf-regression Performance regression. label Apr 29, 2023

This comment has been minimized.

Sign in to view

matthiaskrgr deleted the rollup-vfbl7gm branch March 16, 2024 18:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rollup of 8 pull requests #110967

Rollup of 8 pull requests #110967

matthiaskrgr commented Apr 28, 2023

matthiaskrgr commented Apr 28, 2023

bors commented Apr 28, 2023

bors commented Apr 28, 2023

bors commented Apr 29, 2023

rust-timer commented Apr 29, 2023

rust-timer commented Apr 29, 2023

lqd commented May 2, 2023

This comment has been minimized.

rust-timer commented May 2, 2023

lqd commented May 2, 2023

compiler-errors commented May 2, 2023

lqd commented May 2, 2023

Rollup of 8 pull requests #110967

Rollup of 8 pull requests #110967

Conversation

matthiaskrgr commented Apr 28, 2023

matthiaskrgr commented Apr 28, 2023

bors commented Apr 28, 2023

bors commented Apr 28, 2023

bors commented Apr 29, 2023

rust-timer commented Apr 29, 2023

rust-timer commented Apr 29, 2023

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

lqd commented May 2, 2023

This comment has been minimized.

rust-timer commented May 2, 2023

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Binary size

lqd commented May 2, 2023

compiler-errors commented May 2, 2023

lqd commented May 2, 2023