subtree-push nightly-2024-04-08 #6140
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.)
…a macro arg in rustfmt
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.
Currently it only checks calls to functions marked with
`#[rustc_lint_diagnostics]`. This commit changes it to check calls to
any function with an `impl Into<{D,Subd}iagMessage>` parameter. This
greatly improves its coverage and doesn't rely on people remembering to
add `#[rustc_lint_diagnostics]`.
The commit also adds `#[allow(rustc::untranslatable_diagnostic)`]
attributes to places that need it that are caught by the improved lint.
These places that might be easy to convert to translatable diagnostics.
Finally, it also:
- Expands and corrects some comments.
- Does some minor formatting improvements.
- Adds missing `DecorateLint` cases to
`tests/ui-fulldeps/internal-lints/diagnostics.rs`.
…c-lint, r=davidtwco
Rework `untranslatable_diagnostic` lint
Currently it only checks calls to functions marked with `#[rustc_lint_diagnostics]`. This PR changes it to check calls to any function with an `impl Into<{D,Subd}iagnosticMessage>` parameter. This greatly improves its coverage and doesn't rely on people remembering to add `#[rustc_lint_diagnostics]`. It also lets us add `#[rustc_lint_diagnostics]` to a number of functions that don't have an `impl Into<{D,Subd}iagnosticMessage>`, such as `Diag::span`.
r? ``@davidtwco``
This change is primarily meant to allow rustfmt to ignore all diagnostics when using the `SilentEmitter`. Back in PR 121301 the `SilentEmitter` was shared between rustc and rustfmt. This changed rustfmt's behavior from ignoring all diagnostic to emitting fatal diagnostics. These changes allow rustfmt to maintain it's previous behaviour when using the SilentEmitter, while allowing rustc code to still emit fatal diagnostics.
Stop using `box PAT` syntax for deref patterns, as it's misleading and also causes their semantics being tangled up.
Experimental feature postfix match This has a basic experimental implementation for the RFC postfix match (rust-lang/rfcs#3295, #121618). [Liaison is](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/Postfix.20Match.20Liaison/near/423301844) ```@scottmcm``` with the lang team's [experimental feature gate process](https://github.com/rust-lang/lang-team/blob/master/src/how_to/experiment.md). This feature has had an RFC for a while, and there has been discussion on it for a while. It would probably be valuable to see it out in the field rather than continue discussing it. This feature also allows to see how popular postfix expressions like this are for the postfix macros RFC, as those will take more time to implement. It is entirely implemented in the parser, so it should be relatively easy to remove if needed. This PR is split in to 5 commits to ease review. 1. The implementation of the feature & gating. 2. Add a MatchKind field, fix uses, fix pretty. 3. Basic rustfmt impl, as rustfmt crashes upon seeing this syntax without a fix. 4. Add new MatchSource to HIR for Clippy & other HIR consumers
…c, r=davidtwco conditionally ignore fatal diagnostic in the SilentEmitter This change is primarily meant to allow rustfmt to ignore all diagnostics when using the `SilentEmitter`. Back in #121301 the `SilentEmitter` was shared between rustc and rustfmt. This changed rustfmt's behavior from ignoring all diagnostic to emitting fatal diagnostics, which lead to rust-lang#6109. These changes allow rustfmt to maintain its previous behaviour when using the `SilentEmitter`, while allowing rustc code to still emit fatal diagnostics.
We need to allow `StyleEditionDefault` because it will be used to implement `style_edition`, and we didn't need to explicitly import it for tests since it's already imported by `use super::*;`.
|
@calebcartwright I think we need to remove or disable the |
I see you're working on getting that fixed upstream, happy to consider alternatives in parallel (e.g. adjusting feature specification on our end) |
|
Yeah, I thought I'd take a stab at resolving the issue in llogiq/bytecount#92. CI is passing so I'm guessing I ported things over from
Let me know if anything else comes to mind or if you'd prefer to move forward with one of these options. |
|
I don't think there's any particular urgency to push a sync through at the moment. It's been too long since the last one, but that was the case last week and that will remain the case next week. As such I'd rather take a pause and seek a resolution to the root cause as opposed to trying to work around symptoms |
|
Works for me. Hopefully the |
fixes compilation issues with the `generic-simd` feature
|
@calebcartwright |
|
@calebcartwright is it fine to remove |
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)]
| +
```
ytmimi
force-pushed
the
subtree-push-nightly-2024-04-08
branch
2 times, most recently
from
41778bd
to
44a0dae
Compare
The syntax changed from `expr: ty` -> `builtin # type_ascribe(expr, ty)` For now, rustfmt will just emit the contents of the span.
ytmimi
force-pushed
the
subtree-push-nightly-2024-04-08
branch
from
44a0dae
to
35ad744
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Bumping the toolchain version as part of a git subtree push
current toolchain (nightly-2023-12-28):
latest toolchain (nightly-2024-04-08):
r? @calebcartwright