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 6 pull requests #120170

Merged
merged 16 commits into from
Jan 20, 2024
Merged

Rollup of 6 pull requests #120170

merged 16 commits into from
Jan 20, 2024

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

smoelius and others added 16 commits January 15, 2024 12:26
Co-authored-by: fee1-dead <ent3rm4n@gmail.com>
Currently stable users can't benefit from this because GlobaAlloc doesn't support
alignment-changing realloc and neither do most posix allocators.
So in practice it always results in an extra memcpy.
"It's" expands to "it is". "Its" is the possessive form.
…n-ty-alias, r=notriddle

Fix impl stripped in rustdoc HTML whereas it should not be in case the impl is implemented on a type alias

Fixes rust-lang#119015.

I talked about it a bit with ```@petrochenkov.``` They might change what `EffectiveVisibilities` return for impl items like this one and make them not only reachable but also re-exported, which would fix this case. It could also potentially break other things, so it'll be done whenever they can and then we can check together.

Surprisingly, this fix is making rustdoc even closer to rustc in term of errors (the CI currently fails because currently accepted broken codes aren't working anymore with this change). Not sure exactly why though. This is linked to rust-lang#110631 from what I could find.

So either I'm missing something here, or we consider it's ok and we consider the failing tests as "should fail" and I'll update `rustdoc-ui` ones.

r? ```@notriddle```
Ensure `callee_id`s are body owners

This PR makes the `callee_id` argument of Clippy's `implements_trait_with_env` optional, and when it is passed, ensures it is a body owner.

rust-lang#118661 added the `callee_id` parameter to alleviate an ICE. Specifically, the `callee_id` is used to determine an "effect arg" in certain situations.

Frankly, I [do not completely understand](rust-lang#118661 (comment)) what an "effect arg" is. But the code that determines it seems to require that `callee_id` is a body owner:
- https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/src/tools/clippy/clippy_utils/src/ty.rs#L286-L288
- https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/ty/util.rs#L834
- https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/hir/map/mod.rs#L372

In the current head, some def ids passed as `callee_id`s are not body owners. This PR fixes that.

cc ``@rust-lang/clippy``

r? ``@fee1-dead``
…ilstrieb

Remove special handling of `box` expressions from parser

rust-lang#108471 added a temporary hack to parse `box expr`. It's been almost a year since then, so I think it's safe to remove the special handling.

As a drive-by cleanup, move `parser/removed-syntax*` tests to their own directory.
…viper

Remove alignment-changing in-place collect

This removes the alignment-changing in-place collect optimization introduced in rust-lang#110353
Currently stable users can't benefit from the optimization because GlobaAlloc doesn't support alignment-changing realloc and neither do most posix allocators. So in practice it has a negative impact on performance.

Explanation from rust-lang#120091 (comment):

> > You mention that in case of alignment mismatch -- when the new alignment is less than the old -- the implementation calls `mremap`.
>
> I was trying to note that this isn't really the case in practice, due to the semantics of Rust's allocator APIs. The only use of the allocator within the `in_place_collect` implementation itself is [a call to `Allocator::shrink()`](https://github.com/rust-lang/rust/blob/db7125f008cfd72e8951c9a863178956e2cbacc3/library/alloc/src/vec/in_place_collect.rs#L299-L303), which per its documentation [allows decreasing the required alignment](https://doc.rust-lang.org/1.75.0/core/alloc/trait.Allocator.html). However, in stable Rust, the only available `Allocator` is [`Global`](https://doc.rust-lang.org/1.75.0/alloc/alloc/struct.Global.html), which delegates to the registered `GlobalAlloc`. Since `GlobalAlloc::realloc()` [cannot change the required alignment](https://doc.rust-lang.org/1.75.0/core/alloc/trait.GlobalAlloc.html#method.realloc), the implementation of [`<Global as Allocator>::shrink()`](https://github.com/rust-lang/rust/blob/db7125f008cfd72e8951c9a863178956e2cbacc3/library/alloc/src/alloc.rs#L280-L321) must fall back to creating a brand-new allocation, `memcpy`ing the data into it, and freeing the old allocation, whenever the alignment doesn't remain exactly the same.
>
> Therefore, the underlying allocator, provided by libc or some other source, has no opportunity to internally `mremap()` the data when the alignment is changed, since it has no way of knowing that the allocation is the same.
…albertlarsan68

Increase vscode settings.json `git.detectSubmodulesLimit`

The default vscode git integration throws a warning on the rust repository because it has more than the default limit of 10 submodules. This adds an increase to 20 to the settings.json that x.py offers to install on setup.

<img width="461" alt="Screen Shot 2024-01-19 at 11 47 47 PM" src="https://github.com/rust-lang/rust/assets/230691/440dfa4e-32e3-41f7-b8c6-5a07ade7aa14">

Also reported at https://stackoverflow.com/questions/60917209/disable-vs-code-warning-submodules-which-wont-be-opened-automatically
Spelling fix

"It's" expands to "it is". "It is use..." doesn't make sense.

"Its" is the possessive form, and the intended form here.
@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Jan 20, 2024
@GuillaumeGomez
Copy link
Member Author

@bors r+ p=5 rollup=never

@bors
Copy link
Contributor

bors commented Jan 20, 2024

📌 Commit 774cd3a has been approved by GuillaumeGomez

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 Jan 20, 2024
@bors
Copy link
Contributor

bors commented Jan 20, 2024

⌛ Testing commit 774cd3a with merge 038d115...

@bors
Copy link
Contributor

bors commented Jan 20, 2024

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 038d115 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 20, 2024
@bors bors merged commit 038d115 into rust-lang:master Jan 20, 2024
12 checks passed
@rustbot rustbot added this to the 1.77.0 milestone Jan 20, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#119997 Fix impl stripped in rustdoc HTML whereas it should not be … b503ef92612ff65b60638162281362caa5dbcecf (link)
#120000 Ensure callee_ids are body owners 16c0519dcce1abf1277fdf8f1ef71fd3754eb0d8 (link)
#120063 Remove special handling of box expressions from parser d8a79d12dcc6afb666bdb8595229fb9ab6e184c3 (link)
#120116 Remove alignment-changing in-place collect fe5341d6235e017b14b0ee8b66e8c8739521d8b4 (link)
#120138 Increase vscode settings.json git.detectSubmodulesLimit 22378c7471c11c438c3fc9830703e69f9d3ff432 (link)
#120169 Spelling fix b4ad68a7094beb7be6305ea87292f0b67921ab21 (link)

previous master: 1828461982

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 (038d115): 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)
0.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.9% [-1.4%, -0.5%] 2
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.4% [-1.4%, 0.6%] 3

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)
6.3% [3.2%, 9.5%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-7.9% [-12.7%, -3.3%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.2% [-12.7%, 9.5%] 5

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

Binary size

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

Bootstrap: 665.294s -> 665.187s (-0.02%)
Artifact size: 308.36 MiB -> 308.34 MiB (-0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Jan 20, 2024
@GuillaumeGomez GuillaumeGomez deleted the rollup-edqdf30 branch January 21, 2024 11:33
@Kobzol
Copy link
Contributor

Kobzol commented Jan 23, 2024

A tiny regression on a single benchmark, I don't think that it's worth it to delve deeper.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Jan 23, 2024
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. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants