Lint buffering currently relies on a giant enum `BuiltinLintDiag`
containing all the lints that might potentially get buffered. In
addition to being an unwieldy enum in a central crate, this also makes
`rustc_lint_defs` a build bottleneck: it depends on various types from
various crates (with a steady pressure to add more), and many crates
depend on it.
Having all of these variants in a separate crate also prevents detecting
when a variant becomes unused, which we can do with a dedicated type
defined and used in the same crate.
Refactor this to use a dyn trait, to allow using `LintDiagnostic` types
directly.
This requires boxing, but all of this is already on the slow path
(emitting an error).
Because the existing `BuiltinLintDiag` requires some additional types in
order to decorate some variants, which are only available later in
`rustc_lint`, use an enum `DecorateDiagCompat` to handle both the `dyn
LintDiagnostic` case and the `BuiltinLintDiag` case.
Some crates depend on `rustc_hir` but only want `HirId` and similar id
types. `rustc_hir` is a heavy dependency, since it pulls in
`rustc_target`. Split these types out into their own crate
`rustc_hir_id`.
This allows `rustc_errors` to drop its direct dependency on `rustc_hir`.
(`rustc_errors` still depends on `rustc_hir` indirectly through
`rustc_lint_defs`; a subsequent commit will fix that.)
`rustc_errors` depends on numerous crates, solely to implement its
`IntoDiagArg` trait on types from those crates. Many crates depend on
`rustc_errors`, and it's on the critical path.
We can't swap things around to make all of those crates depend on
`rustc_errors` instead, because `rustc_errors` would end up in
dependency cycles.
Instead, move `IntoDiagArg` into `rustc_error_messages`, which has far
fewer dependencies, and then have most of these crates depend on
`rustc_error_messages`.
This allows `rustc_errors` to drop dependencies on several crates,
including the large `rustc_target`.
(This doesn't fully reduce dependency chains yet, as `rustc_errors`
still depends on `rustc_hir` which depends on `rustc_target`. That will
get fixed in a subsequent commit.)
Extend `QueryStability` to handle `IntoIterator` implementations
This PR extends the `rustc::potential_query_instability` lint to check values passed as `IntoIterator` implementations.
Full disclosure: I want the lint to warn about this line (please see #138871 for why): aa8f0fd716/src/librustdoc/json/mod.rs (L261)
However, the lint warns about several other lines as well.
Final note: the functions `get_callee_generic_args_and_args` and `get_input_traits_and_projections` were copied directly from [Clippy's source code](4fd8c04da0/src/tools/clippy/clippy_lints/src/methods/unnecessary_to_owned.rs (L445-L496)).
Fix adjacent code
Fix duplicate warning; merge test into `tests/ui-fulldeps/internal-lints`
Use `rustc_middle::ty::FnSig::inputs`
Address two review comments
- https://github.com/rust-lang/rust/pull/139345#discussion_r2109006991
- https://github.com/rust-lang/rust/pull/139345#discussion_r2109058588
Use `Instance::try_resolve`
Import `rustc_middle::ty::Ty` as `Ty` rather than `MiddleTy`
Simplify predicate handling
Add more `#[allow(rustc::potential_query_instability)]` following rebase
Remove two `#[allow(rustc::potential_query_instability)]` following rebase
Address review comment
Update compiler/rustc_lint/src/internal.rs
Co-authored-by: lcnr <rust@lcnr.de>
Include whitespace in "remove |" suggestion and make it hidden
Tweak error rendering of patterns with an extra `|` on either end.
Built on #137409. Only last commit is relevant.
? ``@compiler-errors``
Point at the `Fn()` or `FnMut()` bound that coerced a closure, which caused a move error
When encountering a move error involving a closure because the captured value isn't `Copy`, and the obligation comes from a bound on a type parameter that requires `Fn` or `FnMut`, we point at it and explain that an `FnOnce` wouldn't cause the move error.
```
error[E0507]: cannot move out of `foo`, a captured variable in an `Fn` closure
--> f111.rs:15:25
|
14 | fn do_stuff(foo: Option<Foo>) {
| --- ----------- move occurs because `foo` has type `Option<Foo>`, which does not implement the `Copy` trait
| |
| captured outer variable
15 | require_fn_trait(|| async {
| -- ^^^^^ `foo` is moved here
| |
| captured by this `Fn` closure
16 | if foo.map_or(false, |f| f.foo()) {
| --- variable moved due to use in coroutine
|
help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
--> f111.rs:12:53
|
12 | fn require_fn_trait<F: Future<Output = ()>>(_: impl Fn() -> F) {}
| ^^^^^^^^^
help: consider cloning the value if the performance cost is acceptable
|
16 | if foo.clone().map_or(false, |f| f.foo()) {
| ++++++++
```
Fixrust-lang/rust#68119, by pointing at `Fn` and `FnMut` bounds involved in move errors.
Use `eq_ignore_ascii_case` to avoid heap alloc in `detect_confuse_type`
A small optimization has been made, using `to_ascii_lowercase()` instead of `to_lowercase().to_string()`.
r? compiler
When encountering a move error involving a closure because the captured value isn't `Copy`, and the obligation comes from a bound on a type parameter that requires `Fn` or `FnMut`, we point at it and explain that an `FnOnce` wouldn't cause the move error.
```
error[E0507]: cannot move out of `foo`, a captured variable in an `Fn` closure
--> f111.rs:15:25
|
14 | fn do_stuff(foo: Option<Foo>) {
| --- ----------- move occurs because `foo` has type `Option<Foo>`, which does not implement the `Copy` trait
| |
| captured outer variable
15 | require_fn_trait(|| async {
| -- ^^^^^ `foo` is moved here
| |
| captured by this `Fn` closure
16 | if foo.map_or(false, |f| f.foo()) {
| --- variable moved due to use in coroutine
|
help: `Fn` and `FnMut` closures require captured values to be able to be consumed multiple times, but an `FnOnce` consume them only once
--> f111.rs:12:53
|
12 | fn require_fn_trait<F: Future<Output = ()>>(_: impl Fn() -> F) {}
| ^^^^^^^^^
help: consider cloning the value if the performance cost is acceptable
|
16 | if foo.clone().map_or(false, |f| f.foo()) {
| ++++++++
```
Implement declarative (`macro_rules!`) attribute macros (RFC 3697)
This implements [RFC 3697](https://github.com/rust-lang/rust/issues/143547), "Declarative (`macro_rules!`) attribute macros".
I would suggest reading this commit-by-commit. This first introduces the
feature gate, then adds parsing for attribute rules (doing nothing with them),
then adds the ability to look up and apply `macro_rules!` attributes by path,
then adds support for local attributes, then adds a test, and finally makes
various improvements to errors.
`TyCtxt::short_string` ensures that user visible type paths aren't overwhelming on the terminal output, and properly saves the long name to disk as a side-channel. We already use these throughout the compiler and have been using them as needed when users find cases where the output is verbose. This is a proactive search of some cases to use `short_string`.
We add support for shortening the path of "trait path only".
Every manual use of `short_string` is a bright marker that that error should be using structured diagnostics instead (as they have proper handling of long types without the maintainer having to think abou tthem).
When we don't actually print out a shortened type we don't need the "use `--verbose`" note.
On E0599 show type identity to avoid expanding the receiver's generic parameters.
Unify wording on `long_ty_path` everywhere.
Extend `is_case_difference` to handle digit-letter confusables
This PR extends `is_case_difference` to handle digit-letter confusables
Add support for detecting 0/O, 1/l, 5/S, 8/B, 9/g confusables in error suggestions.
r? `@estebank`
chore: Improve how the other suggestions message gets rendered
Note: This change is part of my ongoing work to use `annotate-snippets` as `rustc`'s emitter
This change started as a way to remove some specialty code paths from `annotate-snippets`, by making the "and {} other candidates" message get rendered like a secondary message with no level, but turned into a fix for the message's Unicode output. Before this change, when using the Unicode output, the other suggestions message would get rendered outside of the main suggestion block, making it feel disconnected from what it was referring to. This change makes it so that the message is on the last line of the block, aligning its rendering with other secondary messages, and making it clear what the message is referring to.
Before:
```
error[E0433]: failed to resolve: use of undeclared type `IntoIter`
╭▸ $DIR/issue-82956.rs:28:24
│
LL │ let mut iter = IntoIter::new(self);
│ ━━━━━━━━ use of undeclared type `IntoIter`
╰╴
help: consider importing one of these structs
╭╴
LL + use std::array::IntoIter;
├╴
LL + use std::collections::binary_heap::IntoIter;
├╴
LL + use std::collections::btree_map::IntoIter;
├╴
LL + use std::collections::btree_set::IntoIter;
╰╴
and 9 other candidates
```
After:
```
error[E0433]: failed to resolve: use of undeclared type `IntoIter`
╭▸ $DIR/issue-82956.rs:28:24
│
LL │ let mut iter = IntoIter::new(self);
│ ━━━━━━━━ use of undeclared type `IntoIter`
╰╴
help: consider importing one of these structs
╭╴
LL + use std::array::IntoIter;
├╴
LL + use std::collections::binary_heap::IntoIter;
├╴
LL + use std::collections::btree_map::IntoIter;
├╴
LL + use std::collections::btree_set::IntoIter;
│
╰ and 9 other candidates
```
Make -Ztrack-diagnostics emit like a note
[#t-compiler/diagnostics > Rendering -Ztrack-diagnostics like a note](https://rust-lang.zulipchat.com/#narrow/channel/147480-t-compiler.2Fdiagnostics/topic/Rendering.20-Ztrack-diagnostics.20like.20a.20note/with/526608647)
As discussed on the Zulip thread above, I want to make `-Ztrack-diagnostics` emit like a `note`. This is because I find its current output jarring, and the fact that it gets rendered completely left-aligned, [even in the middle of a snippet](86e05cd300/tests/ui/track-diagnostics/track6.stderr), seems like something that should be changed. Turning it into a `note` seems like the best choice, as it would align it with the rest of the output, and `note` is already used for somewhat similar things, like seeing why a lint was fired.
---
Note: turning `-Ztrack-diagnostics` into a `note` will also make `annotate-snippets` API a bit cleaner
fix: Emit suggestion filename if primary diagnostic span is dummy
While working on using `annotate-snippets` as `rustc`'s emitter, I came across [`tests/ui/resolve/resolve-conflict-extern-crate-vs-extern-crate.stderr`](b03b3a7ec9/tests/ui/resolve/resolve-conflict-extern-crate-vs-extern-crate.stderr), which lacked a filename for both the annotation and the suggestion, which seemed like a bug to me. After some investigation, I found that this is happening because the primary span is a dummy, so its filename doesn't get output, and its filename matches the one for the suggestion, so the suggestion's filename doesn't get output. To fix this issue, I made it so that the filename for a suggestion will get output if the primary span is a dummy.
Add runtime check to avoid overwrite arg in `Diag`
## Origin PR description
At first, I set up a `debug_assert` check for the arg method to make sure that `args` in `Diag` aren't easily overwritten, and I added the `remove_arg()` method, so that if you do need to overwrite an arg, then you can explicitly call `remove_arg()` to remove it first, then call `arg()` to overwrite it.
For the code before the rust-lang/rust#142015 change, it won't compile because it will report an error
```
arg `instance`already exists.
```
This PR also modifies all diagnostics that fail the check to pass the check. There are two cases of check failure:
1. ~~Between *the parent diagnostic and the subdiagnostic*, or *between the subdiagnostics* have the same field between them. In this case, I renamed the conflicting fields.~~
2. ~~For subdiagnostics stored in `Vec`, the rendering may iteratively write the same arg over and over again. In this case, I changed the auto-generation with `derive(SubDiagnostic)` to manually implementing `SubDiagnostic` and manually rendered it with `eagerly_translate()`, similar to https://github.com/rust-lang/rust/issues/142031#issuecomment-2984812090, and after rendering it I manually deleted useless arg with the newly added `remove_arg` method.~~
## Final Decision
After trying and discussing, we made a final decision.
For `#[derive(Subdiagnostic)]`, This PR made two changes:
1. After the subdiagnostic is rendered, remove all args of this subdiagnostic, which allows for usage like `Vec<Subdiag>`.
2. Store `diag.args` before setting arguments, so that you can restore the contents of the main diagnostic after deleting the arguments after subdiagnostic is rendered, to avoid deleting the main diagnostic's arg when they have the same name args.
Add codegen timing section
And since we now start and end the sections also using separate functions, also add some light checking if we're generating the sections correctly.
I'm integrating `--timings` into Cargo, and I realized that the codegen timings would be quite useful for that. Frontend can be computed simply as `[start of compilation, start of codegen]` for now.
r? `@nnethercote`
Implement initial support for timing sections (`--json=timings`)
This PR implements initial support for emitting high-level compilation section timings. The idea is to provide a very lightweight way of emitting durations of various compilation sections (frontend, backend, linker, or on a more granular level macro expansion, typeck, borrowck, etc.). The ultimate goal is to stabilize this output (in some form), make Cargo pass `--json=timings` and then display this information in the HTML output of `cargo build --timings`, to make it easier to quickly profile "what takes so long" during the compilation of a Cargo project. I would personally also like if Cargo printed some of this information in the interactive `cargo build` output, but the `build --timings` use-case is the main one.
Now, this information is already available with several other sources, but I don't think that we can just use them as they are, which is why I proposed a new way of outputting this data (`--json=timings`):
- This data is available under `-Zself-profile`, but that is very expensive and forever unstable. It's just a too big of a hammer to tell us the duration it took to run the linker.
- It could also be extracted with `-Ztime-passes`. That is pretty much "for free" in terms of performance, and it can be emitted in a structured form to JSON via `-Ztime-passes-format=json`. I guess that one alternative might be to stabilize this flag in some form, but that form might just be `--json=timings`? I guess what we could do in theory is take the already emitted time passes and reuse them for `--json=timings`. Happy to hear suggestions!
I'm sending this PR mostly for a vibeck, to see if the way I implemented it is passable. There are some things to figure out:
- How do we represent the sections? Originally I wanted to output `{ section, duration }`, but then I realized that it might be more useful to actually emit `start` and `end` events. Both because it enables to see the output incrementally (in case compilation takes a long time and you read the outputs directly, or Cargo decides to show this data in `cargo build` some day in the future), and because it makes it simpler to represent hierarchy (see below). The timestamps currently emit microseconds elapsed from a predetermined point in time (~start of rustc), but otherwise they are fully opaque, and should be only ever used to calculate the duration using `end - start`. We could also precompute the duration for the user in the `end` event, but that would require doing more work in rustc, which I would ideally like to avoid :P
- Do we want to have some form of hierarchy? I think that it would be nice to show some more granular sections rather than just frontend/backend/linker (e.g. macro expansion, typeck and borrowck as a part of the frontend). But for that we would need some way of representing hierarchy. A simple way would be something like `{ parent: "frontend" }`, but I realized that with start/end timestamps we get the hierarchy "for free", only the client will need to reconstruct it from the order of start/end events (e.g. `start A`, `start B` means that `B` is a child of `A`).
- What exactly do we want to stabilize? This is probably a question for later. I think that we should definitely stabilize the format of the emitted JSON objects, and *maybe* some specific section names (but we should also make it clear that they can be missing, e.g. you don't link everytime you invoke `rustc`).
The PR be tested e.g. with `rustc +stage1 src/main.rs --json=timings --error-format=json -Zunstable-options` on a crate without dependencies (it is not easy to use `--json` with stock Cargo, because it also passes this flag to `rustc`, so this will later need Cargo integration to be usable with it).
Zulip discussions: [#t-compiler > Outputting time spent in various compiler sections](https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Outputting.20time.20spent.20in.20various.20compiler.20sections/with/518850162)
MCP: https://github.com/rust-lang/compiler-team/issues/873
r? ``@nnethercote``
