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 10 pull requests #122151

Merged
merged 32 commits into from
Mar 8, 2024
Merged

Conversation

GuillaumeGomez
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

weiznich and others added 30 commits February 27, 2024 08:50
…mented]` attribute

This PR stabilizes the `#[diagnostic]` attribute namespace and a minimal
option of the `#[diagnostic::on_unimplemented]` attribute.

The `#[diagnostic]` attribute namespace is meant to provide a home for
attributes that allow users to influence error messages emitted by the
compiler. The compiler is not guaranteed to use any of this hints,
however it should accept any (non-)existing attribute in this namespace
and potentially emit lint-warnings for unused attributes and options.
This is meant to allow discarding certain attributes/options in the
future to allow fundamental changes to the compiler without the need to
keep then non-meaningful options working.

The `#[diagnostic::on_unimplemented]` attribute is allowed to appear
on a trait definition. This allows crate authors to hint the compiler
to emit a specific error message if a certain trait is not implemented.
For the `#[diagnostic::on_unimplemented]` attribute the following
options are implemented:

* `message` which provides the text for the top level error message
* `label` which provides the text for the label shown inline in the
broken code in the error message
* `note` which provides additional notes.

The `note` option can appear several times, which results in several
note messages being emitted. If any of the other options appears several
times the first occurrence of the relevant option specifies the actually
used value. Any other occurrence generates an lint warning. For any
other non-existing option a lint-warning is generated.

All three options accept a text as argument. This text is allowed to
contain format parameters referring to generic argument or `Self` by
name via the `{Self}` or `{NameOfGenericArgument}` syntax. For any
non-existing argument a lint warning is generated.

Tracking issue: rust-lang#111996
…d increase contrast by setting text color to black
This method would previously take a target scope, and then verify that it
was equal to the scope on top of the if-then scope stack.

In practice, this means that callers have to go out of their way to pass around
redundant scope information that's already on the if-then stack.

So it's easier to just retrieve the correct scope directly from the if-then
stack, and simplify the other code that was passing it around.
This will avoid rebuilds due to the script being more recent than the
rest of the original files.
try_instantiate_mir_and_normalize_erasing_regions was changed in rust-lang@dbc2cc8, but not instantiate_mir_and_normalize_erasing_regions
…pace, r=compiler-errors

Stabilize the `#[diagnostic]` namespace and `#[diagnostic::on_unimplemented]` attribute

This PR stabilizes the `#[diagnostic]` attribute namespace and a minimal option of the `#[diagnostic::on_unimplemented]` attribute.

The `#[diagnostic]` attribute namespace is meant to provide a home for attributes that allow users to influence error messages emitted by the compiler. The compiler is not guaranteed to use any of this hints, however it should accept any (non-)existing attribute in this namespace and potentially emit lint-warnings for unused attributes and options. This is meant to allow discarding certain attributes/options in the future to allow fundamental changes to the compiler without the need to keep then non-meaningful options working.

The `#[diagnostic::on_unimplemented]` attribute is allowed to appear on a trait definition. This allows crate authors to hint the compiler to emit a specific error message if a certain trait is not implemented. For the `#[diagnostic::on_unimplemented]` attribute the following options are implemented:

* `message` which provides the text for the top level error message
* `label` which provides the text for the label shown inline in the broken code in the error message
* `note` which provides additional notes.

The `note` option can appear several times, which results in several note messages being emitted. If any of the other options appears several times the first occurrence of the relevant option specifies the actually used value. Any other occurrence generates an lint warning. For any other non-existing option a lint-warning is generated.

All three options accept a text as argument. This text is allowed to contain format parameters referring to generic argument or `Self` by name via the `{Self}` or `{NameOfGenericArgument}` syntax. For any non-existing argument a lint warning is generated.

This allows to have a trait definition like:

```rust
#[diagnostic::on_unimplemented(
    message = "My Message for `ImportantTrait<{A}>` is not implemented for `{Self}`",
    label = "My Label",
    note = "Note 1",
    note = "Note 2"
)]
trait ImportantTrait<A> {}

```

which then generates for the following code

```rust
fn use_my_trait(_: impl ImportantTrait<i32>) {}

fn main() {
    use_my_trait(String::new());
}
```

this error message:

```
error[E0277]: My Message for `ImportantTrait<i32>` is not implemented for `String`
  --> src/main.rs:14:18
   |
14 |     use_my_trait(String::new());
   |     ------------ ^^^^^^^^^^^^^ My Label
   |     |
   |     required by a bound introduced by this call
   |
   = help: the trait `ImportantTrait<i32>` is not implemented for `String`
   = note: Note 1
   = note: Note 2
```

[Playground with the unstable feature](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=05133acce8e1d163d481e97631f17536)

Fixes rust-lang#111996
…enkov

Remove `feed_local_def_id`

best reviewed commit by commit

Basically I returned `TyCtxtFeed` from `create_def` and then preserved that in the local caches

based on rust-lang#121084

r? ````@petrochenkov````
…piler-errors

AST validation: Improve handling of inherent impls nested within functions and anon consts

Minimal fix for issue rust-lang#121607 extracted from PR rust-lang#120698 for ease of backporting and since I'd like to improve PR rust-lang#120698 in such a way that it makes AST validator truly robust against such sort of regressions (AST validator is generally *beyond* footgun-y atm). The current version of PR rust-lang#120698 sort of does that already but there's still room for improvement.

Fixes rust-lang#89342.
Fixes [after beta-backport] rust-lang#121607.
Partially addresses rust-lang#119924 (rust-lang#120698 aims to fully fix it).

---

### Explainer

The last commit of PR rust-lang#119505 regressed issue rust-lang#121607.

Previously we would reject visibilities on associated items with `visibility_not_permitted` if we were in a trait (by checking the parameter `ctxt` of `visit_assoc_item` which was 100% accurate) or if we were in a trait impl (by checking a flag called `in_trait_impl` tracked in `AstValidator` which was/is only accurate if the visitor methods correctly updated it which isn't actually the case giving rise to the old open issue rust-lang#89342).

In PR rust-lang#119505, I moved even more state into the `AstValidator` by generalizing the flag `in_trait_impl` to `trait_or_trait_impl` to be able to report more precise diagnostics (modeling *Trait | TraitImpl*). However since we/I didn't update `trait_or_trait_impl` in all places to reflect reality (similar to us not updating `in_trait_impl` before), this lead to rust-lang#121607 (comment) getting wrongfully rejected. Since PR rust-lang#119505 we reject visibilities if the “globally tracked” (wrt. to `AstValidator`) `outer_trait_or_trait_impl` is `Some`.

Crucially, when visiting an inherent impl, I never reset `outer_trait_or_trait_impl` back to `None` leading us to believe that `bar` in the stack [`trait Foo` > `fn foo` > `impl Bar` > `pub fn bar`] (from the MCVE) was an inherent associated item (we saw `trait Foo` but not `impl Bar` before it).

The old open issue rust-lang#89342 is caused by the aforementioned issue of us never updating `in_trait_impl` prior to my PR rust-lang#119505 / `outer_trait_or_trait` after my PR. Stack: [`impl Default for Foo` > `{` > `impl Foo` > `pub const X`] (we only saw `impl Default for Foo` but not the `impl Foo` before it).

---

This PR is only meant to be a *hot fix*. I plan on completely *rewriting* `AstValidator` from the ground up to not rely on “globally tracked” state like this or at least make it close to impossible to forget updating it when descending into nested items (etc.). Other visitors do a way better job at that (e.g. AST lowering). I actually plan on experimenting with moving more and more logic from `AstValidator` into the AST lowering pass/stage/visitor to follow the [Parse, don't validate](https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/) “pattern”.

---

r? `@compiler-errors`
…, r=notriddle

Add missing background color for top-level rust documentation page and increase contrast by setting text color to black

Fixes rust-lang#121954.

r? ``@notriddle``
…rk-Simulacrum

Include all library files in artifact summary on CI

It's not worth it to maintain any custom logic here. Just print all files in the `lib` directory, this should be forward compatible.

This fixes rust-lang#121866, based on rust-lang#121967.

r? `@Mark-Simulacrum`
…jasper

Don't pass a break scope to `Builder::break_for_else`

This method would previously take a target scope, and then verify that it was equal to the scope on top of the if-then scope stack.

In practice, this means that callers have to go out of their way to pass around redundant scope information that's already on the if-then stack.

So it's easier to just retrieve the correct scope directly from the if-then stack, and simplify the other code that was passing it around.

---

Both ways of indicating the break target were introduced in rust-lang#88572. I haven't been able to find any strong indication of whether this was done deliberately, or whether it was just an implementation artifact. But to me it doesn't seem useful to carefully pass around the same scope in two different ways.
Record mtime in bootstrap's LLVM linker script

As discovered in https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60ui.60.20tests.20re-running.3F the linker script added in rust-lang#121967 can trigger rebuilds or new test executions, as it's more recent than some of the existing files themselves.

This PR copies the mtime to the linker script to prevent a second invocation of `./x test tests/ui` from rerunning all of the tests.
sync (try_)instantiate_mir_and_normalize_erasing_regions implementation

`try_instantiate_mir_and_normalize_erasing_regions` was changed in rust-lang@dbc2cc8, but not `instantiate_mir_and_normalize_erasing_regions`, sync them.

see https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/instantiate_mir_and_normalize_erasing_regions.20vs.20try_*.20ver

r? `@lcnr`
…er-errors

cleanup rustc_infer

the commits should be self-contained

r? types
…ngjubilee

Make `std::os::unix::ucred` module private

Tracking issue: rust-lang#42839

Currently, this unstable module exists: [`std::os::unix::ucred`](https://doc.rust-lang.org/stable/std/os/unix/ucred/index.html).
All it does is provide `UCred` (which is also available from `std::os::unix::net`), `impl_*` (which is probably a mishap and should be private) and `peer_cred` (which is undocumented but has a documented counterpart at `std::os::unix::net::UnixStream::peer_cred`).

This PR makes the entire `ucred` module private and moves it into `net`, because that's where it is used.

I hope it's fine to simply remove it without a deprecation phase. Otherwise, I can add back a deprecated reexport module `std::os::unix::ucred`.

`@rustbot` label: -T-libs +T-libs-api
@rustbot rustbot added O-unix Operating system: Unix-like 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. rollup A PR which is a rollup labels Mar 7, 2024
@GuillaumeGomez
Copy link
Member Author

@bors r+ p=5 rollup=never

@bors
Copy link
Contributor

bors commented Mar 7, 2024

📌 Commit 92d7e02 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 7, 2024
@bors
Copy link
Contributor

bors commented Mar 7, 2024

⌛ Testing commit 92d7e02 with merge 9823f17...

@bors
Copy link
Contributor

bors commented Mar 8, 2024

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez
Pushing 9823f17 to master...

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

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#119888 Stabilize the #[diagnostic] namespace and `#[diagnostic::… 0443b9923135d1c6c44364ad3c332f4cf48146a5 (link)
#121089 Remove feed_local_def_id e9ddc231bc5f70e599e470dec747e2b1d92fcf87 (link)
#122004 AST validation: Improve handling of inherent impls nested w… 97034b14b1393b8051def81729af071bcae303b1 (link)
#122087 Add missing background color for top-level rust documentati… 395ac03469ea21be283495066fdc46dec98cad76 (link)
#122136 Include all library files in artifact summary on CI 12ccc45eb7e2156ca1d6133d1f32f341da236577 (link)
#122137 Don't pass a break scope to Builder::break_for_else 5fd72c5cce94255eae62663cba8b1bada9f86917 (link)
#122138 Record mtime in bootstrap's LLVM linker script aa8f343f70bc6f4514b8fa677fba504a41012012 (link)
#122141 sync (try_)instantiate_mir_and_normalize_erasing_regions im… 62b80a12d2cdb3563a1b821df0e21c4558edf004 (link)
#122142 cleanup rustc_infer 6fcace39a2e9ff8a64e52c927ffd18447515381f (link)
#122147 Make std::os::unix::ucred module private d65ead21a7b4bd99841168ca2c1b7bf927f01783 (link)

previous master: 9c3ad802d9

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 (9823f17): 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)
-3.1% [-3.1%, -3.1%] 1
Improvements ✅
(secondary)
-0.2% [-0.2%, -0.2%] 1
All ❌✅ (primary) -3.1% [-3.1%, -3.1%] 1

Max RSS (memory usage)

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

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

Binary size

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

Bootstrap: 647.485s -> 648.175s (0.11%)
Artifact size: 172.63 MiB -> 172.54 MiB (-0.05%)

@GuillaumeGomez GuillaumeGomez deleted the rollup-hfxr9kv branch March 8, 2024 09:41
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. O-unix Operating system: Unix-like 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.