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

subtree-push nightly-2024-06-13 #6193

Merged

Conversation

ytmimi
Copy link
Contributor

@ytmimi ytmimi commented Jun 13, 2024

Bumping the toolchain version as part of a git subtree push

current toolchain (nightly-2023-12-28):

  • 1.77.0-nightly (89e2160c4 2023-12-27)

latest toolchain (nightly-2024-06-13):

  • 1.81.0-nightly (8337ba918 2024-06-12)

shepmaster and others added 30 commits January 1, 2024 17:47
Otherwise tests fail due to unknown lint and dead code warnings.
For consistency with other `Emitter` impls, such as `JsonEmitter`,
`SilentEmitter`, `SharedEmitter`, etc.
`Emitter` cleanups

Some improvements I found while looking at this code.

r? `@oli-obk`
This works for most of its call sites. This is nice, because `emit` very
much makes sense as a consuming operation -- indeed,
`DiagnosticBuilderState` exists to ensure no diagnostic is emitted
twice, but it uses runtime checks.

For the small number of call sites where a consuming emit doesn't work,
the commit adds `DiagnosticBuilder::emit_without_consuming`. (This will
be removed in subsequent commits.)

Likewise, `emit_unless` becomes consuming. And `delay_as_bug` becomes
consuming, while `delay_as_bug_without_consuming` is added (which will
also be removed in subsequent commits.)

All this requires significant changes to `DiagnosticBuilder`'s chaining
methods. Currently `DiagnosticBuilder` method chaining uses a
non-consuming `&mut self -> &mut Self` style, which allows chaining to
be used when the chain ends in `emit()`, like so:
```
    struct_err(msg).span(span).emit();
```
But it doesn't work when producing a `DiagnosticBuilder` value,
requiring this:
```
    let mut err = self.struct_err(msg);
    err.span(span);
    err
```
This style of chaining won't work with consuming `emit` though. For
that, we need to use to a `self -> Self` style. That also would allow
`DiagnosticBuilder` production to be chained, e.g.:
```
    self.struct_err(msg).span(span)
```
However, removing the `&mut self -> &mut Self` style would require that
individual modifications of a `DiagnosticBuilder` go from this:
```
    err.span(span);
```
to this:
```
    err = err.span(span);
```
There are *many* such places. I have a high tolerance for tedious
refactorings, but even I gave up after a long time trying to convert
them all.

Instead, this commit has it both ways: the existing `&mut self -> Self`
chaining methods are kept, and new `self -> Self` chaining methods are
added, all of which have a `_mv` suffix (short for "move"). Changes to
the existing `forward!` macro lets this happen with very little
additional boilerplate code. I chose to add the suffix to the new
chaining methods rather than the existing ones, because the number of
changes required is much smaller that way.

This doubled chainging is a bit clumsy, but I think it is worthwhile
because it allows a *lot* of good things to subsequently happen. In this
commit, there are many `mut` qualifiers removed in places where
diagnostics are emitted without being modified. In subsequent commits:
- chaining can be used more, making the code more concise;
- more use of chaining also permits the removal of redundant diagnostic
  APIs like `struct_err_with_code`, which can be replaced easily with
  `struct_err` + `code_mv`;
- `emit_without_diagnostic` can be removed, which simplifies a lot of
  machinery, removing the need for `DiagnosticBuilderState`.
`is_force_warn` is only possible for diagnostics with `Level::Warning`,
but it is currently stored in `Diagnostic::code`, which every diagnostic
has.

This commit:
- removes the boolean `DiagnosticId::Lint::is_force_warn` field;
- adds a `ForceWarning` variant to `Level`.

Benefits:
- The common `Level::Warning` case now has no arguments, replacing
  lots of `Warning(None)` occurrences.
- `rustc_session::lint::Level` and `rustc_errors::Level` are more
  similar, both having `ForceWarning` and `Warning`.
One consequence is that errors returned by
`maybe_new_parser_from_source_str` now must be consumed, so a bunch of
places that previously ignored those errors now cancel them. (Most of
them explicitly dropped the errors before. I guess that was to indicate
"we are explicitly ignoring these", though I'm not 100% sure.)
All the other `emit`/`emit_diagnostic` methods were recently made
consuming (e.g. #119606), but this one wasn't. But it makes sense to.

Much of this is straightforward, and lots of `clone` calls are avoided.
There are a couple of tricky bits.
- `Emitter::primary_span_formatted` no longer takes a `Diagnostic` and
  returns a pair. Instead it takes the two fields from `Diagnostic` that
  it used (`span` and `suggestions`) as `&mut`, and modifies them. This
  is necessary to avoid the cloning of `diag.children` in two emitters.
- `from_errors_diagnostic` is rearranged so various uses of `diag` occur
  before the consuming `emit_diagnostic` call.
…artwright

Format `async` trait bounds in rustfmt

r? `@ytmimi` or `@calebcartwright`

This PR opts to do formatting in the rust-lang/rust tree because otherwise we'd have to wait until a full sync, and rustfmt is currently totally removing the `async` keyword.

cc rust-lang#6070
This makes it more like `hir::TyKind::Err`, and avoids a
`span_delayed_bug` call in `LoweringContext::lower_ty_direct`.

It also requires adding `ast::TyKind::Dummy`, now that
`ast::TyKind::Err` can't be used for that purpose in the absence of an
error emission.

There are a couple of cases that aren't as neat as I would have liked,
marked with `FIXME` comments.
Subdiagnostics don't need to be lazily translated, they can always be
eagerly translated. Eager translation is slightly more complex as we need
to have a `DiagCtxt` available to perform the translation, which involves
slightly more threading of that context.

This slight increase in complexity should enable later simplifications -
like passing `DiagCtxt` into `AddToDiagnostic` and moving Fluent messages
into the diagnostic structs rather than having them in separate files
(working on that was what led to this change).

Signed-off-by: David Wood <david@davidtw.co>
…ercote

errors: only eagerly translate subdiagnostics

Subdiagnostics don't need to be lazily translated, they can always be eagerly translated. Eager translation is slightly more complex as we need to have a `DiagCtxt` available to perform the translation, which involves slightly more threading of that context.

This slight increase in complexity should enable later simplifications - like passing `DiagCtxt` into `AddToDiagnostic` and moving Fluent messages into the diagnostic structs rather than having them in separate files (working on that was what led to this change).

r? ```@nnethercote```
Commit 72b172b in #121206 changed things so that
`emit_stashed_diagnostics` is only called from `run_compiler`. But
rustfmt doesn't use `run_compiler`, so it needs to call
`emit_stashed_diagnostics` itself to avoid an abort in
`DiagCtxtInner::drop` when stashed diagnostics occur.

Fixes #121450.
Implement RFC 3373: Avoid non-local definitions in functions

This PR implements [RFC 3373: Avoid non-local definitions in functions](rust-lang/rust#120363).
This call was added to `parse_crate_mod` in #121487, to fix a case where
a stashed diagnostic wasn't emitted. But there is another path where a
stashed diagnostic might fail to be emitted if there's a parse error, if
the `build` call in `parse_crate_inner` fails before `parse_crate_mod`
is reached.

So this commit moves the `emit_stashed_diagnostic` call outwards, from
`parse_crate_mod` to `format_project`, just after the
`Parser::parse_crate` call. This should be far out enough to catch any
parsing errors.

Fixes #121517.
Add `ErrorGuaranteed` to `ast::ExprKind::Err`

See #119967 for context
```
      \
       \
          _~^~^~_
      \) /  o o  \ (/
        '_   -   _'
        / '-----' \
```

r? fmease
Move `emit_stashed_diagnostic` call in rustfmt.

This call was added to `parse_crate_mod` in #121487, to fix a case where a stashed diagnostic wasn't emitted. But there is another path where a stashed diagnostic might fail to be emitted if there's a parse error, if the `build` call in `parse_crate_inner` fails before `parse_crate_mod` is reached.

So this commit moves the `emit_stashed_diagnostic` call outwards, from `parse_crate_mod` to `format_project`, just after the `Parser::parse_crate` call. This should be far out enough to catch any parsing errors.

Fixes #121517.

r? `@oli-obk`
cc `@ytmimi`
Rollup of 6 pull requests

Successful merges:

 - #121389 (llvm-wrapper: fix few warnings)
 - #121493 (By changing some attributes to only_local, reducing encoding attributes in the crate metadate.)
 - #121615 (Move `emit_stashed_diagnostic` call in rustfmt.)
 - #121617 (Actually use the right closure kind when checking async Fn goals)
 - #121628 (Do not const prop unions)
 - #121629 (fix some references to no-longer-existing ReprOptions.layout_seed)

r? `@ghost`
`@rustbot` modify labels: rollup
I started by changing it to `DiagData`, but that didn't feel right.
`DiagInner` felt much better.
GKFX and others added 20 commits April 28, 2024 14:35
The functionality of all three crates is now available in the standard library.
…r=oli-obk

Add StaticForeignItem and use it on ForeignItemKind

This is in preparation for unsafe extern blocks that adds a safe variant for functions inside extern blocks.

r? `@oli-obk`
cc `@compiler-errors`
Remove braces when fixing a nested use tree into a single item

[Back in 2019](rust-lang/rust#56645) I added rustfix support for the `unused_imports` lint, to automatically remove them when running `cargo fix`. For the most part this worked great, but when removing all but one childs of a nested use tree it turned `use foo::{Unused, Used}` into `use foo::{Used}`. This is slightly annoying, because it then requires you to run `rustfmt` to get `use foo::Used`.

This PR automatically removes braces and the surrouding whitespace when all but one child of a nested use tree are unused. To get it done I had to add the span of the nested use tree to the AST, and refactor a bit the code I wrote back then.

A thing I noticed is, there doesn't seem to be any `//@ run-rustfix` test for fixing the `unused_imports` lint. I created a test in `tests/suggestions` (is that the right directory?) that for now tests just what I added in the PR. I can followup in a separate PR to add more tests for fixing `unused_lints`.

This PR is best reviewed commit-by-commit.
```rust
reuse prefix::{a, b, c}
```
It's a zero-value wrapper of `Parser::new`.
Currently we have an awkward mix of fallible and infallible functions:
```
       new_parser_from_source_str
 maybe_new_parser_from_source_str
       new_parser_from_file
(maybe_new_parser_from_file)        // missing
      (new_parser_from_source_file) // missing
 maybe_new_parser_from_source_file
       source_str_to_stream
 maybe_source_file_to_stream
```
We could add the two missing functions, but instead this commit removes
of all the infallible ones and renames the fallible ones leaving us with
these which are all fallible:
```
new_parser_from_source_str
new_parser_from_file
new_parser_from_source_file
source_str_to_stream
source_file_to_stream
```
This requires making `unwrap_or_emit_fatal` public so callers of
formerly infallible functions can still work.

This does make some of the call sites slightly more verbose, but I think
it's worth it for the simpler API. Also, there are two `catch_unwind`
calls and one `catch_fatal_errors` call in this diff that become
removable thanks this change. (I will do that in a follow-up PR.)
The `Input::File` and `Input::Text` cases should be very similar.
However, currently the `Input::File` case uses `catch_unwind` because,
until recently (#125815) there was a fallible version of
`new_parser_from_source_str` but only an infallible version of
`new_parser_from_file`. This difference wasn't fundamental, just an
overlooked gap in the API of `rustc_parse`.

Both of those operations are now fallible, so the `Input::File` and
`Input::Text` cases can made more similar, with no need for
`catch_unwind`. This also lets us simplify an `Option<Vec<Diag>>` to
`Vec<Diag>`.
Unsafe extern blocks

This implements RFC 3484.

Tracking issue #123743 and RFC rust-lang/rfcs#3484

This is better reviewed commit by commit.
…, r=davidtwco"

This reverts commit 57dad1d75e562ff73051c1c43b07eaf65c7dbd74, reversing
changes made to 36316df9fe6c3e246153fe6e78967643cf08c148.
We need to allow `StyleEditionDefault` because it will be used to
implement `style_edition`, but that work is currently ongoing.
fixes compilation issues with the `generic-simd` feature
Can't run `cargo test --all` for `error-chain` anymore. The tests don't
compile because of `#[deny(invalid_doc_attributes)]`. Here's  the error
message:

```
error: this attribute can only be applied at the crate level
   --> tests/tests.rs:508:7
    |
508 | #[doc(test)]
    |       ^^^^
    |
    = note: read <https://doc.rust-lang.org/nightly/rustdoc/the-doc-attribute.html#at-the-crate-level> for more information
    = note: `#[deny(invalid_doc_attributes)]` on by default
help: to apply to the crate, use an inner attribute
    |
508 | #![doc(test)]
    |  +
```
The syntax changed from `expr: ty` -> `builtin # type_ascribe(expr, ty)`
For now, rustfmt will just emit the contents of the span.
Our diff-check job was failing in part due to removing `unsafe` from any
`#[unsafe(attributes)]`. To prevent that I added a quick implementation
for this.
Also formats unforamatted files in the crate.
@ytmimi
Copy link
Contributor Author

ytmimi commented Jun 13, 2024

Diff-Check is still failing, but all the failures seem expected to me.

@calebcartwright
Copy link
Member

sorry i should've done this before the doc site fix PR 🤦

@calebcartwright calebcartwright merged commit 3ffd7d4 into rust-lang:master Jun 17, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.