Remember mutability in `DefKind::Static`.
This allows to compute the `BodyOwnerKind` from `DefKind` only, and
removes a direct dependency of some MIR queries onto HIR.
As a side effect, it also simplifies metadata, since we don't need 4
flavours of `EntryKind::*Static` any more.
allow arbitrary inherent impls for builtin types in core
Part of https://github.com/rust-lang/compiler-team/issues/487. Slightly adjusted after some talks with `@m-ou-se` about the requirements of `t-libs-api`.
This adds a crate attribute `#![rustc_coherence_is_core]` which allows arbitrary impls for builtin types in core.
For other library crates impls for builtin types should be avoided if possible. We do have to allow the existing stable impls however. To prevent us from accidentally adding more of these in the future, there is a second attribute `#[rustc_allow_incoherent_impl]` which has to be added to **all impl items**. This only supports impls for builtin types but can easily be extended to additional types in a future PR.
This implementation does not check for overlaps in these impls. Perfectly checking that requires us to check the coherence of these incoherent impls in every crate, as two distinct dependencies may add overlapping methods. It should be easy enough to detect if it goes wrong and the attribute is only intended for use inside of std.
The first two commits are mostly unrelated cleanups.
Rollup of 5 pull requests
Successful merges:
- #95294 (Document Linux kernel handoff in std::io::copy and std::fs::copy)
- #95443 (Clarify how `src/tools/x` searches for python)
- #95452 (fix since field version for termination stabilization)
- #95460 (Spellchecking compiler code)
- #95461 (Spellchecking some comments)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
This allows to compute the `BodyOwnerKind` from `DefKind` only, and
removes a direct dependency of some MIR queries onto HIR.
As a side effect, it also simplifies metadata, since we don't need 4
flavours of `EntryKind::*Static` any more.
Remove `Session::one_time_diagnostic`
This is untracked mutable state, which modified the behaviour of queries.
It was used for 2 things: some full-blown errors, but mostly for lint declaration notes ("the lint level is defined here" notes).
It is replaced by the diagnostic deduplication infra which already exists in the diagnostic emitter.
A new diagnostic level `OnceNote` is introduced specifically for lint notes, to deduplicate subdiagnostics.
As a drive-by, diagnostic emission takes a `&mut` to allow dropping the `SubDiagnostic`s.
Swap DtorckConstraint to DropckConstraint
This change was made as per suspicion that this struct was never renamed after consistent use of DropCk.
This also clarifies the meaning behind the name of this structure.
Fixes https://github.com/rust-lang/rust/issues/94310
Clarify which kinds of MIR are allowed during which phases.
This enhances documentation with these details and extends the validator to check these requirements more thoroughly. Most of these conditions were already being checked.
There was also some disagreement between the `MirPhase` docs and validator as to what it meant for the `body.phase` field to have a certain value. This PR resolves those disagreements in favor of the `MirPhase` docs (which is what the pass manager implemented), adjusting the validator accordingly. The result is now that the `DropLowering` phase begins with the end of the elaborate drops pass, and lasts until the beginning of the generator lowring pass. This doesn't feel entirely natural to me, but as long as it's documented accurately it should be ok.
r? rust-lang/mir-opt
This change was made as per suspicion that this struct was never renamed after consistent use of DropCk.
This also clarifies the meaning behind the name of this structure.
Change Thir to lazily create constants
To allow `AbstractConst`s to work with the previous thir changes we made and those we want to make, i.e. to avoid problems due to `ValTree` and `ConstValue` conversions, we instead switch to a thir representation for constants that allows us to lazily create constants.
r? `@oli-obk`
Properly track `ImplObligations`
Instead of probing for all possible `impl`s that could have caused an
`ImplObligation`, keep track of its `DefId` and obligation spans for
accurate error reporting.
Follow to #89580. Addresses #89418.
Instead of probing for all possible impls that could have caused an
`ImplObligation`, keep track of its `DefId` and obligation spans for
accurate error reporting.
Follow up to #89580. Addresses #89418.
Remove some unnecessary clones.
Tweak output for auto trait impl obligations.
This enhances documentation with these details and extends the validator to check these requirements
more thoroughly. As a part of this, we add a new `Deaggregated` phase, and rename other phases so
that their names more naturally correspond to what they represent.
There are a few places were we have to construct it, though, and a few
places that are more invasive to change. To do this, we create a
constructor with a long obvious name.
fix typos
Rework of #94603 which got closed as I was trying to unmerge and repush. This is a subset of changes from the original pr as I sed'd whatever typos I remembered from the original PR
thanks to `@cuishuang` for the original PR
Improve `expect` impl and handle `#[expect(unfulfilled_lint_expectations)]` (RFC 2383)
This PR updates unstable `ExpectationIds` in stashed diagnostics and adds some asserts to ensure that the stored expectations are really empty in the end. Additionally, it handles the `#[expect(unfulfilled_lint_expectations)]` case.
According to the [Errors and lints docs](https://rustc-dev-guide.rust-lang.org/diagnostics.html#diagnostic-levels) the `error` level should only be used _"when the compiler detects a problem that makes it unable to compile the program"_. As this isn't the case with `#[expect(unfulfilled_lint_expectations)]` I decided to only create a warning. To avoid adding a new lint only for this case, I simply emit a `unfulfilled_lint_expectations` diagnostic with an additional note.
---
r? `@wesleywiser` I'm requesting a review from you since you reviewed the previous PR https://github.com/rust-lang/rust/pull/87835. You are welcome to reassign it if you're busy 🙃
rfc: [RFC-2383](https://rust-lang.github.io/rfcs/2383-lint-reasons.html)
tracking issue: https://github.com/rust-lang/rust/issues/85549
cc: `@flip1995` In case you're also interested in this :)
Type params and assoc types have unit metadata if they are sized
Extend the logic in `Pointee` projection to ensure that we can satisfy `<T as Pointee>::Metadata = ()` if `T: Sized`.
cc: `@SimonSapin` and #93959
Return early to fix ICE
This fixes#94627, ICE happens because compiler tries to suggest constraining type parameter but the only constraint is implicit `std::Sized` one, so it gets removed and there is nothing to suggest resulting in ICE.
Improve `AdtDef` interning.
This commit makes `AdtDef` use `Interned`. Much of the commit is tedious
changes to introduce getter functions. The interesting changes are in
`compiler/rustc_middle/src/ty/adt.rs`.
r? `@fee1-dead`
CTFE/Miri: detect out-of-bounds pointers in offset_from
Also I became uneasy with aggressively doing `try_to_int` here -- this will always succeed on Miri, leading to the wrong codepath being taken. We should rather try to convert them both to pointers, and use the integer path as a fallback, so that's what I implemented now.
Hiding whitespaces helps with the diff.
Fixes https://github.com/rust-lang/miri/issues/1950
r? ``@oli-obk``
Change several HashMaps to IndexMap to improve incremental hashing performance
Stable hashing hash maps in incremental mode takes a lot of time, especially for some benchmarks like `clap`. As noted by `@Mark-Simulacrum` [here](https://github.com/rust-lang/rust/pull/89404#issuecomment-950043892), this cost could be reduced by replacing some hash maps by indexmaps.
I gathered some statistics and found several hash maps that took a lot of time to hash and replaced them by indexmaps. However, in order for this to work, we need to make sure that these indexmaps have deterministic insertion order. These three are used only in visitors as far as I can see, which seems deterministic. Can we enforce this somehow? Or should some explaining comment be included for these maps?
This commit makes `AdtDef` use `Interned`. Much the commit is tedious
changes to introduce getter functions. The interesting changes are in
`compiler/rustc_middle/src/ty/adt.rs`.
mir-opt: Replace clone on primitives with copy
We can't do it for everything, but it would be nice to at least stop making calls to clone methods in debug from things like derived-clones.
r? `@ghost`
This updates the standard library's documentation to use the new syntax. The
documentation is worthwhile to update as it should be more idiomatic
(particularly for features like this, which are nice for users to get acquainted
with). The general codebase is likely more hassle than benefit to update: it'll
hurt git blame, and generally updates can be done by folks updating the code if
(and when) that makes things more readable with the new format.
A few places in the compiler and library code are updated (mostly just due to
already having been done when this commit was first authored).
diagnostics: use rustc_on_unimplemented to recommend `[].iter()`
To make this work, the `#[rustc_on_unimplemented]` data needs to be used to
report method resolution errors, which is most of what this commit does.
Fixes#94581
This change is somewhat extensive, since it affects MIR -- since this is called to determine Copy vs Move -- so any test that's `no_core` needs to actually have the normal `impl`s it uses.
Treat constant values as mir::ConstantKind::Val
Another step that is necessary for the introduction of Valtrees: we don't want to treat `ty::Const` instances of kind `ty::ConstKind::Value` as `mir::ConstantKind::Ty` anymore.
r? `@oli-obk`
add `#[rustc_pass_by_value]` to more types
the only interesting changes here should be to `TransitiveRelation`, but I believe to be highly unlikely that we will ever use a non `Copy` type with this type.
To make this work, the `#[rustc_on_unimplemented]` data needs to be used to
report method resolution errors, which is most of what this commit does.
Fixes#94581
Use impl substs in `#[rustc_on_unimplemented]`
We were using the trait-ref substs instead of impl substs in `rustc_on_unimplemented`, even when computing the `rustc_on_unimplemented` attached to an impl block. Let's not do that.
This PR also untangles impl and trait def-ids in the logic in `on_unimplemented` a bit.
Fixes#94675
Clarify `Layout` interning.
`Layout` is another type that is sometimes interned, sometimes not, and
we always use references to refer to it so we can't take any advantage
of the uniqueness properties for hashing or equality checks.
This commit renames `Layout` as `LayoutS`, and then introduces a new
`Layout` that is a newtype around an `Interned<LayoutS>`. It also
interns more layouts than before. Previously layouts within layouts
(via the `variants` field) were never interned, but now they are. Hence
the lifetime on the new `Layout` type.
Unlike other interned types, these ones are in `rustc_target` instead of
`rustc_middle`. This reflects the existing structure of the code, which
does layout-specific stuff in `rustc_target` while `TyAndLayout` is
generic over the `Ty`, allowing the type-specific stuff to occur in
`rustc_middle`.
The commit also adds a `HashStable` impl for `Interned`, which was
needed. It hashes the contents, unlike the `Hash` impl which hashes the
pointer.
r? `@fee1-dead`
`Layout` is another type that is sometimes interned, sometimes not, and
we always use references to refer to it so we can't take any advantage
of the uniqueness properties for hashing or equality checks.
This commit renames `Layout` as `LayoutS`, and then introduces a new
`Layout` that is a newtype around an `Interned<LayoutS>`. It also
interns more layouts than before. Previously layouts within layouts
(via the `variants` field) were never interned, but now they are. Hence
the lifetime on the new `Layout` type.
Unlike other interned types, these ones are in `rustc_target` instead of
`rustc_middle`. This reflects the existing structure of the code, which
does layout-specific stuff in `rustc_target` while `TyAndLayout` is
generic over the `Ty`, allowing the type-specific stuff to occur in
`rustc_middle`.
The commit also adds a `HashStable` impl for `Interned`, which was
needed. It hashes the contents, unlike the `Hash` impl which hashes the
pointer.
Currently some `Allocation`s are interned, some are not, and it's very
hard to tell at a use point which is which.
This commit introduces `ConstAllocation` for the known-interned ones,
which makes the division much clearer. `ConstAllocation::inner()` is
used to get the underlying `Allocation`.
In some places it's natural to use an `Allocation`, in some it's natural
to use a `ConstAllocation`, and in some places there's no clear choice.
I've tried to make things look as nice as possible, while generally
favouring `ConstAllocation`, which is the type that embodies more
information. This does require quite a few calls to `inner()`.
The commit also tweaks how `PartialOrd` works for `Interned`. The
previous code was too clever by half, building on `T: Ord` to make the
code shorter. That caused problems with deriving `PartialOrd` and `Ord`
for `ConstAllocation`, so I changed it to build on `T: PartialOrd`,
which is slightly more verbose but much more standard and avoided the
problems.
Ensure stability directives are checked in all cases
Split off #93017
Stability and deprecation were not checked in all cases, for instance if a type error happened.
This PR moves the check earlier in the pipeline to ensure the errors are emitted in all cases.
r? `@lcnr`
improve comments for `simplify_type`
Should now correctly describe what's going on. Experimented with checking the invariant for projections
but that ended up requiring fairly involved changes. I assume that it is not possible to get unsoundness here,
at least for now and I can pretty much guarantee that it's impossible to trigger it by accident.
r? `````@nikomatsakis````` cc #92721
Implementation of the `expect` attribute (RFC 2383)
This is an implementation of the `expect` attribute as described in [RFC-2383](https://rust-lang.github.io/rfcs/2383-lint-reasons.html). The attribute allows the suppression of lint message by expecting them. Unfulfilled lint expectations (meaning no expected lint was caught) will emit the `unfulfilled_lint_expectations` lint at the `expect` attribute.
### Example
#### input
```rs
// required feature flag
#![feature(lint_reasons)]
#[expect(unused_mut)] // Will warn about an unfulfilled expectation
#[expect(unused_variables)] // Will be fulfilled by x
fn main() {
let x = 0;
}
```
#### output
```txt
warning: this lint expectation is unfulfilled
--> $DIR/trigger_lint.rs:3:1
|
LL | #[expect(unused_mut)] // Will warn about an unfulfilled expectation
| ^^^^^^^^^^
|
= note: `#[warn(unfulfilled_lint_expectations)]` on by default
```
### Implementation
This implementation introduces `Expect` as a new lint level for diagnostics, which have been expected. All lint expectations marked via the `expect` attribute are collected in the [`LintLevelsBuilder`] and assigned an ID that is stored in the new lint level. The `LintLevelsBuilder` stores all found expectations and the data needed to emit the `unfulfilled_lint_expectations` in the [`LintLevelsMap`] which is the result of the [`lint_levels()`] query.
The [`rustc_errors::HandlerInner`] is the central error handler in rustc and handles the emission of all diagnostics. Lint message with the level `Expect` are suppressed during this emission, while the expectation ID is stored in a set which marks them as fulfilled. The last step is then so simply check if all expectations collected by the [`LintLevelsBuilder`] in the [`LintLevelsMap`] have been marked as fulfilled in the [`rustc_errors::HandlerInner`]. Otherwise, a new lint message will be emitted.
The implementation of the `LintExpectationId` required some special handling to make it stable between sessions. Lints can be emitted during [`EarlyLintPass`]es. At this stage, it's not possible to create a stable identifier. The level instead stores an unstable identifier, which is later converted to a stable `LintExpectationId`.
### Followup TO-DOs
All open TO-DOs have been marked with `FIXME` comments in the code. This is the combined list of them:
* [ ] The current implementation doesn't cover cases where the `unfulfilled_lint_expectations` lint is actually expected by another `expect` attribute.
* This should be easily possible, but I wanted to get some feedback before putting more work into this.
* This could also be done in a new PR to not add to much more code to this one
* [ ] Update unstable documentation to reflect this change.
* [ ] Update unstable expectation ids in [`HandlerInner::stashed_diagnostics`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/struct.HandlerInner.html#structfield.stashed_diagnostics)
### Open questions
I also have a few open questions where I would like to get feedback on:
1. The RFC discussion included a suggestion to change the `expect` attribute to something else. (Initiated by `@Ixrec` [here](https://github.com/rust-lang/rfcs/pull/2383#issuecomment-378424091), suggestion from `@scottmcm` to use `#[should_lint(...)]` [here](https://github.com/rust-lang/rfcs/pull/2383#issuecomment-378648877)). No real conclusion was drawn on that point from my understanding. Is this still open for discussion, or was this discarded with the merge of the RFC?
2. How should the expect attribute deal with the new `force-warn` lint level?
---
This approach was inspired by a discussion with `@LeSeulArtichaut.`
RFC tracking issue: #54503
Mentoring/Implementation issue: #85549
[`LintLevelsBuilder`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/levels/struct.LintLevelsBuilder.html
[`LintLevelsMap`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/lint/struct.LintLevelMap.html
[`lint_levels()`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html#method.lint_levels
[`rustc_errors::HandlerInner`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_errors/struct.HandlerInner.html
[`EarlyLintPass`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/trait.EarlyLintPass.html
Miri/CTFE: properly treat overflow in (signed) division/rem as UB
To my surprise, it looks like LLVM treats overflow of signed div/rem as UB. From what I can tell, MIR `Div`/`Rem` directly lowers to the corresponding LLVM operation, so to make that correct we also have to consider these overflows UB in the CTFE/Miri interpreter engine.
r? `@oli-obk`
Restore the local filter on mono item sorting
In `CodegenUnit::items_in_deterministic_order`, there's a comment that
only local HirIds should be taken into account, but #90408 removed the
`as_local` call that sets others to None. Restoring that check fixes the
s390x hangs seen in [RHBZ 2058803].
[RHBZ 2058803]: https://bugzilla.redhat.com/show_bug.cgi?id=2058803
Adt copy suggestions
Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type
- Can be copy
- All predicates that need to be satisfied for that are based on type params
i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`.
An example:
```rust
fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) {
(t, t)
}
```
New error (current compiler doesn't provide `help`:):
```text
error[E0382]: use of moved value: `t`
--> t.rs:2:9
|
1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) {
| - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait
2 | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider restricting type parameter `T`
|
1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) {
| ++++++
```
Fixes#93623
r? ``````````@estebank``````````
``````````@rustbot`````````` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement
----
I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')
Direct users towards using Rust target feature names in CLI
This PR consists of a couple of changes on how we handle target features.
In particular there is a bug-fix wherein we avoid passing through features that aren't prefixed by `+` or `-` to LLVM. These appear to be causing LLVM to assert, which is pretty poor a behaviour (and also makes it pretty clear we expect feature names to be prefixed).
The other commit, I anticipate to be somewhat more controversial is outputting a warning when users specify a LLVM-specific, or otherwise unknown, feature name on the CLI. In those situations we request users to either replace it with a known Rust feature name (e.g. `bmi` -> `bmi1`) or file a feature request. I've a couple motivations for this: first of all, if users are specifying these features on the command line, I'm pretty confident there is also a need for these features to be usable via `#[cfg(target_feature)]` machinery. And second, we're growing a fair number of backends recently and having ability to provide some sort of unified-ish interface in this place seems pretty useful to me.
Sponsored by: standard.ai
In `CodegenUnit::items_in_deterministic_order`, there's a comment that
only local HirIds should be taken into account, but #90408 removed the
`as_local` call that sets others to None. Restoring that check fixes the
s390x hangs seen in [RHBZ 2058803].
[RHBZ 2058803]: https://bugzilla.redhat.com/show_bug.cgi?id=2058803
Add `rustc_middle::ty::suggest_constraining_type_params` that suggests
adding multiple constraints.
`suggest_constraining_type_param` now just forwards params to this new
function.
Caching the stable hash of Ty within itself
Instead of computing stable hashes on types as needed, we compute it during interning.
This way we can, when a hash is requested, just hash that hash, which is significantly faster than traversing the type itself.
We only do this for incremental for now, as incremental is the only frequent user of stable hashing.
As a next step we can try out
* moving the hash and TypeFlags to Interner, so projections and regions get the same benefit (tho regions are not nested, so maybe that's not a good idea? Would be nice for dedup tho)
* start comparing types via their stable hash instead of their address?
Apply noundef attribute to all scalar types which do not permit raw init
Beyond `&`/`&mut`/`Box`, this covers `char`, enum discriminants, `NonZero*`, etc.
All such types currently cause a Miri error if left uninitialized,
and an `invalid_value` lint in cases like `mem::uninitialized::<char>()`.
Note that this _does not_ change whether or not it is UB for `u64` (or
other integer types with no invalid values) to be undef.
Fixes (partially) #74378.
r? `@ghost` (blocked on #94127)
`@rustbot` label S-blocked
Avoid query cache sharding code in single-threaded mode
In non-parallel compilers, this is just adding needless overhead at compilation time (since there is only one shard statically anyway). This amounts to roughly ~10 seconds reduction in bootstrap time, with overall neutral (some wins, some losses) performance results.
Parallel compiler performance should be largely unaffected by this PR; sharding is kept there.
Beyond `&`/`&mut`/`Box`, this covers `char`, discriminants, `NonZero*`, etc.
All such types currently cause a Miri error if left uninitialized,
and an `invalid_value` lint in cases like `mem::uninitialized::<char>()`
Note that this _does not_ change whether or not it is UB for `u64` (or
other integer types with no invalid values) to be undef.
fix a message
implement a rustfix-applicable suggestion
implement `suggest_floating_point_literal`
add `ObligationCauseCode::BinOp`
remove duplicate code
fix function names in uitests
use `Diagnostic` instead of `DiagnosticBuilder`
Print `ParamTy` and `ParamConst` instead of displaying them
Display for `ParamTy` and `ParamConst` is implemented in terms of print.
Using print avoids creating a new `FmtPrinter` just to display the
parameter name.
r? `@Mark-Simulacrum`
Remove in band lifetimes
As discussed in t-lang backlog bonanza, the `in_band_lifetimes` FCP closed in favor for the feature not being stabilized. This PR removes `#![feature(in_band_lifetimes)]` in its entirety.
Let me know if this PR is too hasty, and if we should instead do something intermediate for deprecate the feature first.
r? `@scottmcm` (or feel free to reassign, just saw your last comment on #44524)
Closes#44524
Use undef for (some) partially-uninit constants
There needs to be some limit to avoid perf regressions on large arrays
with undef in each element (see comment in the code).
Fixes: #84565
Original PR: #83698
Depends on LLVM 14: #93577
Convert `newtype_index` to a proc macro
The `macro_rules!` implementation was becomng excessively complicated,
and difficult to modify. The new proc macro implementation should make
it much easier to add new features (e.g. skipping certain `#[derive]`s)
rustc_errors: let `DiagnosticBuilder::emit` return a "guarantee of emission".
That is, `DiagnosticBuilder` is now generic over the return type of `.emit()`, so we'll now have:
* `DiagnosticBuilder<ErrorReported>` for error (incl. fatal/bug) diagnostics
* can only be created via a `const L: Level`-generic constructor, that limits allowed variants via a `where` clause, so not even `rustc_errors` can accidentally bypass this limitation
* asserts `diagnostic.is_error()` on emission, just in case the construction restriction was bypassed (e.g. by replacing the whole `Diagnostic` inside `DiagnosticBuilder`)
* `.emit()` returns `ErrorReported`, as a "proof" token that `.emit()` was called
(though note that this isn't a real guarantee until after completing the work on
#69426)
* `DiagnosticBuilder<()>` for everything else (warnings, notes, etc.)
* can also be obtained from other `DiagnosticBuilder`s by calling `.forget_guarantee()`
This PR is a companion to other ongoing work, namely:
* #69426
and it's ongoing implementation:
#93222
the API changes in this PR are needed to get statically-checked "only errors produce `ErrorReported` from `.emit()`", but doesn't itself provide any really strong guarantees without those other `ErrorReported` changes
* #93244
would make the choices of API changes (esp. naming) in this PR fit better overall
In order to be able to let `.emit()` return anything trustable, several changes had to be made:
* `Diagnostic`'s `level` field is now private to `rustc_errors`, to disallow arbitrary "downgrade"s from "some kind of error" to "warning" (or anything else that doesn't cause compilation to fail)
* it's still possible to replace the whole `Diagnostic` inside the `DiagnosticBuilder`, sadly, that's harder to fix, but it's unlikely enough that we can paper over it with asserts on `.emit()`
* `.cancel()` now consumes `DiagnosticBuilder`, preventing `.emit()` calls on a cancelled diagnostic
* it's also now done internally, through `DiagnosticBuilder`-private state, instead of having a `Level::Cancelled` variant that can be read (or worse, written) by the user
* this removes a hazard of calling `.cancel()` on an error then continuing to attach details to it, and even expect to be able to `.emit()` it
* warnings were switched to *only* `can_emit_warnings` on emission (instead of pre-cancelling early)
* `struct_dummy` was removed (as it relied on a pre-`Cancelled` `Diagnostic`)
* since `.emit()` doesn't consume the `DiagnosticBuilder` <sub>(I tried and gave up, it's much more work than this PR)</sub>,
we have to make `.emit()` idempotent wrt the guarantees it returns
* thankfully, `err.emit(); err.emit();` can return `ErrorReported` both times, as the second `.emit()` call has no side-effects *only* because the first one did do the appropriate emission
* `&mut Diagnostic` is now used in a lot of function signatures, which used to take `&mut DiagnosticBuilder` (in the interest of not having to make those functions generic)
* the APIs were already mostly identical, allowing for low-effort porting to this new setup
* only some of the suggestion methods needed some rework, to have the extra `DiagnosticBuilder` functionality on the `Diagnostic` methods themselves (that change is also present in #93259)
* `.emit()`/`.cancel()` aren't available, but IMO calling them from an "error decorator/annotator" function isn't a good practice, and can lead to strange behavior (from the caller's perspective)
* `.downgrade_to_delayed_bug()` was added, letting you convert any `.is_error()` diagnostic into a `delay_span_bug` one (which works because in both cases the guarantees available are the same)
This PR should ideally be reviewed commit-by-commit, since there is a lot of fallout in each.
r? `@estebank` cc `@Manishearth` `@nikomatsakis` `@mark-i-m`
These links never worked, but the lint was suppressed due to the fact
that the span was pointing into the macro. With the new macro
implementation, the span now points directly to the doc comment in the
macro invocation, so it's no longer suppressed.
Always format to internal String in FmtPrinter
This avoids monomorphizing for different parameters, decreasing generic code
instantiated downstream from rustc_middle -- locally seeing 7% unoptimized LLVM IR
line wins on rustc_borrowck, for example.
We likely can't/shouldn't get rid of the Result-ness on most functions, though some
further cleanup avoiding fmt::Error where we now know it won't occur may be possible,
though somewhat painful -- fmt::Write is a pretty annoying API to work with in practice
when you're trying to use it infallibly.
Display for `ParamTy` and `ParamConst` is implemented in terms of print.
Using print avoids creating a new `FmtPrinter` just to display the
parameter name.
Rollup of 7 pull requests
Successful merges:
- #94169 (Fix several asm! related issues)
- #94178 (tidy: fire less "ignoring file length unneccessarily" warnings)
- #94179 (solarish current_exe using libc call directly)
- #94196 (compiletest: Print process output info with less whitespace)
- #94208 (Add the let else tests found missing in the stabilization report)
- #94237 (Do not suggest wrapping an item if it has ambiguous un-imported methods)
- #94246 (ScalarMaybeUninit is explicitly hexadecimal in its formatting)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
ScalarMaybeUninit is explicitly hexadecimal in its formatting
This makes `ScalarMaybeUninit` consistent with `Scalar` after the changes in https://github.com/rust-lang/rust/pull/94189.
r? ``@oli-obk``
Fix several asm! related issues
This is a combination of several fixes, each split into a separate commit. Splitting these into PRs is not practical since they conflict with each other.
Fixes#92378Fixes#85247
r? ``@nagisa``
CTFE engine: Scalar: expose size-generic to_(u)int methods
This matches the size-generic constructors `Scalar::from_(u)int`, and it would have helped in https://github.com/rust-lang/miri/pull/1978.
r? `@oli-obk`
safely `transmute<&List<Ty<'tcx>>, &List<GenericArg<'tcx>>>`
This PR has 3 relevant steps which are is split in distinct commits.
The first commit now interns `List<Ty<'tcx>>` and `List<GenericArg<'tcx>>` together, potentially reusing memory while allowing free conversions between these two using `List<Ty<'tcx>>::as_substs()` and `SubstsRef<'tcx>::try_as_type_list()`.
Using this, we then use `&'tcx List<Ty<'tcx>>` instead of a `SubstsRef<'tcx>` for tuple fields, simplifying a bunch of code.
Finally, as tuple fields and other generic arguments now use a different `TypeFoldable<'tcx>` impl, we optimize the impl for `List<Ty<'tcx>>` improving perf by slightly less than 1% in tuple heavy benchmarks.
This reverts commit a240ccd81c, reversing
changes made to 393fdc1048.
This PR was likely responsible for a relatively large regression in
dist-x86_64-msvc-alt builder times, from approximately 1.7 to 2.8 hours,
bringing that builder into the pool of the slowest builders we currently have.
This seems to be limited to the alt builder due to needing parallel-compiler
enabled, likely leading to slow LLVM compilation for some reason.
Improve `unused_unsafe` lint
I’m going to add some motivation and explanation below, particularly pointing the changes in behavior from this PR.
_Edit:_ Looking for existing issues, looks like this PR fixes#88260.
_Edit2:_ Now also contains code that closes#90776.
Main motivation: Fixes some issues with the current behavior. This PR is
more-or-less completely re-implementing the unused_unsafe lint; it’s also only
done in the MIR-version of the lint, the set of tests for the `-Zthir-unsafeck`
version no longer succeeds (and is thus disabled, see `lint-unused-unsafe.rs`).
On current nightly,
```rs
unsafe fn unsf() {}
fn inner_ignored() {
unsafe {
#[allow(unused_unsafe)]
unsafe {
unsf()
}
}
}
```
doesn’t create any warnings. This situation is not unrealistic to come by, the
inner `unsafe` block could e.g. come from a macro. Actually, this PR even
includes removal of one unused `unsafe` in the standard library that was missed
in a similar situation. (The inner `unsafe` coming from an external macro hides
the warning, too.)
The reason behind this problem is how the check currently works:
* While generating MIR, it already skips nested unsafe blocks (i.e. unsafe
nested in other unsafe) so that the inner one is always the one considered
unused
* To differentiate the cases of no unsafe operations inside the `unsafe` vs.
a surrounding `unsafe` block, there’s some ad-hoc magic walking up the HIR to
look for surrounding used `unsafe` blocks.
There’s a lot of problems with this approach besides the one presented above.
E.g. the MIR-building uses checks for `unsafe_op_in_unsafe_fn` lint to decide
early whether or not `unsafe` blocks in an `unsafe fn` are redundant and ought
to be removed.
```rs
unsafe fn granular_disallow_op_in_unsafe_fn() {
unsafe {
#[deny(unsafe_op_in_unsafe_fn)]
{
unsf();
}
}
}
```
```
error: call to unsafe function is unsafe and requires unsafe block (error E0133)
--> src/main.rs:13:13
|
13 | unsf();
| ^^^^^^ call to unsafe function
|
note: the lint level is defined here
--> src/main.rs:11:16
|
11 | #[deny(unsafe_op_in_unsafe_fn)]
| ^^^^^^^^^^^^^^^^^^^^^^
= note: consult the function's documentation for information on how to avoid undefined behavior
warning: unnecessary `unsafe` block
--> src/main.rs:10:5
|
9 | unsafe fn granular_disallow_op_in_unsafe_fn() {
| --------------------------------------------- because it's nested under this `unsafe` fn
10 | unsafe {
| ^^^^^^ unnecessary `unsafe` block
|
= note: `#[warn(unused_unsafe)]` on by default
```
Here, the intermediate `unsafe` was ignored, even though it contains a unsafe
operation that is not allowed to happen in an `unsafe fn` without an additional `unsafe` block.
Also closures were problematic and the workaround/algorithms used on current
nightly didn’t work properly. (I skipped trying to fully understand what it was
supposed to do, because this PR uses a completely different approach.)
```rs
fn nested() {
unsafe {
unsafe { unsf() }
}
}
```
```
warning: unnecessary `unsafe` block
--> src/main.rs:10:9
|
9 | unsafe {
| ------ because it's nested under this `unsafe` block
10 | unsafe { unsf() }
| ^^^^^^ unnecessary `unsafe` block
|
= note: `#[warn(unused_unsafe)]` on by default
```
vs
```rs
fn nested() {
let _ = || unsafe {
let _ = || unsafe { unsf() };
};
}
```
```
warning: unnecessary `unsafe` block
--> src/main.rs:9:16
|
9 | let _ = || unsafe {
| ^^^^^^ unnecessary `unsafe` block
|
= note: `#[warn(unused_unsafe)]` on by default
warning: unnecessary `unsafe` block
--> src/main.rs:10:20
|
10 | let _ = || unsafe { unsf() };
| ^^^^^^ unnecessary `unsafe` block
```
*note that this warning kind-of suggests that **both** unsafe blocks are redundant*
--------------------------------------------------------------------------------
I also dislike the fact that it always suggests keeping the outermost `unsafe`.
E.g. for
```rs
fn granularity() {
unsafe {
unsafe { unsf() }
unsafe { unsf() }
unsafe { unsf() }
}
}
```
I prefer if `rustc` suggests removing the more-course outer-level `unsafe`
instead of the fine-grained inner `unsafe` blocks, which it currently does on nightly:
```
warning: unnecessary `unsafe` block
--> src/main.rs:10:9
|
9 | unsafe {
| ------ because it's nested under this `unsafe` block
10 | unsafe { unsf() }
| ^^^^^^ unnecessary `unsafe` block
|
= note: `#[warn(unused_unsafe)]` on by default
warning: unnecessary `unsafe` block
--> src/main.rs:11:9
|
9 | unsafe {
| ------ because it's nested under this `unsafe` block
10 | unsafe { unsf() }
11 | unsafe { unsf() }
| ^^^^^^ unnecessary `unsafe` block
warning: unnecessary `unsafe` block
--> src/main.rs:12:9
|
9 | unsafe {
| ------ because it's nested under this `unsafe` block
...
12 | unsafe { unsf() }
| ^^^^^^ unnecessary `unsafe` block
```
--------------------------------------------------------------------------------
Needless to say, this PR addresses all these points. For context, as far as my
understanding goes, the main advantage of skipping inner unsafe blocks was that
a test case like
```rs
fn top_level_used() {
unsafe {
unsf();
unsafe { unsf() }
unsafe { unsf() }
unsafe { unsf() }
}
}
```
should generate some warning because there’s redundant nested `unsafe`, however
every single `unsafe` block _does_ contain some statement that uses it. Of course
this PR doesn’t aim change the warnings on this kind of code example, because
the current behavior, warning on all the inner `unsafe` blocks, makes sense in this case.
As mentioned, during MIR building all the unsafe blocks *are* kept now, and usage
is attributed to them. The way to still generate a warning like
```
warning: unnecessary `unsafe` block
--> src/main.rs:11:9
|
9 | unsafe {
| ------ because it's nested under this `unsafe` block
10 | unsf();
11 | unsafe { unsf() }
| ^^^^^^ unnecessary `unsafe` block
|
= note: `#[warn(unused_unsafe)]` on by default
warning: unnecessary `unsafe` block
--> src/main.rs:12:9
|
9 | unsafe {
| ------ because it's nested under this `unsafe` block
...
12 | unsafe { unsf() }
| ^^^^^^ unnecessary `unsafe` block
warning: unnecessary `unsafe` block
--> src/main.rs:13:9
|
9 | unsafe {
| ------ because it's nested under this `unsafe` block
...
13 | unsafe { unsf() }
| ^^^^^^ unnecessary `unsafe` block
```
in this case is by emitting a `unused_unsafe` warning for all of the `unsafe`
blocks that are _within a **used** unsafe block_.
The previous code had a little HIR traversal already anyways to collect a set of
all the unsafe blocks (in order to afterwards determine which ones are unused
afterwards). This PR uses such a traversal to do additional things including logic
like _always_ warn for an `unsafe` block that’s inside of another **used**
unsafe block. The traversal is expanded to include nested closures in the same go,
this simplifies a lot of things.
The whole logic around `unsafe_op_in_unsafe_fn` is a little complicated, there’s
some test cases of corner-cases in this PR. (The implementation involves
differentiating between whether a used unsafe block was used exclusively by
operations where `allow(unsafe_op_in_unsafe_fn)` was active.) The main goal was
to make sure that code should compile successfully if all the `unused_unsafe`-warnings
are addressed _simultaneously_ (by removing the respective `unsafe` blocks)
no matter how complicated the patterns of `unsafe_op_in_unsafe_fn` being
disallowed and allowed throughout the function are.
--------------------------------------------------------------------------------
One noteworthy design decision I took here: An `unsafe` block
with `allow(unused_unsafe)` **is considered used** for the purposes of
linting about redundant contained unsafe blocks. So while
```rs
fn granularity() {
unsafe { //~ ERROR: unnecessary `unsafe` block
unsafe { unsf() }
unsafe { unsf() }
unsafe { unsf() }
}
}
```
warns for the outer `unsafe` block,
```rs
fn top_level_ignored() {
#[allow(unused_unsafe)]
unsafe {
#[deny(unused_unsafe)]
{
unsafe { unsf() } //~ ERROR: unnecessary `unsafe` block
unsafe { unsf() } //~ ERROR: unnecessary `unsafe` block
unsafe { unsf() } //~ ERROR: unnecessary `unsafe` block
}
}
}
```
warns on the inner ones.
Move ty::print methods to Drop-based scope guards
Primary goal is reducing codegen of the TLS access for each closure, which shaves ~3 seconds of bootstrap time over rustc as a whole.
This was largely just caching the shard value at this point, which is not
particularly useful -- in the use sites the key was being hashed nearby anyway.
Adopt let else in more places
Continuation of #89933, #91018, #91481, #93046, #93590, #94011.
I have extended my clippy lint to also recognize tuple passing and match statements. The diff caused by fixing it is way above 1 thousand lines. Thus, I split it up into multiple pull requests to make reviewing easier. This is the biggest of these PRs and handles the changes outside of rustdoc, rustc_typeck, rustc_const_eval, rustc_trait_selection, which were handled in PRs #94139, #94142, #94143, #94144.
Suggest `impl Trait` return type when incorrectly using a generic return type
Address #85991
When there is a type mismatch error and the return type is generic, and that generic parameter is not used in the function parameters, suggest replacing that generic with the `impl Trait` syntax.
r? `@estebank`
Address #85991
Suggest the `impl Trait` return type syntax if the user tried to return a generic parameter and we get a type mismatch
The suggestion is not emitted if the param appears in the function parameters, and only get the bounds that actually involve `T: ` directly
It also checks whether the generic param is contained in any where bound (where it isn't the self type), and if one is found (like `Option<T>: Send`), it is not suggested.
This also adds `TyS::contains`, which recursively vistits the type and looks if the other type is contained anywhere
Suggest copying trait associated type bounds on lifetime error
Closes#92033
Kind of the most simple suggestion to make - we don't try to be fancy. Turns out, it's still pretty useful (the couple existing tests that trigger this error end up fixed - for this error - upon applying the fix).
r? ``@estebank``
cc ``@nikomatsakis``
Improve comments about type folding/visiting.
I have found this code confusing for years. I've always roughly
understood it, but never exactly. I just made my fourth(?) attempt and
finally cracked it.
This commit improves the comments. In particular, it explicitly
describes how you can't do a custom fold/visit of any type; there are
actually a handful of "types of interest" (e.g. `Ty`, `Predicate`,
`Region`, `Const`) that can be custom folded/visted, and all other types
just get a generic traversal. I think this was the part that eluded me
on all my prior attempts at understanding.
The commit also updates comments to account for some newer changes such
as the fallible/infallible folding distinction, does some minor
reorderings, and moves one `impl` to a better place.
r? `@BoxyUwU`
I have found this code confusing for years. I've always roughly
understood it, but never exactly. I just made my fourth(?) attempt and
finally cracked it.
This commit improves the comments. In particular, it explicitly
describes how you can't do a custom fold/visit of any type; there are
actually a handful of "types of interest" (e.g. `Ty`, `Predicate`,
`Region`, `Const`) that can be custom folded/visted, and all other types
just get a generic traversal. I think this was the part that eluded me
on all my prior attempts at understanding.
The commit also updates comments to account for some newer changes such
as the fallible/infallible folding distinction, does some minor
reorderings, and moves one `impl` to a better place.
Fix inconsistent symbol mangling with -Zverbose
Always skip arguments that are the defaults of their respective
parameters, to avoid generating inconsistent symbols for builds
with `-Zverbose` flag and without it.
Support pretty printing of invalid constants
Make it possible to pretty print invalid constants by introducing a
fallible variant of `destructure_const` and falling back to debug
formatting when it fails.
Closes#93688.
Treat static refs as `mir::ConstantKind::Val`
With the upcoming introduction of Valtrees we want to treat more values as `mir::ConstantKind::Val` directly.
r? `@lcnr`
cc `@oli-obk`
Always skip arguments that are the defaults of their respective
parameters, to avoid generating inconsistent symbols for builds
with `-Zverbose` flag and without it.
Make it possible to pretty print invalid constants by introducing a
fallible variant of `destructure_const` and falling back to debug
formatting when it fails.
Specifically, rename the `Const` struct as `ConstS` and re-introduce `Const` as
this:
```
pub struct Const<'tcx>(&'tcx Interned<ConstS>);
```
This now matches `Ty` and `Predicate` more closely, including using
pointer-based `eq` and `hash`.
Notable changes:
- `mk_const` now takes a `ConstS`.
- `Const` was copy, despite being 48 bytes. Now `ConstS` is not, so need a
we need separate arena for it, because we can't use the `Dropless` one any
more.
- Many `&'tcx Const<'tcx>`/`&Const<'tcx>` to `Const<'tcx>` changes
- Many `ct.ty` to `ct.ty()` and `ct.val` to `ct.val()` changes.
- Lots of tedious sigil fiddling.
The variant names are exported, so we can use them directly (possibly
with a `ty::` qualifier). Lots of places already do this, this commit
just increases consistency.
Specifically, change `Region` from this:
```
pub type Region<'tcx> = &'tcx RegionKind;
```
to this:
```
pub struct Region<'tcx>(&'tcx Interned<RegionKind>);
```
This now matches `Ty` and `Predicate` more closely.
Things to note
- Regions have always been interned, but we haven't been using pointer-based
`Eq` and `Hash`. This is now happening.
- I chose to impl `Deref` for `Region` because it makes pattern matching a lot
nicer, and `Region` can be viewed as just a smart wrapper for `RegionKind`.
- Various methods are moved from `RegionKind` to `Region`.
- There is a lot of tedious sigil changes.
- A couple of types like `HighlightBuilder`, `RegionHighlightMode` now have a
`'tcx` lifetime because they hold a `Ty<'tcx>`, so they can call `mk_region`.
- A couple of test outputs change slightly, I'm not sure why, but the new
outputs are a little better.
Specifically, change `Ty` from this:
```
pub struct Predicate<'tcx> { inner: &'tcx PredicateInner<'tcx> }
```
to this:
```
pub struct Predicate<'tcx>(&'tcx Interned<PredicateS<'tcx>>)
```
where `PredicateInner` is renamed as `PredicateS`.
This (plus a few other minor changes) makes the parallels with `Ty` and
`TyS` much clearer, and makes the uniqueness more explicit.
Specifically, change `Ty` from this:
```
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
```
to this
```
pub struct Ty<'tcx>(Interned<'tcx, TyS<'tcx>>);
```
There are two benefits to this.
- It's now a first class type, so we can define methods on it. This
means we can move a lot of methods away from `TyS`, leaving `TyS` as a
barely-used type, which is appropriate given that it's not meant to
be used directly.
- The uniqueness requirement is now explicit, via the `Interned` type.
E.g. the pointer-based `Eq` and `Hash` comes from `Interned`, rather
than via `TyS`, which wasn't obvious at all.
Much of this commit is boring churn. The interesting changes are in
these files:
- compiler/rustc_middle/src/arena.rs
- compiler/rustc_middle/src/mir/visit.rs
- compiler/rustc_middle/src/ty/context.rs
- compiler/rustc_middle/src/ty/mod.rs
Specifically:
- Most mentions of `TyS` are removed. It's very much a dumb struct now;
`Ty` has all the smarts.
- `TyS` now has `crate` visibility instead of `pub`.
- `TyS::make_for_test` is removed in favour of the static `BOOL_TY`,
which just works better with the new structure.
- The `Eq`/`Ord`/`Hash` impls are removed from `TyS`. `Interned`s impls
of `Eq`/`Hash` now suffice. `Ord` is now partly on `Interned`
(pointer-based, for the `Equal` case) and partly on `TyS`
(contents-based, for the other cases).
- There are many tedious sigil adjustments, i.e. adding or removing `*`
or `&`. They seem to be unavoidable.
make `find_similar_impl_candidates` even fuzzier
continues the good work of `@BGR360` in #92223. I might have overshot a bit and we're now slightly too fuzzy 😅
with this we can now also simplify `simplify_type`, which is nice :3
Make `Res::SelfTy` a struct variant and update docs
I found pattern matching on a `(Option<DefId>, Option<(DefId, bool)>)` to not be super readable, additionally the doc comments on the types in a tuple variant aren't visible anywhere at use sites as far as I can tell (using rust analyzer + vscode)
The docs incorrectly assumed that the `DefId` in `Option<(DefId, bool)>` would only ever be for an impl item and I also found the code examples to be somewhat unclear about which `DefId` was being talked about.
r? `@lcnr` since you reviewed the last PR changing these docs
Improve chalk integration
- Support subtype bounds in chalk lowering
- Handle universes in canonicalization
- Handle type parameters in chalk responses
- Use `chalk_ir::LifetimeData::Empty` for `ty::ReEmpty`
- Remove `ignore-compare-mode-chalk` for tests that no longer hang (they may still fail or ICE)
This is enough to get a hello world program to compile with `-Zchalk` now. Some of the remaining issues that are needed to get Chalk integration working on larger programs are:
- rust-lang/chalk#234
- rust-lang/chalk#548
- rust-lang/chalk#734
- Generators are handled differently in chalk and rustc
r? `@jackh726`
Apply noundef attribute to &T, &mut T, Box<T>, bool
This doesn't handle `char` because it's a bit awkward to distinguish it from `u32` at this point in codegen.
Note that this _does not_ change whether or not it is UB for `&`, `&mut`, or `Box` to point to undef. It only applies to the pointer itself, not the pointed-to memory.
Fixes (partially) #74378.
r? `@nikic` cc `@RalfJung`
This is required to avoid creating large numbers of universes from each
Chalk query, while still having enough universe information for lifetime
errors.
