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

Merged
merged 16 commits into from
Mar 15, 2024
Merged

Rollup of 6 pull requests #122555

merged 16 commits into from
Mar 15, 2024

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

beepster4096 and others added 16 commits March 13, 2024 12:34
… documentation clearly states that negative indexes will cause error.

Just making the code in the example to return Result::Ok, instead of Result::Error.
Also replace a few `hir_node()` calls with `hir_node_by_def_id()`
Instead of executing the test builder directly, the test builder wrapper
will be called with test builder as the first argument and subsequent
arguments. This is similar to cargo's RUSTC_WRAPPER argument.

The `--test-builder-wrapper` argument can be passed multiple times to
allow "nesting" of wrappers.
Also reword "test-builder-wrapper" argument help.
This avoids unnecessary allocation with a temporary Vec.
…illaumeGomez

rustdoc: add `--test-builder-wrapper` arg to support wrappers such as RUSTC_WRAPPER when building doctests

Currently, `rustdoc` builds test crates with `rustc` directly instead of using [`RUSTC_WRAPPER`](https://doc.rust-lang.org/cargo/reference/config.html#buildrustc-wrapper) (if any is set).

This causes build issues in build systems that use `cargo` but tweak linking flags by setting the `RUSTC_WRAPPER` environment variable.

This change is not meant to be final--it's only a minimal proof of concept.
Please advise on the best way to proceed.

Open questions:
- [x] Does supporting the `rustc` wrappers make sense?
    - yes, `cargo-miri` for example needs a "hack" to workaround the issue
- [X] What environment variable(s) should be read for the rustc wrapper? Should `rustdoc` [use the same names as `cargo`](https://doc.rust-lang.org/cargo/reference/config.html#buildrustc-wrapper)?
    - None, since `rustdoc` takes arguments
- [X] What name should be used for a `rustdoc` CLI option?
    - `--test-builder-wrapper`
- [X] Should a separate workspace wrapper (like `RUSTC_WORKSPACE_WRAPPER`) be supported?
    - `--test-builder-wrapper` can be passed multiple times to get multiple wrappers passed
- [X] How/where should this be documented? It's not obvious to all users that `cargo doc` actually causes `rustdoc` to compile tests with rust
    - Added doc to `src/doc/rustdoc/src/command-line-arguments.md` per `@GuillaumeGomez`
…anup, r=Nadrieril

Cleanup `MirBorrowckCtxt::prefixes`

Some of the uses of this method aren't necessary anymore and `PrefixSet::Supporting` is not used anywhere.

With `PrefixSet::Supporting` removed, this could technically be moved to an extension trait on `PlaceRef`. However, it would have to be moved back to `MirBorrowckCtxt` when the `Derefer` MIR pass is moved before borrowck so I didn't.
…tation-fix, r=Nilstrieb

Cursor.rs documentation fix

Reason:

I've been learning Rust std library and got confused. Seek trait documentation clearly states that negative indexes will cause an error. And the code in the Cursor example uses negative index. I found myself trying to understand what am I missing until I've actually executed the code and got error. I decided to submit small fix to the documentation.
hir: Remove `opt_local_def_id_to_hir_id` and `opt_hir_node_by_def_id`

Also replace a few `hir_node()` calls with `hir_node_by_def_id()`.

Follow up to rust-lang#120943.
less symbol interner locks

This reduces instructions under 1% (in rustdoc run), but essentially free.
@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-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 Mar 15, 2024
@GuillaumeGomez
Copy link
Member Author

@bors r+ p=5 rollup=never

@bors
Copy link
Contributor

bors commented Mar 15, 2024

📌 Commit 6ec4092 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 Mar 15, 2024
@bors
Copy link
Contributor

bors commented Mar 15, 2024

⌛ Testing commit 6ec4092 with merge 72d7897...

@bors
Copy link
Contributor

bors commented Mar 15, 2024

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 72d7897 to master...

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

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#114651 rustdoc: add --test-builder-wrapper arg to support wrappe… be31074b590e8aff4a18c5d8d913516548ef6d9c (link)
#122468 Cleanup MirBorrowckCtxt::prefixes 3920aa996ae93b97d89769cdd430e47d3392205c (link)
#122496 Greatly reduce GCC build logs ece41f3e86c666e3571d82637abad8dab509565b (link)
#122512 Cursor.rs documentation fix 82ed6eca240bf8bcc3e2c50eb02baf2be6a8901c (link)
#122513 hir: Remove opt_local_def_id_to_hir_id and `opt_hir_node_… e4f42f2144a21d96183c480b462669bcb46c44a4 (link)
#122530 less symbol interner locks ca0c18975fd8aa26f2decfca1cbf3493ae54548b (link)

previous master: c5b571310d

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 (72d7897): comparison URL.

Overall result: ❌ regressions - 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.2% [0.2%, 0.2%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
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)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.8% [-1.8%, -1.8%] 1
Improvements ✅
(secondary)
-2.9% [-3.4%, -1.9%] 11
All ❌✅ (primary) -1.8% [-1.8%, -1.8%] 1

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: 670.605s -> 669.355s (-0.19%)
Artifact size: 311.47 MiB -> 311.47 MiB (-0.00%)

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. 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-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.

9 participants