PR #119097 made the decision to make all `IntoDiagnostic` impls generic,
because this allowed a bunch of nice cleanups. But four hand-written
impls were unintentionally overlooked. This commit makes them generic.
Currently `emit_stashed_diagnostic` is called from four(!) different
places: `print_error_count`, `DiagCtxtInner::drop`, `abort_if_errors`,
and `compile_status`.
And `flush_delayed` is called from two different places:
`DiagCtxtInner::drop` and `Queries`.
This is pretty gross! Each one should really be called from a single
place, but there's a bunch of entanglements. This commit cleans up this
mess.
Specifically, it:
- Removes all the existing calls to `emit_stashed_diagnostic`, and adds
a single new call in `finish_diagnostics`.
- Removes the early `flush_delayed` call in `codegen_and_build_linker`,
replacing it with a simple early return if delayed bugs are present.
- Changes `DiagCtxtInner::drop` and `DiagCtxtInner::flush_delayed` so
they both assert that the stashed diagnostics are empty (i.e.
processed beforehand).
- Changes `interface::run_compiler` so that any errors emitted during
`finish_diagnostics` (i.e. late-emitted stashed diagnostics) are
counted and cannot be overlooked. This requires adding
`ErrorGuaranteed` return values to several functions.
- Removes the `stashed_err_count` call in `analysis`. This is possible
now that we don't have to worry about calling `flush_delayed` early
from `codegen_and_build_linker` when stashed diagnostics are pending.
- Changes the `span_bug` case in `handle_tuple_field_pattern_match` to a
`delayed_span_bug`, because it now can be reached due to the removal
of the `stashed_err_count` call in `analysis`.
- Slightly changes the expected output of three tests. If no errors are
emitted but there are delayed bugs, the error count is no longer
printed. This is because delayed bugs are now always printed after the
error count is printed (or not printed, if the error count is zero).
There is a lot going on in this commit. It's hard to break into smaller
pieces because the existing code is very tangled. It took me a long time
and a lot of effort to understand how the different pieces interact, and
I think the new code is a lot simpler and easier to understand.
Currently `has_errors` excludes lint errors. This commit changes it to
include lint errors.
The motivation for this is that for most places it doesn't matter
whether lint errors are included or not. But there are multiple places
where they must be includes, and only one place where they must not be
included. So it makes sense for `has_errors` to do the thing that fits
the most situations, and the new `has_errors_excluding_lint_errors`
method in the one exceptional place.
The same change is made for `err_count`. Annoyingly, this requires the
introduction of `err_count_excluding_lint_errs` for one place, to
preserve existing error printing behaviour. But I still think the change
is worthwhile overall.
Overhaul `Diagnostic` and `DiagnosticBuilder`
Implements the first part of https://github.com/rust-lang/compiler-team/issues/722, which moves functionality and use away from `Diagnostic`, onto `DiagnosticBuilder`.
Likely follow-ups:
- Move things around, because this PR was written to minimize diff size, so some things end up in sub-optimal places. E.g. `DiagnosticBuilder` has impls in both `diagnostic.rs` and `diagnostic_builder.rs`.
- Rename `Diagnostic` as `DiagInner` and `DiagnosticBuilder` as `Diag`.
r? `@davidtwco`
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.
There are lots of functions that modify a diagnostic. This can be via a
`&mut Diagnostic` or a `&mut DiagnosticBuilder`, because the latter type
wraps the former and impls `DerefMut`.
This commit converts all the `&mut Diagnostic` occurrences to `&mut
DiagnosticBuilder`. This is a step towards greatly simplifying
`Diagnostic`. Some of the relevant function are made generic, because
they deal with both errors and warnings. No function bodies are changed,
because all the modifier methods are available on both `Diagnostic` and
`DiagnosticBuilder`.
Tracking import use types for more accurate redundant import checking
fixes#117448
By tracking import use types to check whether it is scope uses or the other situations like module-relative uses, we can do more accurate redundant import checking.
For example unnecessary imports in std::prelude that can be eliminated:
```rust
use std::option::Option::Some;//~ WARNING the item `Some` is imported redundantly
use std::option::Option::None; //~ WARNING the item `None` is imported redundantly
```
fixes#117448
For example unnecessary imports in std::prelude that can be eliminated:
```rust
use std::option::Option::Some;//~ WARNING the item `Some` is imported redundantly
use std::option::Option::None; //~ WARNING the item `None` is imported redundantly
```
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```
Add clippy into the known `cfg` list
In clippy, we are removing the `feature = "cargo-clippy"` cfg to replace it with `clippy` in https://github.com/rust-lang/rust-clippy/pull/12292. But for it to work, we need to declare `clippy` as cfg. It makes it more coherent with other existing tools like rustdoc.
cc `@flip1995`
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>
`cook_lexer_literal` can emit an error about an invalid int literal but
then return a non-`Err` token. And then `integer_lit` has to account for
this to avoid printing a redundant error message.
This commit changes `cook_lexer_literal` to return `Err` in that case.
Then `integer_lit` doesn't need the special case, and
`LitError::LexerError` can be removed.
It's only has a single remaining purpose: to ensure that a diagnostic is
printed when `trimmed_def_paths` is used. It's an annoying mechanism:
weak, with odd semantics, badly named, and gets in the way of other
changes.
This commit replaces it with a simpler `must_produce_diag` mechanism,
getting rid of a diagnostic `Level` along the way.
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````
Remove some `unchecked_claim_error_was_emitted` calls
We want to drive the number of these calls down as much as possible. This PR gets rid of a bunch of them.
r? ``@oli-obk``
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.
`emit_future_breakage` calls
`self.dcx().take_future_breakage_diagnostics()` and then passes the
result to `self.dcx().emit_future_breakage_report(diags)`. This commit
removes the first of these and lets `emit_future_breakage_report` do the
taking.
It also inlines and removes what is left of `emit_future_breakage`,
which has a single call site.
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.
Add the unstable option to reduce the binary size of dynamic library…
# Motivation
The average length of symbol names in the rust standard library is about 100 bytes, while the average length of symbol names in the C++ standard library is about 65 bytes. In some embedded environments where dynamic library are widely used, rust dynamic library symbol name space hash become one of the key bottlenecks of application, Especially when the existing C/C++ module is reconstructed into the rust module.
The unstable option `-Z symbol_mangling_version=hashed` is added to solve the bottleneck caused by too long dynamic library symbol names.
## Test data
The following is a set of test data on the ubuntu 18.04 LTS environment. With this plug-in, the space saving rate of dynamic libraries can reach about 20%.
The test object is the standard library of rust (built based on Xargo), tokio crate, and hyper crate.
The contents of the Cargo.toml file in the construction project of the three dynamic libraries are as follows:
```txt
# Cargo.toml
[profile.release]
panic = "abort"
opt-leve="z"
codegen-units=1
strip=true
debug=true
```
The built dynamic library also removes the `.rustc` segments that are not needed at run time and then compares the size. The detailed data is as follows:
1. libstd.so
> | symbol_mangling_version | size | saving rate |
> | --- | --- | --- |
> | legacy | 804896 ||
> | hashed | 608288 | 0.244 |
> | v0 | 858144 ||
> | hashed | 608288 | 0.291 |
2. libhyper.so
> | symbol_mangling_version(libhyper.so) | symbol_mangling_version(libstd.so) | size | saving rate |
> | --- | --- | --- | --- |
> | legacy | legacy | 866312 ||
> | hashed | legacy | 645128 |0.255|
> | legacy | hashed | 854024 ||
> | hashed | hashed | 632840 |0.259|
Track `verbose` and `verbose_internals`
`verbose_internals` has been UNTRACKED since it was introduced. When i added `verbose` in https://github.com/rust-lang/rust/pull/119129 i made it UNTRACKED as well.
``@bjorn3`` says: https://github.com/rust-lang/rust/pull/119286#discussion_r1436134354
> On errors we don't finalize the incr comp cache, but non-fatal diagnostics are cached afaik.
Otherwise we would have to replay the query in question, which we may not be able to do if the query key is not reconstructible from the dep node fingerprint.
So we must track these flags to avoid replaying incorrect diagnostics.
r? incremental
We have several methods indicating the presence of errors, lint errors,
and delayed bugs. I find it frustrating that it's very unclear which one
you should use in any particular spot. This commit attempts to instill a
basic principle of "use the least general one possible", because that
reflects reality in practice -- `has_errors` is the least general one
and has by far the most uses (esp. via `abort_if_errors`).
Specifics:
- Add some comments giving some usage guidelines.
- Prefer `has_errors` to comparing `err_count` to zero.
- Remove `has_errors_or_span_delayed_bugs` because it's a weird one: in
the cases where we need to count delayed bugs, we should really be
counting lint errors as well.
- Rename `is_compilation_going_to_fail` as
`has_errors_or_lint_errors_or_span_delayed_bugs`, for consistency with
`has_errors` and `has_errors_or_lint_errors`.
- Change a few other `has_errors_or_lint_errors` calls to `has_errors`,
as per the "least general" principle.
This didn't turn out to be as neat as I hoped when I started, but I
think it's still an improvement.
