warning: `peek` never called on `Peekable` iterator
--> compiler\rustc_session\src\utils.rs:130:13
|
130 | let mut args = std::env::args_os().map(|arg| arg.to_string_lossy().to_string()).peekable();
| ^^^^
|
= help: consider removing the call to `peekable`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_peekable
warning: `peek` never called on `Peekable` iterator
--> compiler\rustc_trait_selection\src\traits\error_reporting\suggestions.rs:4934:17
|
4934 | let mut bounds = pred.bounds.iter().peekable();
| ^^^^^^
|
= help: consider removing the call to `peekable`
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unused_peekable
Don't emit an error about failing to produce a file with a specific name if user never gave an explicit name
Fixes#122509
You can ask `rustc` to produce some intermediate results with `--emit foo`, this operation comes in two flavors: `--emit asm` and `--emit asm=foo.s`. First one produces one or more `.s` files without any name guarantees, second one renames it into `foo.s`. Second version only works when compiler produces a single file - for asm files this means using a single compilation unit for example.
In case compilation produced more than a single file `rustc` runs following check to emit some warnings:
```rust
if crate_output.outputs.contains_key(&output_type) {
// 2) Multiple codegen units, with `--emit foo=some_name`. We have
// no good solution for this case, so warn the user.
sess.dcx().emit_warn(errors::IgnoringEmitPath { extension });
} else if crate_output.single_output_file.is_some() {
// 3) Multiple codegen units, with `-o some_name`. We have
// no good solution for this case, so warn the user.
sess.dcx().emit_warn(errors::IgnoringOutput { extension });
} else {
// 4) Multiple codegen units, but no explicit name. We
// just leave the `foo.0.x` files in place.
// (We don't have to do any work in this case.)
}
```
Comment in the final `else` branch implies that if user didn't ask for a specific name - there's no need to emit warnings. However because of the internal representation of `crate_output.outputs` - this doesn't work as expected: if user asked to produce an asm file without giving it an implicit name it will contain `Some(None)`.
To fix the problem new code actually checks if user gave an explicit name. I think this was an original intentional behavior, at least comments imply that.
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 https://github.com/rust-lang/rustfmt/issues/6109.
These changes allow rustfmt to maintain its previous behaviour when using the `SilentEmitter`, while allowing rustc code to still emit fatal diagnostics.
This should assist comprehending the size of coroutines.
In particular, whenever a future is suspended while awaiting another
future, the latter is given the special name `__awaitee`, and now the
type of the awaited future will be printed, allowing identifying
caller/callee — er, I mean, poller/pollee — relationships.
It would be possible to include the type name in more cases, but I
thought that that might be overly verbose (`print-type-sizes` is already
a lot of text) and ordinary named fields or variables are easier for
readers to discover the types of.
Backend and target selection is a mess: the target can override the
backend (via `Target::default_codegen_backend`), *and* the backend can
override the target (via `CodegenBackend::target_override`).
The code that handles this is ugly. It calls `build_target_config`
twice, once before getting the backend and once again afterward. It also
must check that both overrides aren't triggering at the same time.
This commit removes the latter override. It's used in rust-gpu but
@eddyb said via Zulip that removing it would be ok. This simplifies the
code greatly, and will allow some nice follow-up refactorings.
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.
Provide structured suggestion for `#![feature(foo)]`
```
error: `S2<'_>` is forbidden as the type of a const generic parameter
--> $DIR/lifetime-in-const-param.rs:5:23
|
LL | struct S<'a, const N: S2>(&'a ());
| ^^
|
= note: the only supported types are integers, `bool` and `char`
help: add `#![feature(adt_const_params)]` to the crate attributes to enable more complex and user defined types
|
LL + #![feature(adt_const_params)]
|
```
Fix#55941.
```
error: `S2<'_>` is forbidden as the type of a const generic parameter
--> $DIR/lifetime-in-const-param.rs:5:23
|
LL | struct S<'a, const N: S2>(&'a ());
| ^^
|
= note: the only supported types are integers, `bool` and `char`
help: add `#![feature(adt_const_params)]` to the crate attributes to enable more complex and user defined types
|
LL + #![feature(adt_const_params)]
|
```
Fix#55941.
Add `-Z external-clangrt`
This adds the unstable `-Z external-clangrt` flag that will prevent rustc from emitting linker paths for the in-tree LLVM sanitizer runtime library.
coverage: Initial support for branch coverage instrumentation
(This is a review-ready version of the changes that were drafted in #118305.)
This PR adds support for branch coverage instrumentation, gated behind the unstable flag value `-Zcoverage-options=branch`. (Coverage instrumentation must also be enabled with `-Cinstrument-coverage`.)
During THIR-to-MIR lowering (MIR building), if branch coverage is enabled, we collect additional information about branch conditions and their corresponding then/else blocks. We inject special marker statements into those blocks, so that the `InstrumentCoverage` MIR pass can reliably identify them even after the initially-built MIR has been simplified and renumbered.
The rest of the changes are mostly just plumbing needed to gather up the information that was collected during MIR building, and include it in the coverage metadata that we embed in the final binary.
Note that `llvm-cov show` doesn't print branch coverage information in its source views by default; that needs to be explicitly enabled with `--show-branches=count` or similar.
---
The current implementation doesn't have any support for instrumenting `if let` or let-chains. I think it's still useful without that, and adding it would be non-trivial, so I'm happy to leave that for future work.
Make incremental sessions identity no longer depend on the crate names provided by source code
This makes incremental sessions identity no longer depend on the crate names provided by source code, implementing
https://github.com/rust-lang/compiler-team/issues/726.
r? ````@oli-obk````
This adds the unstable `-Z external-sanitizer-runtime` flag that will
prevent rustc from emitting linker paths for the in-tree LLVM sanitizer
runtime library.
coverage: Remove or migrate all unstable values of `-Cinstrument-coverage`
(This PR was substantially overhauled from its original version, which migrated all of the existing unstable values intact.)
This PR takes the three nightly-only values that are currently accepted by `-Cinstrument-coverage`, completely removes two of them (`except-unused-functions` and `except-unused-generics`), and migrates the third (`branch`) over to a newly-introduced unstable flag `-Zcoverage-options`.
I have a few motivations for wanting to do this:
- It's unclear whether anyone actually uses the `except-unused-*` values, so this serves as an opportunity to either remove them, or prompt existing users to object to their removal.
- After #117199, the stable values of `-Cinstrument-coverage` treat it as a boolean-valued flag, so having nightly-only extra values feels out-of-place.
- Nightly-only values also require extra ad-hoc code to make sure they aren't accidentally exposed to stable users.
- The new system allows multiple different settings to be toggled independently, which isn't possible in the current single-value system.
- The new system makes it easier to introduce new behaviour behind an unstable toggle, and then gather nightly-user feedback before possibly making it the default behaviour for all users.
- The new system also gives us a convenient place to put relatively-narrow options that won't ever be the default, but that nightly users might still want access to.
- It's likely that we will eventually want to give stable users more fine-grained control over coverage instrumentation. The new flag serves as a prototype of what that stable UI might eventually look like.
The `branch` option is a placeholder that currently does nothing. It will be used by #122322 to opt into branch coverage instrumentation.
---
I see `-Zcoverage-options` as something that will exist more-or-less indefinitely, though individual sub-options might come and go as appropriate. I think there will always be some demand for nightly-only toggles, so I don't see `-Zcoverage-options` itself ever being stable, though we might eventually stabilize something similar to it.
Verify that query keys result in unique dep nodes
This implements checking that query keys result into unique dep nodes as mentioned in https://github.com/rust-lang/rust/pull/112469.
We could do a perf check to see how expensive this is.
r? `@michaelwoerister`
This new nightly-only flag can be used to toggle fine-grained flags that
control the details of coverage instrumentation.
Currently the only supported flag value is `branch` (or `no-branch`), which is
a placeholder for upcoming support for branch coverage. Other flag values can
be added in the future, to prototype proposed new behaviour, or to enable
special non-default behaviour.
Removing absolute path in proc-macro
With rust 1.75 the absolute build path name is embedding into proc-macro (.rustc section) and which causes reproducibility issues.
Detailed issue description is here - https://github.com/rust-lang/rust/issues/120825#issuecomment-1964307219
With this change the 'absolute path' changed back to '/rust/$hash' format as in earlier revisions.
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``
Prior to the previous commit, `#[rust_lint_diagnostics]` attributes
could only be used on methods with an `impl Into<{D,Subd}iagMessage>`
parameter. But there are many other nearby diagnostic methods (e.g.
`Diag::span`) that don't take such a parameter and should have the
attribute.
This commit adds the missing attribute to these `Diag` methods. This
requires adding some missing
`#[allow(rustc::diagnostic_outside_of_impl)]` markers at call sites to
these methods.
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`.
errors: share `SilentEmitter` between rustc and rustfmt
Fixesrust-lang/rustfmt#6082.
Shares the `SilentEmitter` between rustc and rustfmt, and gives it a fallback bundle (since it can emit diagnostics in some contexts).
Existing names for values of this type are `sess`, `parse_sess`,
`parse_session`, and `ps`. `sess` is particularly annoying because
that's also used for `Session` values, which are often co-located, and
it can be difficult to know which type a value named `sess` refers to.
(That annoyance is the main motivation for this change.) `psess` is nice
and short, which is good for a name used this much.
The commit also renames some `parse_sess_created` values as
`psess_created`.
With rust 1.75 the absolute build path is embedding into '.rustc' section and which causes reproducibility issues. Detailed issue is here.
https://github.com/rust-lang/rust/issues/120825#issuecomment-1964307219
With this change the 'absolute path' changed back to '/rust/$hash' format.
Adds initial support for DataFlowSanitizer to the Rust compiler. It
currently supports `-Zsanitizer-dataflow-abilist`. Additional options
for it can be passed to LLVM command line argument processor via LLVM
arguments using `llvm-args` codegen option (e.g.,
`-Cllvm-args=-dfsan-combine-pointer-labels-on-load=false`).
Emitter cleanups
Some cleanups I made when reading emitter code. In particular, `HumanEmitter` and `JsonEmitter` have gone from three constructors to one.
r? `@oli-obk`
Stashed errors used to be counted as errors, but could then be
cancelled, leading to `ErrorGuaranteed` soundness holes. #120828 changed
that, closing the soundness hole. But it introduced other difficulties
because you sometimes have to account for pending stashed errors when
making decisions about whether errors have occured/will occur and it's
easy to overlook these.
This commit aims for a middle ground.
- Stashed errors (not warnings) are counted immediately as emitted
errors, avoiding the possibility of forgetting to consider them.
- The ability to cancel (or downgrade) stashed errors is eliminated, by
disallowing the use of `steal_diagnostic` with errors, and introducing
the more restrictive methods `try_steal_{modify,replace}_and_emit_err`
that can be used instead.
Other things:
- `DiagnosticBuilder::stash` and `DiagCtxt::stash_diagnostic` now both
return `Option<ErrorGuaranteed>`, which enables the removal of two
`delayed_bug` calls and one `Ty::new_error_with_message` call. This is
possible because we store error guarantees in
`DiagCtxt::stashed_diagnostics`.
- Storing the guarantees also saves us having to maintain a counter.
- Calls to the `stashed_err_count` method are no longer necessary
alongside calls to `has_errors`, which is a nice simplification, and
eliminates two more `span_delayed_bug` calls and one FIXME comment.
- Tests are added for three of the four fixed PRs mentioned below.
- `issue-121108.rs`'s output improved slightly, omitting a non-useful
error message.
Fixes#121451.
Fixes#121477.
Fixes#121504.
Fixes#121508.
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.
rustc_codegen_llvm: add support for writing summary bitcode
Typical uses of ThinLTO don't have any use for this as a standalone file, but distributed ThinLTO uses this to make the linker phase more efficient. With clang you'd do something like `clang -flto=thin -fthin-link-bitcode=foo.indexing.o -c foo.c` and then get both foo.o (full of bitcode) and foo.indexing.o (just the summary or index part of the bitcode). That's then usable by a two-stage linking process that's more friendly to distributed build systems like bazel, which is why I'm working on this area.
I talked some to `@teresajohnson` about naming in this area, as things seem to be a little confused between various blog posts and build systems. "bitcode index" and "bitcode summary" tend to be a little too ambiguous, and she tends to use "thin link bitcode" and "minimized bitcode" (which matches the descriptions in LLVM). Since the clang option is thin-link-bitcode, I went with that to try and not add a new spelling in the world.
Per `@dtolnay,` you can work around the lack of this by using `lld --thinlto-index-only` to do the indexing on regular .o files of bitcode, but that is a bit wasteful on actions when we already have all the information in rustc and could just write out the matching minimized bitcode. I didn't test that at all in our infrastructure, because by the time I learned that I already had this patch largely written.
rust-lld: fallback to rustc's sysroot if there's no path to the linker in the target sysroot
As seen in #125246, some sysroots don't expect to contain `rust-lld` and want to keep it that way, so we fallback to the default rustc sysroot if there is no path to the linker in any of the sysroot tools search paths. This is how we locate codegen-backends' dylibs already.
People also have requested an error if none of these search paths contain the self-contained linker directory, so there's also an error in that case.
r? `@petrochenkov` cc `@ehuss` `@RalfJung`
I'm not sure where we check for `rust-lld`'s existence on the targets where we use it by default, and if we just ignore it when missing or emit a warning (as I assume we don't emit an error), so I just checked for the existence of `gcc-ld`, where `cc` will look for the lld-wrapper binaries.
<sub>*Feel free to point out better ways to do this, it's the middle of the night here.*</sub>
Fixes#125246
Typical uses of ThinLTO don't have any use for this as a standalone
file, but distributed ThinLTO uses this to make the linker phase more
efficient. With clang you'd do something like `clang -flto=thin
-fthin-link-bitcode=foo.indexing.o -c foo.c` and then get both foo.o
(full of bitcode) and foo.indexing.o (just the summary or index part of
the bitcode). That's then usable by a two-stage linking process that's
more friendly to distributed build systems like bazel, which is why I'm
working on this area.
I talked some to @teresajohnson about naming in this area, as things
seem to be a little confused between various blog posts and build
systems. "bitcode index" and "bitcode summary" tend to be a little too
ambiguous, and she tends to use "thin link bitcode" and "minimized
bitcode" (which matches the descriptions in LLVM). Since the clang
option is thin-link-bitcode, I went with that to try and not add a new
spelling in the world.
Per @dtolnay, you can work around the lack of this by using `lld
--thinlto-index-only` to do the indexing on regular .o files of
bitcode, but that is a bit wasteful on actions when we already have all
the information in rustc and could just write out the matching minimized
bitcode. I didn't test that at all in our infrastructure, because by the
time I learned that I already had this patch largely written.
Translation of the lint message happens when the actual diagnostic is
created, not when the lint is buffered. Generating the message from
BuiltinLintDiag ensures that all required data to construct the message
is preserved in the LintBuffer, eventually allowing the messages to be
moved to fluent.
Remove the `msg` field from BufferedEarlyLint, it is either generated
from the data in the BuiltinLintDiag or stored inside
BuiltinLintDiag::Normal.
Relax restrictions on multiple sanitizers
Most combinations of LLVM sanitizers are legal-enough to enable simultaneously. This change will allow simultaneously enabling ASAN and shadow call stacks on supported platforms.
I used this python script to generate the mutually-exclusive sanitizer combinations:
```python
#!/usr/bin/python3
import subprocess
flags = [
["-fsanitize=address"],
["-fsanitize=leak"],
["-fsanitize=memory"],
["-fsanitize=thread"],
["-fsanitize=hwaddress"],
["-fsanitize=cfi", "-flto", "-fvisibility=hidden"],
["-fsanitize=memtag", "--target=aarch64-linux-android", "-march=armv8a+memtag"],
["-fsanitize=shadow-call-stack"],
["-fsanitize=kcfi", "-flto", "-fvisibility=hidden"],
["-fsanitize=kernel-address"],
["-fsanitize=safe-stack"],
["-fsanitize=dataflow"],
]
for i in range(len(flags)):
for j in range(i):
command = ["clang++"] + flags[i] + flags[j] + ["-o", "main.o", "-c", "main.cpp"]
completed = subprocess.run(command, stderr=subprocess.DEVNULL)
if completed.returncode != 0:
first = flags[i][0][11:].replace('-', '').upper()
second = flags[j][0][11:].replace('-', '').upper()
print(f"(SanitizerSet::{first}, SanitizerSet::{second}),")
```
Most combinations of LLVM sanitizers are legal-enough to enable
simultaneously. This change will allow simultaneously enabling ASAN and
shadow call stacks on supported platforms.
Change `SIGPIPE` ui from `#[unix_sigpipe = "..."]` to `-Zon-broken-pipe=...`
In the stabilization [attempt](https://github.com/rust-lang/rust/pull/120832) of `#[unix_sigpipe = "sig_dfl"]`, a concern was [raised ](https://github.com/rust-lang/rust/pull/120832#issuecomment-2007394609) related to using a language attribute for the feature: Long term, we want `fn lang_start()` to be definable by any crate, not just libstd. Having a special language attribute in that case becomes awkward.
So as a first step towards the next stabilization attempt, this PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag `-Zon-broken-pipe=...` to remove that concern, since now the language is not "contaminated" by this feature.
Another point was [also raised](https://github.com/rust-lang/rust/pull/120832#issuecomment-1987023484), namely that the ui should not leak **how** it does things, but rather what the **end effect** is. The new flag uses the proposed naming. This is of course something that can be iterated on further before stabilization.
Tracking issue: https://github.com/rust-lang/rust/issues/97889
In the stabilization attempt of `#[unix_sigpipe = "sig_dfl"]`, a concern
was raised related to using a language attribute for the feature: Long
term, we want `fn lang_start()` to be definable by any crate, not just
libstd. Having a special language attribute in that case becomes
awkward.
So as a first step towards towards the next stabilization attempt, this
PR changes the `#[unix_sigpipe = "..."]` attribute to a compiler flag
`-Zon-broken-pipe=...` to remove that concern, since now the language
is not "contaminated" by this feature.
Another point was also raised, namely that the ui should not leak
**how** it does things, but rather what the **end effect** is. The new
flag uses the proposed naming. This is of course something that can be
iterated on further before stabilization.
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.
bjorn3 says:
> 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.
Improved collapse_debuginfo attribute, added command-line flag
Improved attribute collapse_debuginfo with variants: `#[collapse_debuginfo=(no|external|yes)]`.
Added command-line flag for default behaviour.
Work-in-progress: will add more tests.
cc https://github.com/rust-lang/rust/issues/100758
Rollup of 8 pull requests
Successful merges:
- #119172 (Detect `NulInCStr` error earlier.)
- #119833 (Make tcx optional from StableMIR run macro and extend it to accept closures)
- #119967 (Add `PatKind::Err` to AST/HIR)
- #119978 (Move async closure parameters into the resultant closure's future eagerly)
- #120021 (don't store const var origins for known vars)
- #120038 (Don't create a separate "basename" when naming and opening a MIR dump file)
- #120057 (Don't ICE when deducing future output if other errors already occurred)
- #120073 (Remove spastorino from users_on_vacation)
r? `@ghost`
`@rustbot` modify labels: rollup
Detect `NulInCStr` error earlier.
By making it an `EscapeError` instead of a `LitError`. This makes it like the other errors produced when checking string literals contents, e.g. for invalid escape sequences or bare CR chars.
NOTE: this means these errors are issued earlier, before expansion, which changes behaviour. It will be possible to move the check back to the later point if desired. If that happens, it's likely that all the string literal contents checks will be delayed together.
One nice thing about this: the old approach had some code in `report_lit_error` to calculate the span of the nul char from a range. This code used a hardwired `+2` to account for the `c"` at the start of a C string literal, but this should have changed to a `+3` for raw C string literals to account for the `cr"`, which meant that the caret in `cr"` nul error messages was one short of where it should have been. The new approach doesn't need any of this and avoids the off-by-one error.
r? ```@fee1-dead```
Rework how diagnostic lints are stored.
`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.
r? `@oli-obk`
`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.
Move platform modules into `sys::pal`
This is the initial step of #117276. `sys` just re-exports everything from the current `sys` for now, I'll move the implementations for the individual features one-by-one after this PR merges.
By making it an `EscapeError` instead of a `LitError`. This makes it
like the other errors produced when checking string literals contents,
e.g. for invalid escape sequences or bare CR chars.
NOTE: this means these errors are issued earlier, before expansion,
which changes behaviour. It will be possible to move the check back to
the later point if desired. If that happens, it's likely that all the
string literal contents checks will be delayed together.
One nice thing about this: the old approach had some code in
`report_lit_error` to calculate the span of the nul char from a range.
This code used a hardwired `+2` to account for the `c"` at the start of
a C string literal, but this should have changed to a `+3` for raw C
string literals to account for the `cr"`, which meant that the caret in
`cr"` nul error messages was one short of where it should have been. The
new approach doesn't need any of this and avoids the off-by-one error.
`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`.
In #119606 I added them and used a `_mv` suffix, but that wasn't great.
A `with_` prefix has three different existing uses.
- Constructors, e.g. `Vec::with_capacity`.
- Wrappers that provide an environment to execute some code, e.g.
`with_session_globals`.
- Consuming chaining methods, e.g. `Span::with_{lo,hi,ctxt}`.
The third case is exactly what we want, so this commit changes
`DiagnosticBuilder::foo_mv` to `DiagnosticBuilder::with_foo`.
Thanks to @compiler-errors for the suggestion.
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`.
- `struct_foo` + `emit` -> `foo`
- `create_foo` + `emit` -> `emit_foo`
I have made recent commits in other PRs that have removed some of these
shortcuts for combinations with few uses, e.g.
`struct_span_err_with_code`. But for the remaining combinations that
have high levels of use, we might as well use them wherever possible.
Remove `-Zdont-buffer-diagnostics`.
It was added in #54232. It seems like it was aimed at NLL development, which is well in the past. Also, it looks like `-Ztreat-err-as-bug` can be used to achieve the same effect. So it doesn't seem necessary.
r? ``@pnkfelix``
Add -Zuse-sync-unwind
Currently Rust uses async unwind by default, but async unwind will bring non-negligible size overhead. it would be nice to allow users to choose this.
In addition, async unwind currently prevents LLVM from generate compact unwind for MachO, if one wishes to generate compact unwind for MachO, then also needs this flag.
It was added in #54232. It seems like it was aimed at NLL development,
which is well in the past. Also, it looks like `-Ztreat-err-as-bug` can
be used to achieve the same effect. So it doesn't seem necessary.
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`.
Remove `-Zreport-delayed-bugs`.
It's not used within the repository in any way (e.g. in tests), and doesn't seem useful.
It was added in #52568.
r? ````@oli-obk````
Remove `-Zdump-mir-spanview`
The `-Zdump-mir-spanview` flag was added back in #76074, as a development/debugging aid for the initial work on what would eventually become `-Cinstrument-coverage`. It causes the compiler to emit an HTML file containing a function's source code, with various spans highlighted based on the contents of MIR.
When the suggestion was made to [triage and remove unnecessary `-Z` flags (Zulip)](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60-Z.60.20option.20triage), I noted that this flag could potentially be worth removing, but I wanted to keep it around to see whether I found it useful for my own coverage work.
But when I actually tried to use it, I ran into various issues (e.g. it crashes on `tests/coverage/closure.rs`). If I can't trust it to work properly without a full overhaul, then instead of diving down a rabbit hole of trying to fix arcane span-handling bugs, it seems better to just remove this obscure old code entirely.
---
````@rustbot```` label +A-code-coverage
Currently for these two errors we go to the effort of switching to a
standard JSON emitter, for no obvious reason, and unlike any other
errors. This behaviour was added for `pretty-json` in #45737, and then
`human-annotate-rs` copied it some time later when it was added.
This commit changes things to just using the requested emitter, which is
simpler and consistent with other errors.
Old output:
```
$ rustc --error-format pretty-json
{"$message_type":"diagnostic","message":"`--error-format=pretty-json` is unstable","code":null,"level":"error","spans":[],"children":[],"rendered":"error: `--error-format=pretty-json` is unstable\n\n"}
$ rustc --error-format human-annotate-rs
{"$message_type":"diagnostic","message":"`--error-format=human-annotate-rs` is unstable","code":null,"level":"error","spans":[],"children":[],"rendered":"error: `--error-format=human-annotate-rs` is unstable\n\n"}
```
New output:
```
$ rustc --error-format pretty-json
{
"$message_type": "diagnostic",
"message": "`--error-format=pretty-json` is unstable",
"code": null,
"level": "error",
"spans": [],
"children": [],
"rendered": "error: `--error-format=pretty-json` is unstable\n\n"
}
$ rustc --error-format human-annotate-rs
error: `--error-format=human-annotate-rs` is unstable
```
`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.
This involves lots of breaking changes. There are two big changes that
force changes. The first is that the bitflag types now don't
automatically implement normal derive traits, so we need to derive them
manually.
Additionally, bitflags now have a hidden inner type by default, which
breaks our custom derives. The bitflags docs recommend using the impl
form in these cases, which I did.
rework `-Zverbose`
implements the changes described in https://github.com/rust-lang/compiler-team/issues/706
the first commit is only a name change from `-Zverbose` to `-Zverbose-internals` and does not change behavior. the second commit changes diagnostics.
possible follow up work:
- `ty::pretty` could print more info with `--verbose` than it does currently. `-Z verbose-internals` shows too much info in a way that's not helpful to users. michael had ideas about this i didn't fully understand: https://rust-lang.zulipchat.com/#narrow/stream/233931-t-compiler.2Fmajor-changes/topic/uplift.20some.20-Zverbose.20calls.20and.20rename.20to.E2.80.A6.20compiler-team.23706/near/408984200
- `--verbose` should imply `-Z write-long-types-to-disk=no`. the code in `ty_string_with_limit` should take `--verbose` into account (apparently this affects `Ty::sort_string`, i'm not familiar with this code). writing a file to disk should suggest passing `--verbose`.
r? `@compiler-errors` cc `@estebank`
Separate MIR lints from validation
Add a MIR lint pass, enabled with -Zlint-mir, which identifies undefined or
likely erroneous behaviour.
The initial implementation mostly migrates existing checks of this nature from
MIR validator, where they did not belong (those checks have false positives and
there is nothing inherently invalid about MIR with undefined behaviour).
Fixes#104736Fixes#104843Fixes#116079Fixes#116736Fixes#118990
`IntoDiagnostic` defaults to `ErrorGuaranteed`, because errors are the
most common diagnostic level. It makes sense to do likewise for the
closely-related (and much more widely used) `DiagnosticBuilder` type,
letting us write `DiagnosticBuilder<'a, ErrorGuaranteed>` as just
`DiagnosticBuilder<'a>`. This cuts over 200 lines of code due to many
multi-line things becoming single line things.
We can just get the error level in the `match` and then use
`DiagnosticBuilder::new`. This then means a number of `DiagCtxt`
functions are no longer needed, because this was the one place that used
them.
Note: the commit changes the treatment of spans for `Expect`, which was
different to all the other cases, but this has no apparent effect.
This lets different error levels share the same return type from
`emit_*`.
- A lot of inconsistencies in the `DiagCtxt` API are removed.
- `Noted` is removed.
- `FatalAbort` is introduced for fatal errors (abort via `raise`),
replacing the `EmissionGuarantee` impl for `!`.
- `Bug` is renamed `BugAbort` (to avoid clashing with `Level::Bug` and
to mirror `FatalAbort`), and modified to work in the new way with bug
errors (abort via panic).
- Various diagnostic creators and emitters updated to the new, better
signatures. Note that `DiagCtxt::bug` no longer needs to call
`panic_any`, because `emit` handles that.
Also shorten the obnoxiously long
`diagnostic_builder_emit_producing_guarantee` name.
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.
