Currently many diagnostic modifier methods are available on both
`Diagnostic` and `DiagnosticBuilder`. This commit removes most of them
from `Diagnostic`. To minimize the diff size, it keeps them within
`diagnostic.rs` but changes the surrounding `impl Diagnostic` block to
`impl DiagnosticBuilder`. (I intend to move things around later, to give
a more sensible code layout.)
`Diagnostic` keeps a few methods that it still needs, like `sub`,
`arg`, and `replace_args`.
The `forward!` macro, which defined two additional methods per call
(e.g. `note` and `with_note`), is replaced by the `with_fn!` macro,
which defines one additional method per call (e.g. `with_note`). It's
now also only used when necessary -- not all modifier methods currently
need a `with_*` form. (New ones can be easily added as necessary.)
All this also requires changing `trait AddToDiagnostic` so its methods
take `DiagnosticBuilder` instead of `Diagnostic`, which leads to many
mechanical changes. `SubdiagnosticMessageOp` gains a type parameter `G`.
There are three subdiagnostics -- `DelayedAtWithoutNewline`,
`DelayedAtWithNewline`, and `InvalidFlushedDelayedDiagnosticLevel` --
that are created within the diagnostics machinery and appended to
external diagnostics. These are handled at the `Diagnostic` level, which
means it's now hard to construct them via `derive(Diagnostic)`, so
instead we construct them by hand. This has no effect on what they look
like when printed.
There are lots of new `allow` markers for `untranslatable_diagnostics`
and `diagnostics_outside_of_impl`. This is because
`#[rustc_lint_diagnostics]` annotations were present on the `Diagnostic`
modifier methods, but missing from the `DiagnosticBuilder` modifier
methods. They're now present.
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.
Dejargonize `subst`
In favor of #110793, replace almost every occurence of `subst` and `substitution` from rustc codes, but they still remains in subtrees under `src/tools/` like clippy and test codes (I'd like to replace them after this)
Fix async closures in CTFE
First commit renames `is_coroutine_or_closure` into `is_closure_like`, because `is_coroutine_or_closure_or_coroutine_closure` seems confusing and long.
Second commit fixes some forgotten cases where we want to handle `TyKind::CoroutineClosure` the same as closures and coroutines.
The test exercises the change to `ValidityVisitor::aggregate_field_path_elem` which is the source of #120946, but not the change to `UsedParamsNeedSubstVisitor`, though I feel like it's not that big of a deal. Let me know if you'd like for me to look into constructing a test for the latter, though I have no idea what it'd look like (we can't assert against `TooGeneric` anywhere?).
Fixes#120946
r? oli-obk cc ``@RalfJung``
Rollup of 11 pull requests
Successful merges:
- #120765 (Reorder diagnostics API)
- #120833 (More internal emit diagnostics cleanups)
- #120899 (Gracefully handle non-WF alias in `assemble_alias_bound_candidates_recur`)
- #120917 (Remove a bunch of dead parameters in functions)
- #120928 (Add test for recently fixed issue)
- #120933 (check_consts: fix duplicate errors, make importance consistent)
- #120936 (improve `btree_cursors` functions documentation)
- #120944 (Check that the ABI of the instance we are inlining is correct)
- #120956 (Clean inlined type alias with correct param-env)
- #120962 (Add myself to library/std review)
- #120972 (fix ICE for deref coercions with type errors)
r? `@ghost`
`@rustbot` modify labels: rollup
Remove a bunch of dead parameters in functions
Found this kind of issue when working on https://github.com/rust-lang/rust/pull/119650
I wrote a trivial toy lint and manual review to find these.
Invert diagnostic lints.
That is, change `diagnostic_outside_of_impl` and `untranslatable_diagnostic` from `allow` to `deny`, because more than half of the compiler has been converted to use translated diagnostics.
This commit removes more `deny` attributes than it adds `allow` attributes, which proves that this change is warranted.
r? ````@davidtwco````
Rollup of 9 pull requests
Successful merges:
- #119592 (resolve: Unload speculatively resolved crates before freezing cstore)
- #120103 (Make it so that async-fn-in-trait is compatible with a concrete future in implementation)
- #120206 (hir: Make sure all `HirId`s have corresponding HIR `Node`s)
- #120214 (match lowering: consistently lower bindings deepest-first)
- #120688 (GVN: also turn moves into copies with projections)
- #120702 (docs: also check the inline stmt during redundant link check)
- #120727 (exhaustiveness: Prefer "`0..MAX` not covered" to "`_` not covered")
- #120734 (Add `SubdiagnosticMessageOp` as a trait alias.)
- #120739 (improve pretty printing for associated items in trait objects)
r? `@ghost`
`@rustbot` modify labels: rollup
Mark "unused binding" suggestion as maybe incorrect
Ignoring unused bindings should be a determination made by a human, `rustfix` shouldn't auto-apply the suggested change.
Fix#54196.
That is, change `diagnostic_outside_of_impl` and
`untranslatable_diagnostic` from `allow` to `deny`, because more than
half of the compiler has be converted to use translated diagnostics.
This commit removes more `deny` attributes than it adds `allow`
attributes, which proves that this change is warranted.
Suppress unhelpful diagnostics for unresolved top level attributes
Fixes#118455, unresolved top level attribute error didn't imported prelude and already have emitted an error, report builtin macro and attributes error by the way, so `check_invalid_crate_level_attr` in can ignore them.
Also fixes#89566, fixes#67107.
r? `@petrochenkov`
Because it's almost always static.
This makes `impl IntoDiagnosticArg for DiagnosticArgValue` trivial,
which is nice.
There are a few diagnostics constructed in
`compiler/rustc_mir_build/src/check_unsafety.rs` and
`compiler/rustc_mir_transform/src/errors.rs` that now need symbols
converted to `String` with `to_string` instead of `&str` with `as_str`,
but that' no big deal, and worth it for the simplifications elsewhere.
Error codes are integers, but `String` is used everywhere to represent
them. Gross!
This commit introduces `ErrCode`, an integral newtype for error codes,
replacing `String`. It also introduces a constant for every error code,
e.g. `E0123`, and removes the `error_code!` macro. The constants are
imported wherever used with `use rustc_errors::codes::*`.
With the old code, we have three different ways to specify an error code
at a use point:
```
error_code!(E0123) // macro call
struct_span_code_err!(dcx, span, E0123, "msg"); // bare ident arg to macro call
\#[diag(name, code = "E0123")] // string
struct Diag;
```
With the new code, they all use the `E0123` constant.
```
E0123 // constant
struct_span_code_err!(dcx, span, E0123, "msg"); // constant
\#[diag(name, code = E0123)] // constant
struct Diag;
```
The commit also changes the structure of the error code definitions:
- `rustc_error_codes` now just defines a higher-order macro listing the
used error codes and nothing else.
- Because that's now the only thing in the `rustc_error_codes` crate, I
moved it into the `lib.rs` file and removed the `error_codes.rs` file.
- `rustc_errors` uses that macro to define everything, e.g. the error
code constants and the `DIAGNOSTIC_TABLES`. This is in its new
`codes.rs` file.
dead_code treats #[repr(transparent)] the same as #[repr(C)]
In #92972 we enabled linting on unused fields in tuple structs. In #118297 that lint was enabled by default. That exposed issues like #119659, where the fields of a struct marked `#[repr(transparent)]` were reported by the `dead_code` lint. The language team [decided](https://github.com/rust-lang/rust/issues/119659#issuecomment-1885172045) that the lint should treat `repr(transparent)` the same as `#[repr(C)]`.
Fixes#119659
Gracefully handle missing typeck information if typeck errored
fixes#116893
I created some logs and the typeck of `fn main` is exactly the same, no matter whether the constant's body is what it is, or if it is replaced with `panic!()`. The latter will cause the ICE not to be emitted though. The reason for that is that we abort compilation if *errors* were emitted, but not if *lint errors* were emitted. This took me way too long to debug, and is another reason why I would have liked https://github.com/rust-lang/compiler-team/issues/633
`Diagnostic::code` has the type `DiagnosticId`, which has `Error` and
`Lint` variants. Plus `Diagnostic::is_lint` is a bool, which should be
redundant w.r.t. `Diagnostic::code`.
Seems simple. Except it's possible for a lint to have an error code, in
which case its `code` field is recorded as `Error`, and `is_lint` is
required to indicate that it's a lint. This is what happens with
`derive(LintDiagnostic)` lints. Which means those lints don't have a
lint name or a `has_future_breakage` field because those are stored in
the `DiagnosticId::Lint`.
It's all a bit messy and confused and seems unintentional.
This commit:
- removes `DiagnosticId`;
- changes `Diagnostic::code` to `Option<String>`, which means both
errors and lints can straightforwardly have an error code;
- changes `Diagnostic::is_lint` to `Option<IsLint>`, where `IsLint` is a
new type containing a lint name and a `has_future_breakage` bool, so
all lints can have those, error code or not.
When writing a no_std binary, you'll be greeted with nonsensical errors
mentioning lang items like eh_personality and start. That's pretty bad
because it makes you think that you need to define them somewhere! But
oh no, now you're getting the `internal_features` lint telling you that
you shouldn't use them! But you need a no_std binary! What now?
No problem! Writing a no_std binary is super easy. Just use panic=abort
and supply your own platform specific entrypoint symbol (like `main`)
and you're good to go. Would be nice if the compiler told you that,
right?
This makes it so that it does do that.
We have `span_delayed_bug` and often pass it a `DUMMY_SP`. This commit
adds `delayed_bug`, which matches pairs like `err`/`span_err` and
`warn`/`span_warn`.
The existing uses are replaced in one of three ways.
- In a function that also has calls to `emit`, just rearrange the code
so that exactly one of `delay_as_bug` or `emit` is called on every
path.
- In a function returning a `DiagnosticBuilder`, use
`downgrade_to_delayed_bug`. That's good enough because it will get
emitted later anyway.
- In `unclosed_delim_err`, one set of errors is being replaced with
another set, so just cancel the original errors.
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`.
rustc_span: Optimize syntax context comparisons
Including comparisons with root context.
- `eq_ctxt` doesn't require retrieving full `SpanData`, or taking the span interner lock twice.
- Checking `SyntaxContext` for "rootness" is cheaper than extracting a full outer `ExpnData` for it and checking *it* for rootness.
The internal lint for `eq_ctxt` is also tweaked to detect `a.ctxt() != b.ctxt()` in addition to `a.ctxt() == b.ctxt()`.
Rollup of 8 pull requests
Successful merges:
- #119151 (Hide foreign `#[doc(hidden)]` paths in import suggestions)
- #119350 (Imply outlives-bounds on lazy type aliases)
- #119354 (Make `negative_bounds` internal & fix some of its issues)
- #119506 (Use `resolutions(()).effective_visiblities` to avoid cycle errors in `report_object_error`)
- #119554 (Fix scoping for let chains in match guards)
- #119563 (Check yield terminator's resume type in borrowck)
- #119589 (cstore: Remove unnecessary locking from `CrateMetadata`)
- #119622 (never patterns: Document behavior of never patterns with macros-by-example)
r? `@ghost`
`@rustbot` modify labels: rollup
Fix scoping for let chains in match guards
If let guards were previously represented as a different type of guard in HIR and THIR. This meant that let chains in match guards were not handled correctly because they were treated exactly like normal guards.
- Remove `hir::Guard` and `thir::Guard`.
- Make the scoping different between normal guards and if let guards also check for let chains.
closes#118593
Replace a number of FxHashMaps/Sets with stable-iteration-order alternatives
This PR replaces almost all of the remaining `FxHashMap`s in query results with either `FxIndexMap` or `UnordMap`. The only case that is missing is the `EffectiveVisibilities` struct which turned out to not be straightforward to transform. Once that is done too, we can remove the `HashStable` implementation from `HashMap`.
The first commit adds the `StableCompare` trait which is a companion trait to `StableOrd`. Some types like `Symbol` can be compared in a cross-session stable way, but their `Ord` implementation is not stable. In such cases, a `StableCompare` implementation can be provided to offer a lightweight way for stable sorting. The more heavyweight option is to sort via `ToStableHashKey`, but then sorting needs to have access to a stable hashing context and `ToStableHashKey` can also be expensive as in the case of `Symbol` where it has to allocate a `String`.
The rest of the commits are rather mechanical and don't overlap, so they are best reviewed individually.
Part of [MCP 533](https://github.com/rust-lang/compiler-team/issues/533).
`Diagnostic` has 40 methods that return `&mut Self` and could be
considered setters. Four of them have a `set_` prefix. This doesn't seem
necessary for a type that implements the builder pattern. This commit
removes the `set_` prefixes on those four methods.
Also walk bindings created by if-let guards
This change makes the `unused_variables` lint pick up unused bindings created by if-let guards.
Fixes#119383
Make closures carry their own ClosureKind
Right now, we use the "`movability`" field of `hir::Closure` to distinguish a closure and a coroutine. This is paired together with the `CoroutineKind`, which is located not in the `hir::Closure`, but the `hir::Body`. This is strange and redundant.
This PR introduces `ClosureKind` with two variants -- `Closure` and `Coroutine`, which is put into `hir::Closure`. The `CoroutineKind` is thus removed from `hir::Body`, and `Option<Movability>` no longer needs to be a stand-in for "is this a closure or a coroutine".
r? eholk
And make all hand-written `IntoDiagnostic` impls generic, by using
`DiagnosticBuilder::new(dcx, level, ...)` instead of e.g.
`dcx.struct_err(...)`.
This means the `create_*` functions are the source of the error level.
This change will let us remove `struct_diagnostic`.
Note: `#[rustc_lint_diagnostics]` is added to `DiagnosticBuilder::new`,
it's necessary to pass diagnostics tests now that it's used in
`into_diagnostic` functions.
rustc_passes: Enforce `rustc::potential_query_instability` lint
Stop allowing `rustc::potential_query_instability` in all of `rustc_passes` and instead allow it on a case-by-case basis if it is safe. In this case, all instances of the lint are safe to allow.
Part of https://github.com/rust-lang/rust/issues/84447 which is E-help-wanted.
Renamings:
- find -> opt_hir_node
- get -> hir_node
- find_by_def_id -> opt_hir_node_by_def_id
- get_by_def_id -> hir_node_by_def_id
Fix rebase changes using removed methods
Use `tcx.hir_node_by_def_id()` whenever possible in compiler
Fix clippy errors
Fix compiler
Apply suggestions from code review
Co-authored-by: Vadim Petrochenkov <vadim.petrochenkov@gmail.com>
Add FIXME for `tcx.hir()` returned type about its removal
Simplify with with `tcx.hir_node_by_def_id`
Stop allowing `rustc::potential_query_instability` in all of
`rustc_passes` and instead allow it on a case-by-case basis if it is
safe. In this case, all instances of the lint are safe to allow.
Add `never_patterns` feature gate
This PR adds the feature gate and most basic parsing for the experimental `never_patterns` feature. See the tracking issue (https://github.com/rust-lang/rust/issues/118155) for details on the experiment.
`@scottmcm` has agreed to be my lang-team liaison for this experiment.
Currently we always do this:
```
use rustc_fluent_macro::fluent_messages;
...
fluent_messages! { "./example.ftl" }
```
But there is no need, we can just do this everywhere:
```
rustc_fluent_macro::fluent_messages! { "./example.ftl" }
```
which is shorter.
