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

Merged
merged 15 commits into from
Sep 8, 2023
Merged

Rollup of 6 pull requests #115672

merged 15 commits into from
Sep 8, 2023

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

mkrasnitski and others added 15 commits March 13, 2023 01:31
Remake of "List matching impls on type aliases"
* 4b1d13d
* 6f552c8
* 2ce7cd9

Partially reverts "Fix infinite loop when retrieving impls for
type alias", but keeps the test case.

This version of the PR avoids the infinite loop by structurally
matching types instead of using full unification. This version
does not support type alias trait bounds, but the compiler does
not enforce those anyway
(rust-lang#21903).
Co-authored-by: León Orell Valerian Liehr <me@fmease.dev>
modify fuction clond() -> cloned()

optimize the code

Handle the problem that the pathset is empty and modify the judgment of the builder::tests::test_exclude_kind

Delete unnecessary judegment conditions

skip test for library/std duo to OOM in benches as library/alloc

Add FIXME for WASM32
…-docs, r=oli-obk

Clarify stability guarantee for lifetimes in enum discriminants

Since `std::mem::Discriminant` erases lifetimes, it should be clarified that changing the concrete value of a lifetime parameter does not change the value of an enum discriminant for a given variant. This is useful as it guarantees that it is safe to transmute `Discriminant<Foo<'a>>` to `Discriminant<Foo<'b>>` for any combination of `'a` and `'b`. This also holds for type-generics as long as the type parameters do not change, e.g. `Discriminant<Foo<T, 'a>>` can be transmuted to `Discriminant<Foo<T, 'b>>`.

Side note: Is what I've written actually enough to imply soundness (or rather codify it), or should it specifically be spelled out that it's OK to transmute in the above way?
…an68

Fix Step Skipping Caused by Using the `--exclude` Option

The original code was overreacting to the `--exclude` option,
https://github.com/rust-lang/rust/blob/eadf69a6c6edfe220fc5b1b659e46e271d75a3a1/src/bootstrap/builder.rs#L257-L260
For example:
When `x test --exclude alloc` or `x test --exclude library/alloc` is passed, the entire libraray test is skipped.

Related issues:
rust-lang#112009
…l-list, r=GuillaumeGomez

rustdoc: list matching impls on type aliases

Fixes rust-lang#32077

Fixes rust-lang#99952

Remake of rust-lang#112429

Partially reverts rust-lang#112543, but keeps the test case.

This version of the PR avoids the infinite loop by structurally matching types instead of using full unification. This version does not support type alias trait bounds, but the compiler does not enforce those anyway (rust-lang#21903).

r? `@GuillaumeGomez`

CC `@lcnr`
…t-node, r=petrochenkov

Lint node for `PRIVATE_BOUNDS`/`PRIVATE_INTERFACES` is the item which names the private type

The HIR that the `PRIVATE_BOUNDS` lint should be attached to is the item that has the *bounds*, not the private type. This PR also aligns this behavior with the `EXPORTED_PRIVATE_DEPENDENCIES` lint, which also requires putting the `allow` on the item that names the private type.

Fixes rust-lang#115475

r? petrochenkov
`-Cllvm-args` usability improvement

fixes: rust-lang#26338
fixes: rust-lang#115564

Two problems were found during playing with `-Cllvm-args`

1. When `llvm.link-shared` is set to `false` in `config.toml`, output of `rustc -C llvm-args='--help-list-hidden'` doesn't contain `--emit-dwarf-unwind` and `--emulated-tls`. When it is set to `true`, `rustc -C llvm-args='--help-list-hidden'` emits `--emit-dwarf-unwind`, but `--emulated-tls` is still missing.
2. Setting `-Cllvm-args=--emit-dwarf-unwind=always` doesn't take any effect, but `-Cllvm-args=-machine-outliner-reruns=3` does work.

### 1

Adding `RegisterCodeGenFlags` to register codegen flags fixed the first problem. `rustc -C llvm-args='--help-list-hidden'` emits full codegen flags including `--emit-dwarf-unwind` and `--emulated-tls`.

### 2

Constructing `TargetOptions` from `InitTargetOptionsFromCodeGenFlags` in `LLVMRustCreateTargetMachine` fixed the second problem. The `LLVMRustSetLLVMOptions` calls `ParseCommandLineOptions` which parses given `llvm-args`. For options like `machine-outliner-reruns`, it just works, since the codegen logic directly consumes the parsing result:

[machine-outliner-reruns register](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/lib/CodeGen/MachineOutliner.cpp#L114)
[machine-outliner-reruns consumption](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/lib/CodeGen/MachineOutliner.cpp#L1138)

But for flags defined in `TargetOptions` and `MCTargetOptions` to take effect, constructing them with `InitTargetOptionsFromCodeGenFlags` is essential, or the parsing result is just not consumed. Similar patterns can be observed in [lli](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/tools/llc/llc.cpp#L494), [llc](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/tools/lli/lli.cpp#L517), etc.
…-obk

fix: return early when has tainted in mir-lint

Fixes rust-lang#115203

`a[..]` is of indeterminate size, it had been reported error during borrow check, therefore we skip the mir lint process.
@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc 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-infra Relevant to the infrastructure 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 Sep 8, 2023
@GuillaumeGomez
Copy link
Member Author

@bors r+ p=6 rollup=never

@bors
Copy link
Contributor

bors commented Sep 8, 2023

📌 Commit 60327bb 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 Sep 8, 2023
@bors
Copy link
Contributor

bors commented Sep 8, 2023

⌛ Testing commit 60327bb with merge 309af34...

@bors
Copy link
Contributor

bors commented Sep 8, 2023

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 309af34 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 8, 2023
@bors bors merged commit 309af34 into rust-lang:master Sep 8, 2023
12 checks passed
@rustbot rustbot added this to the 1.74.0 milestone Sep 8, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#104299 Clarify stability guarantee for lifetimes in enum discrimin… 540e5b70bf8a5e234a80e94e31e9cde6897597f1 (link)
#115088 Fix Step Skipping Caused by Using the --exclude Option eeafad5c9ccb863e5dfef11f92ad56d9af873785 (link)
#115201 rustdoc: list matching impls on type aliases e85c7bb8c7ec5b39297facb20d1b2244ce82bdf2 (link)
#115633 Lint node for PRIVATE_BOUNDS/PRIVATE_INTERFACES is the … b82c7c31a619a95488a912d614ccec94f9e4c6c7 (link)
#115638 -Cllvm-args usability improvement 78e7f036ef4674d52139db821a8ab778597b57f3 (link)
#115643 fix: return early when has tainted in mir-lint 7d9f4bc42b0f4adb0855a7731e6134555f24e29e (link)

previous master: cd71a37f32

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 (309af34): comparison URL.

Overall result: ❌ regressions - 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)
4.2% [0.8%, 9.8%] 5
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 4.2% [0.8%, 9.8%] 5

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

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)
4.9% [2.8%, 8.4%] 4
Regressions ❌
(secondary)
3.3% [2.6%, 3.6%] 4
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 4.9% [2.8%, 8.4%] 4

Binary size

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

Bootstrap: 631.473s -> 629.777s (-0.27%)
Artifact size: 318.18 MiB -> 318.21 MiB (0.01%)

@rustbot rustbot added the perf-regression Performance regression. label Sep 8, 2023
@lqd
Copy link
Member

lqd commented Sep 8, 2023

Ah ok it's probably just rustdoc doing more work. So likely #115201, cc @notriddle @GuillaumeGomez: I'm not sure if this is unexpected, or completely fine?

@GuillaumeGomez
Copy link
Member Author

It list impls on all type aliases, so it was to be expected. I didn't think it would be this much though.

@notriddle
Copy link
Contributor

size:doc_bytes shows image and stm32f4 getting a lot bigger. docs.rs shows a lot of typedefs in those libraries, too.

@lqd
Copy link
Member

lqd commented Sep 8, 2023

yeah stm32f4 is almost like a rustdoc stress test at this point 😅. The benchmark is around 400ms slower than before on the server, so it's not unbearable per se, but maybe there are ways to lower that overhead for these big cases.

(Still if it's not worth it and t-rustdoc feels this is acceptable, I'll let you two mention that here for the purposes of the weekly perf triage and mark this regression triaged.)

@GuillaumeGomez
Copy link
Member Author

Unfortunately, there isn't anything obvious that can be done perf-wise from what I can see. I think the feature outweights the regression cost so it's fine.

@lqd
Copy link
Member

lqd commented Sep 8, 2023

Gotcha. Then: @rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc 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-infra Relevant to the infrastructure 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.