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 7 pull requests #112102

Merged
merged 25 commits into from
May 30, 2023
Merged

Rollup of 7 pull requests #112102

merged 25 commits into from
May 30, 2023

Conversation

Noratrieb
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

bvanjoi and others added 25 commits May 27, 2023 00:52
…lias

Before:
```
thread 'main' panicked at 'no entry found for key', builder.rs:110:30
```

After:
```
thread 'main' panicked at 'missing crate for path library', check.rs:89:26
```
This is useful for profiling metadata generation.

This comes very close to removing all_krates, but doesn't quite -
there's one last usage left in `doc`.
This fixes the panic from the previous commit.
Previously `description` only supported `Testing` and `Benchmarking`,
and `msg` gave weird results for `doc` (it would say `Docing`).
Previously they were using `all_krates` and various hacks to determine
which crates to document. Switch them to `crate_or_deps` so `ShouldRun`
tells them which crate to document instead of having to guess.

This also makes a few other refactors:
- Remove the now unused `all_krates`; new code should only use
  `crate_or_deps`.
- Add tests for documenting Std
- Remove the unnecessary `run_cargo_rustdoc_for` closure so that we only
  run cargo once
- Give a more helpful error message when documenting a no_std target
- Use `builder.msg` in the Steps instead of `builder.info`
and document why the single remaining place can't switch
- Switch from `cargo rustdoc` to `cargo doc`

  This allows passing `-p` to multiple packages.

- Remove `OsStr` support

  It doesn't work with RUSTDOCFLAGS, and we don't support non-utf8 paths
  anyway.

- Pass `-p std` for each crate in the standard library

  By default cargo only documents the top-level crate, which is
  `sysroot` and has no docs.
Only make the use-dot-operator-to-call-method suggestion, but do not
double down and use the recovered type to perform method call
typechecking as it will produce confusing diagnostics on the "fixed"
code.
…cked, r=WaffleLapkin

Uplift `clippy::invalid_utf8_in_unchecked` lint

This PR aims at uplifting the `clippy::invalid_utf8_in_unchecked` lint into two lints.

## `invalid_from_utf8_unchecked`

(deny-by-default)

The `invalid_from_utf8_unchecked` lint checks for calls to `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut` with an invalid UTF-8 literal.

### Example

```rust
unsafe {
    std::str::from_utf8_unchecked(b"cl\x82ippy");
}
```

### Explanation

Creating such a `str` would result in undefined behavior as per documentation for `std::str::from_utf8_unchecked` and `std::str::from_utf8_unchecked_mut`.

## `invalid_from_utf8`

(warn-by-default)

The `invalid_from_utf8` lint checks for calls to `std::str::from_utf8` and `std::str::from_utf8_mut` with an invalid UTF-8 literal.

### Example

```rust
std::str::from_utf8(b"ru\x82st");
```

### Explanation

Trying to create such a `str` would always return an error as per documentation for `std::str::from_utf8` and `std::str::from_utf8_mut`.

-----

Mostly followed the instructions for uplifting a clippy lint described here: rust-lang#99696 (review)

````@rustbot```` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::invalid_utf8_in_unchecked` into rustc
fix: dedup `static_candidates` before report

Fixes rust-lang#103646

`record_static_candidate` had been executed twice, resulting in the presence of two identical `CandidateSource::Trait(Cat)`  in static_candidates. This PR aims to deduplication the `static_candidates` list, allowing it to execute `suggest_associated_call_syntax` properly.
bootstrap: Various Step refactors

This ended up being a bunch of only somewhat related changes, sorry 😓 on the bright side, they're all fairly small  and well-commented in the commit messages.

- Document `ShouldRun::crates`

- Switch Steps from crates to crate_or_deps where possible

    and document why the single remaining place can't switch

- Switch doc::{Std, Rustc} to `crate_or_deps`

    Previously they were using `all_krates` and various hacks to determine
    which crates to document. Switch them to `crate_or_deps` so `ShouldRun`
    tells them which crate to document instead of having to guess.

    This also makes a few other refactors:
    - Remove the now unused `all_krates`; new code should only use
      `crate_or_deps`.
    - Add tests for documenting Std
    - Remove the unnecessary `run_cargo_rustdoc_for` closure so that we only
      run cargo once
    - Give a more helpful error message when documenting a no_std target
    - Use `builder.msg` in the Steps instead of `builder.info`

- Extend `msg` and `description` to work with any subcommand

    Previously `description` only supported `Testing` and `Benchmarking`,
    and `msg` gave weird results for `doc` (it would say `Docing`).

- Add a `make_run_crates` function and use it Rustc and Std

    This fixes the panic from the previous commit.

- Allow checking individual crates

    This is useful for profiling metadata generation.

    This comes very close to removing all_krates, but doesn't quite -
    there's one last usage left in `doc`. This is fixed in a later commit.

- Give a more helpful error when calling `cargo_crates_in_set` for an alias

    Before:
    ```
    thread 'main' panicked at 'no entry found for key', builder.rs:110:30
    ```

    After:
    ```
    thread 'main' panicked at 'missing crate for path library', check.rs:89:26
    ```
`EarlyBinder::new` -> `EarlyBinder::bind`

for consistency with `Binder::bind`. it may make sense to also add `EarlyBinder::dummy` in places where we know that no parameters exist, but I left that out of this PR.

r? `@jackh726` `@kylematsuda`
…lor-9, r=notriddle

Migrate GUI colors test to original CSS color format

Follow-up of rust-lang#111459.

The `browser-ui-test` update is a fix when converting the alpha value to hex format. More information [here](GuillaumeGomez/browser-UI-test#511).

r? ````@notriddle````
Don't typecheck recovered method call from suggestion

Only make the use-dot-operator-to-call-method suggestion, but do not double down and use the recovered type to perform method call typechecking as it will produce confusing diagnostics relevant for the *fixed* code.

### Code Sample

```rust
struct Client;

impl Client {
    fn post<T: std::ops::Add>(&self, _: T, _: T) {}
}

fn f() {
    let c = Client;
    post(c, ());
}
```

### Before This PR

```
error[[E0277]](https://doc.rust-lang.org/stable/error_codes/E0277.html): cannot add `()` to `()`
 --> src/lib.rs:9:5
  |
9 |     post(c, ());
  |     ^^^^^^^^^^^ no implementation for `() + ()`
  |
  = help: the trait `Add` is not implemented for `()`
note: required by a bound in `Client::post`
 --> src/lib.rs:4:16
  |
4 |     fn post<T: std::ops::Add>(&self, _: T, _: T) {}
  |                ^^^^^^^^^^^^^ required by this bound in `Client::post`

error[[E0061]](https://doc.rust-lang.org/stable/error_codes/E0061.html): this function takes 2 arguments but 1 argument was supplied
 --> src/lib.rs:9:5
  |
9 |     post(c, ());
  |     ^^^^ an argument of type `()` is missing
  |
note: method defined here
 --> src/lib.rs:4:8
  |
4 |     fn post<T: std::ops::Add>(&self, _: T, _: T) {}
  |        ^^^^                   -----  ----  ----
help: provide the argument
  |
9 |     post((), ())(c, ());
  |         ++++++++

error[[E0425]](https://doc.rust-lang.org/stable/error_codes/E0425.html): cannot find function `post` in this scope
 --> src/lib.rs:9:5
  |
9 |     post(c, ());
  |     ^^^^ not found in this scope
  |
help: use the `.` operator to call the method `post` on `&Client`
  |
9 -     post(c, ());
9 +     c.post(());
  |

Some errors have detailed explanations: E0061, E0277, E0425.
For more information about an error, try `rustc --explain E0061`.
```

### After This PR

```
error[E0425]: cannot find function `post` in this scope
 --> tests/ui/typeck/issue-106929.rs:9:5
  |
9 |     post(c, ());
  |     ^^^^ not found in this scope
  |
help: use the `.` operator to call the method `post` on `&Client`
  |
9 -     post(c, ());
9 +     c.post(());
  |

error: aborting due to previous error

For more information about this error, try `rustc --explain E0425`.
```

Fixes rust-lang#106929.
@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. labels May 30, 2023
@rustbot rustbot added 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 May 30, 2023
@Noratrieb
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented May 30, 2023

📌 Commit cc12182 has been approved by Nilstrieb

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 May 30, 2023
@bors
Copy link
Contributor

bors commented May 30, 2023

⌛ Testing commit cc12182 with merge a9251b6...

@bors
Copy link
Contributor

bors commented May 30, 2023

☀️ Test successful - checks-actions
Approved by: Nilstrieb
Pushing a9251b6 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 30, 2023
@bors bors merged commit a9251b6 into rust-lang:master May 30, 2023
@rustbot rustbot added this to the 1.72.0 milestone May 30, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Perf Build Sha
#112100 f8df530f4c49243702394880a75225f6a1dcebbb
#112064 5176664d19d93b4df575c0c7f0cf66c9129abf66
#112060 c22cdf6e53057c3838aa391db600c86a69966915
#111955 fd84c70137568e92e1c284befd4959dd5e7e30cc
#111872 529f62d3a24e45112cf54c983b79d51b6fa01ff1
#111543 6edf235474fa662495b007c6f83ad890eeb919d0
#107916 0b22844cfad4c5ca24f4f66ee0c256c9b60422cf

previous master: 3266c36624

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@Noratrieb Noratrieb deleted the rollup-ivu1hmc branch May 30, 2023 16:07
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a9251b6): comparison URL.

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

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)
1.6% [1.6%, 1.6%] 1
Regressions ❌
(secondary)
3.4% [2.1%, 4.7%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.5% [-2.5%, -2.5%] 1
All ❌✅ (primary) 1.6% [1.6%, 1.6%] 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: 643.826s -> 644.295s (0.07%)

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