Make TAIT behave exactly like RPIT
fixes https://github.com/rust-lang/rust/issues/96552
This makes type-alias-impl-trait behave like return-position-impl-trait. Unfortunately it also causes some cases to stop compiling due to "needing type annotations" and makes panicking cause fallback for the hidden type to `()`.
All of these are addressable, but we should probably address them for RPIT and TAIT together
r? ``@lcnr``
Avoid some `&str` to `String` conversions with `MultiSpan::push_span_label`
This patch removes some`&str` to `String` conversions with `MultiSpan::push_span_label`.
Clean up arg mismatch diagnostic, generalize tuple wrap suggestion
This is based on top of #97542, so just look at the last commit which contains the relevant changes.
1. Remove `final_arg_types` which was one of the last places we were using raw (`usize`) indices instead of typed indices in the arg mismatch suggestion code.
2. Improve the tuple wrap suggestion, now we suggest things like `call(a, b, c, d)` -> `call(a, (b, c), d)` 😺
3. Folded in fix#98645
Reverse folder hierarchy
#91318 introduced a trait for infallible folders distinct from the fallible version. For some reason (completely unfathomable to me now that I look at it with fresh eyes), the infallible trait was a supertrait of the fallible one: that is, all fallible folders were required to also be infallible. Moreover the `Error` associated type was defined on the infallible trait! It's so absurd that it has me questioning whether I was entirely sane.
This trait reverses the hierarchy, so that the fallible trait is a supertrait of the infallible one: all infallible folders are required to also be fallible (which is a trivial blanket implementation). This of course makes much more sense! It also enables the `Error` associated type to sit on the fallible trait, where it sensibly belongs.
There is one downside however: folders expose a `tcx` accessor method. Since the blanket fallible implementation for infallible folders only has access to a generic `F: TypeFolder`, we need that trait to expose such an accessor to which we can delegate. Alternatively it's possible to extract that accessor into a separate `HasTcx` trait (or similar) that would then be a supertrait of both the fallible and infallible folder traits: this would ensure that there's only one unambiguous `tcx` method, at the cost of a little additional boilerplate. If desired, I can submit that as a separate PR.
r? ````@jackh726````
fix universes in the NLL type tests
In the NLL code, we were not accommodating universes in the
`type_test` logic.
Fixes#98095.
r? `@compiler-errors`
This breaks some tests, however, so the purpose of this branch is more explanatory and perhaps to do a crater run.
Point at return expression for RPIT-related error
Certainly this needs some diagnostic refining, but I wanted to show that it was possible first and foremost. Not sure if this is the right approach. Open to feedback.
Fixes#80583
lub: don't bail out due to empty binders
allows for the following to compile. The equivalent code using `struct Wrapper<'upper>(fn(&'upper ());` already compiles on stable.
```rust
let _: fn(&'upper ()) = match v {
true => lt_in_fn::<'a>(),
false => lt_in_fn::<'b>(),
};
```
see https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7034a677190110941223cafac6632f70 for a complete example
r? ```@rust-lang/types```
#91318 introduced a trait for infallible folders distinct from the fallible version. For some reason (completely unfathomable to me now that I look at it with fresh eyes), the infallible trait was a supertrait of the fallible one: that is, all fallible folders were required to also be infallible. Moreover the `Error` associated type was defined on the infallible trait! It's so absurd that it has me questioning whether I was entirely sane.
This trait reverses the hierarchy, so that the fallible trait is a supertrait of the infallible one: all infallible folders are required to also be fallible (which is a trivial blanket implementation). This of course makes much more sense! It also enables the `Error` associated type to sit on the fallible trait, where it sensibly belongs.
There is one downside however: folders expose a `tcx` accessor method. Since the blanket fallible implementation for infallible folders only has access to a generic `F: TypeFolder`, we need that trait to expose such an accessor to which we can delegate. Alternatively it's possible to extract that accessor into a separate `HasTcx` trait (or similar) that would then be a supertrait of both the fallible and infallible folder traits: this would ensure that there's only one unambiguous `tcx` method, at the cost of a little additional boilerplate. If desired, I can submit that as a separate PR.
r? @jackh726
The code now accepts `Binder<OutlivesPredicate>`
instead of just `OutlivesPredicate` and thus exercises
the new, generalized `IfEqBound` codepaths. Note though
that we never *produce* Binder<OutlivesPredicate>, so we
are only testing a subset of those codepaths that excludes
actual higher-ranked outlives bounds.
Make `ExprKind::Closure` a struct variant.
Simple refactor since we both need it to introduce additional fields in `ExprKind::Closure`.
r? ``@Aaron1011``
Rename the `ConstS::val` field as `kind`.
And likewise for the `Const::val` method.
Because its type is called `ConstKind`. Also `val` is a confusing name
because `ConstKind` is an enum with seven variants, one of which is
called `Value`. Also, this gives consistency with `TyS` and `PredicateS`
which have `kind` fields.
The commit also renames a few `Const` variables from `val` to `c`, to
avoid confusion with the `ConstKind::Value` variant.
r? `@BoxyUwU`
Remove RegionckMode in favor of calling new skip_region_resolution
Simple cleanup. We can skip a bunch of stuff for places where NLL does the region checking, so skip earlier.
r? rust-lang/types
And likewise for the `Const::val` method.
Because its type is called `ConstKind`. Also `val` is a confusing name
because `ConstKind` is an enum with seven variants, one of which is
called `Value`. Also, this gives consistency with `TyS` and `PredicateS`
which have `kind` fields.
The commit also renames a few `Const` variables from `val` to `c`, to
avoid confusion with the `ConstKind::Value` variant.
Mention `infer::Trace` methods on `infer::At` methods' docs
I missed that you could do `infcx.at(...).trace(...).eq(a, b)` when `a` and `b` dont implement `ToTrace` but does implement `Relate` these docs would have helped see that 😅
This commit makes type folding more like the way chalk does it.
Currently, `TypeFoldable` has `fold_with` and `super_fold_with` methods.
- `fold_with` is the standard entry point, and defaults to calling
`super_fold_with`.
- `super_fold_with` does the actual work of traversing a type.
- For a few types of interest (`Ty`, `Region`, etc.) `fold_with` instead
calls into a `TypeFolder`, which can then call back into
`super_fold_with`.
With the new approach, `TypeFoldable` has `fold_with` and
`TypeSuperFoldable` has `super_fold_with`.
- `fold_with` is still the standard entry point, *and* it does the
actual work of traversing a type, for all types except types of
interest.
- `super_fold_with` is only implemented for the types of interest.
Benefits of the new model.
- I find it easier to understand. The distinction between types of
interest and other types is clearer, and `super_fold_with` doesn't
exist for most types.
- With the current model is easy to get confused and implement a
`super_fold_with` method that should be left defaulted. (Some of the
precursor commits fixed such cases.)
- With the current model it's easy to call `super_fold_with` within
`TypeFolder` impls where `fold_with` should be called. The new
approach makes this mistake impossible, and this commit fixes a number
of such cases.
- It's potentially faster, because it avoids the `fold_with` ->
`super_fold_with` call in all cases except types of interest. A lot of
the time the compile would inline those away, but not necessarily
always.
Remove migrate borrowck mode
Closes#58781Closes#43234
# Stabilization proposal
This PR proposes the stabilization of `#![feature(nll)]` and the removal of `-Z borrowck`. Current borrow checking behavior of item bodies is currently done by first infering regions *lexically* and reporting any errors during HIR type checking. If there *are* any errors, then MIR borrowck (NLL) never occurs. If there *aren't* any errors, then MIR borrowck happens and any errors there would be reported. This PR removes the lexical region check of item bodies entirely and only uses MIR borrowck. Because MIR borrowck could never *not* be run for a compiled program, this should not break any programs. It does, however, change diagnostics significantly and allows a slightly larger set of programs to compile.
Tracking issue: #43234
RFC: https://github.com/rust-lang/rfcs/blob/master/text/2094-nll.md
Version: 1.63 (2022-06-30 => beta, 2022-08-11 => stable).
## Motivation
Over time, the Rust borrow checker has become "smarter" and thus allowed more programs to compile. There have been three different implementations: AST borrowck, MIR borrowck, and polonius (well, in progress). Additionally, there is the "lexical region resolver", which (roughly) solves the constraints generated through HIR typeck. It is not a full borrow checker, but does emit some errors.
The AST borrowck was the original implementation of the borrow checker and was part of the initially stabilized Rust 1.0. In mid 2017, work began to implement the current MIR borrow checker and that effort ompleted by the end of 2017, for the most part. During 2018, efforts were made to migrate away from the AST borrow checker to the MIR borrow checker - eventually culminating into "migrate" mode - where HIR typeck with lexical region resolving following by MIR borrow checking - being active by default in the 2018 edition.
In early 2019, migrate mode was turned on by default in the 2015 edition as well, but with MIR borrowck errors emitted as warnings. By late 2019, these warnings were upgraded to full errors. This was followed by the complete removal of the AST borrow checker.
In the period since, various errors emitted by the MIR borrow checker have been improved to the point that they are mostly the same or better than those emitted by the lexical region resolver.
While there do remain some degradations in errors (tracked under the [NLL-diagnostics tag](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3ANLL-diagnostics), those are sufficiently small and rare enough that increased flexibility of MIR borrow check-only is now a worthwhile tradeoff.
## What is stabilized
As said previously, this does not fundamentally change the landscape of accepted programs. However, there are a [few](https://github.com/rust-lang/rust/issues?q=is%3Aopen+is%3Aissue+label%3ANLL-fixed-by-NLL) cases where programs can compile under `feature(nll)`, but not otherwise.
There are two notable patterns that are "fixed" by this stabilization. First, the `scoped_threads` feature, which is a continutation of a pre-1.0 API, can sometimes emit a [weird lifetime error](https://github.com/rust-lang/rust/issues/95527) without NLL. Second, actually seen in the standard library. In the `Extend` impl for `HashMap`, there is an implied bound of `K: 'a` that is available with NLL on but not without - this is utilized in the impl.
As mentioned before, there are a large number of diagnostic differences. Most of them are better, but some are worse. None are serious or happen often enough to need to block this PR. The biggest change is the loss of error code for a number of lifetime errors in favor of more general "lifetime may not live long enough" error. While this may *seem* bad, the former error codes were just attempts to somewhat-arbitrarily bin together lifetime errors of the same type; however, on paper, they end up being roughly the same with roughly the same kinds of solutions.
## What isn't stabilized
This PR does not completely remove the lexical region resolver. In the future, it may be possible to remove that (while still keeping HIR typeck) or to remove it together with HIR typeck.
## Tests
Many test outputs get updated by this PR. However, there are number of tests specifically geared towards NLL under `src/test/ui/nll`
## History
* On 2017-07-14, [tracking issue opened](https://github.com/rust-lang/rust/issues/43234)
* On 2017-07-20, [initial empty MIR pass added](https://github.com/rust-lang/rust/pull/43271)
* On 2017-08-29, [RFC opened](https://github.com/rust-lang/rfcs/pull/2094)
* On 2017-11-16, [Integrate MIR type-checker with NLL](https://github.com/rust-lang/rust/pull/45825)
* On 2017-12-20, [NLL feature complete](https://github.com/rust-lang/rust/pull/46862)
* On 2018-07-07, [Don't run AST borrowck on mir mode](https://github.com/rust-lang/rust/pull/52083)
* On 2018-07-27, [Add migrate mode](https://github.com/rust-lang/rust/pull/52681)
* On 2019-04-22, [Enable migrate mode on 2015 edition](https://github.com/rust-lang/rust/pull/59114)
* On 2019-08-26, [Don't downgrade errors on 2015 edition](https://github.com/rust-lang/rust/pull/64221)
* On 2019-08-27, [Remove AST borrowck](https://github.com/rust-lang/rust/pull/64790)
rewrite error handling for unresolved inference vars
Pretty much completely rewrites `fn emit_inference_failure_err`.
This new setup should hopefully be easier to extend and is already a lot better when looking for generic arguments.
Because this is a rewrite there are still some parts which are lacking, these are tracked in #94483 and will be fixed in later PRs.
r? `@estebank` `@petrochenkov`
Diagnose anonymous lifetimes errors more uniformly between async and regular fns
Async fns and regular fns are desugared differently. For the former, we create a generic parameter at HIR level. For the latter, we just create an anonymous region for typeck.
I plan to migrate regular fns to the async fn desugaring.
Before that, this PR attempts to merge the diagnostics for both cases.
r? ```@estebank```
Finish bumping stage0
It looks like the last time had left some remaining cfg's -- which made me think
that the stage0 bump was actually successful. This brings us to a released 1.62
beta though.
This now brings us to cfg-clean, with the exception of check-cfg-features in bootstrap;
I'd prefer to leave that for a separate PR at this time since it's likely to be more tricky.
cc https://github.com/rust-lang/rust/pull/97147#issuecomment-1132845061
r? `@pietroalbini`
It looks like the last time had left some remaining cfg's -- which made me think
that the stage0 bump was actually successful. This brings us to a released 1.62
beta though.
Lifetime variance fixes for rustc
#97287 migrates rustc to a `Ty` type that is invariant over its lifetime `'tcx`, so I need to fix a bunch of places that assume that `Ty<'a>` and `Ty<'b>` can be unified by shortening both to some common lifetime.
This is doable, since many lifetimes are already `'tcx`, so all this PR does is be a bit more explicit that elided lifetimes are actually `'tcx`.
Split out from #97287 so the compiler team can review independently.
Add EarlyBinder
Chalk has no concept of `Param` (e0ade19d13/chalk-ir/src/lib.rs (L579)) or `ReEarlyBound` (e0ade19d13/chalk-ir/src/lib.rs (L1308)). Everything is just "bound" - the equivalent of rustc's late-bound. It's not completely clear yet whether to move everything to the same time of binder in rustc or add `Param` and `ReEarlyBound` in Chalk.
Either way, tracking when we have or haven't already substituted out these in rustc can be helpful.
As a first step, I'm just adding a `EarlyBinder` newtype that is required to call `subst`. I also add a couple "transparent" `bound_*` wrappers around a couple query that are often immediately substituted.
r? `@nikomatsakis`
Begin fixing all the broken doctests in `compiler/`
Begins to fix#95994.
All of them pass now but 24 of them I've marked with `ignore HELP (<explanation>)` (asking for help) as I'm unsure how to get them to work / if we should leave them as they are.
There are also a few that I marked `ignore` that could maybe be made to work but seem less important.
Each `ignore` has a rough "reason" for ignoring after it parentheses, with
- `(pseudo-rust)` meaning "mostly rust-like but contains foreign syntax"
- `(illustrative)` a somewhat catchall for either a fragment of rust that doesn't stand on its own (like a lone type), or abbreviated rust with ellipses and undeclared types that would get too cluttered if made compile-worthy.
- `(not-rust)` stuff that isn't rust but benefits from the syntax highlighting, like MIR.
- `(internal)` uses `rustc_*` code which would be difficult to make work with the testing setup.
Those reason notes are a bit inconsistently applied and messy though. If that's important I can go through them again and try a more principled approach. When I run `rg '```ignore \(' .` on the repo, there look to be lots of different conventions other people have used for this sort of thing. I could try unifying them all if that would be helpful.
I'm not sure if there was a better existing way to do this but I wrote my own script to help me run all the doctests and wade through the output. If that would be useful to anyone else, I put it here: https://github.com/Elliot-Roberts/rust_doctest_fixing_tool
Only crate root def-ids don't have a parent, and in majority of cases the argument of `DefIdTree::parent` cannot be a crate root.
So we now panic by default in `parent` and introduce a new non-panicing function `opt_parent` for cases where the argument can be a crate root.
Same applies to `local_parent`/`opt_local_parent`.
Erase type params when suggesting fully qualified path
When suggesting the use of a fully qualified path for a method call that
is ambiguous because it has multiple candidates, erase type params in
the resulting code, as they would result in an error when applied. We
replace them with `_` in the output to rely on inference. There might be
cases where this still produces slighlty incomplete suggestions, but it
otherwise produces many more errors in relatively common cases.
Fix#96292
Change `span_suggestion` (and variants) to take `impl ToString` rather
than `String` for the suggested code, as this simplifies the
requirements on the diagnostic derive.
Signed-off-by: David Wood <david.wood@huawei.com>
Because NLL borrowck is run after typeck, `in_progress_typeck_results`
was always `None` which was preventing the retrieval of the span to which
the suggestion is suppose to add the lifetime bound.
We now manually pass the `LocalDefId` owner to `construct_generic_bound_failure`
so that under NLL, we give the owner id of the current body.
When suggesting the use of a fully qualified path for a method call that
is ambiguous because it has multiple candidates, erase type params in
the resulting code, as they would result in an error when applied. We
replace them with `_` in the output to rely on inference. There might be
cases where this still produces slighlty incomplete suggestions, but it
otherwise produces many more errors in relatively common cases.
Fix#96292
Better method call error messages
Rebase/continuation of #71827
~Based on #92360~
~Based on #93118~
There's a decent description in #71827 that I won't copy here (for now at least)
In addition to rebasing, I've tried to restore most of the original suggestions for invalid arguments. Unfortunately, this does make some of the errors a bit verbose. To fix this will require a bit of refactoring to some of the generalized error suggestion functions, and I just don't have the time to go into it right now.
I think this is in a state that the error messages are overall better than before without a reduction in the suggestions given.
~I've tried to split out some of the easier and self-contained changes into separate commits (mostly in #92360, but also one here). There might be more than can be done here, but again just lacking time.~
r? `@estebank` as the original reviewer of #71827
This attempts to bring better error messages to invalid method calls, by applying some heuristics to identify common mistakes.
The algorithm is inspired by Levenshtein distance and longest common sub-sequence. In essence, we treat the types of the function, and the types of the arguments you provided as two "words" and compute the edits to get from one to the other.
We then modify that algorithm to detect 4 cases:
- A function input is missing
- An extra argument was provided
- The type of an argument is straight up invalid
- Two arguments have been swapped
- A subset of the arguments have been shuffled
(We detect the last two as separate cases so that we can detect two swaps, instead of 4 parameters permuted.)
It helps to understand this argument by paying special attention to terminology: "inputs" refers to the inputs being *expected* by the function, and "arguments" refers to what has been provided at the call site.
The basic sketch of the algorithm is as follows:
- Construct a boolean grid, with a row for each argument, and a column for each input. The cell [i, j] is true if the i'th argument could satisfy the j'th input.
- If we find an argument that could satisfy no inputs, provided for an input that can't be satisfied by any other argument, we consider this an "invalid type".
- Extra arguments are those that can't satisfy any input, provided for an input that *could* be satisfied by another argument.
- Missing inputs are inputs that can't be satisfied by any argument, where the provided argument could satisfy another input
- Swapped / Permuted arguments are identified with a cycle detection algorithm.
As each issue is found, we remove the relevant inputs / arguments and check for more issues. If we find no issues, we match up any "valid" arguments, and start again.
Note that there's a lot of extra complexity:
- We try to stay efficient on the happy path, only computing the diagonal until we find a problem, and then filling in the rest of the matrix.
- Closure arguments are wrapped in a tuple and need to be unwrapped
- We need to resolve closure types after the rest, to allow the most specific type constraints
- We need to handle imported C functions that might be variadic in their inputs.
I tried to document a lot of this in comments in the code and keep the naming clear.
Stabilize `derive_default_enum`
This stabilizes `#![feature(derive_default_enum)]`, as proposed in [RFC 3107](https://github.com/rust-lang/rfcs/pull/3107) and tracked in #87517. In short, it permits you to `#[derive(Default)]` on `enum`s, indicating what the default should be by placing a `#[default]` attribute on the desired variant (which must be a unit variant in the interest of forward compatibility).
```````@rustbot``````` label +S-waiting-on-review +T-lang
Cached stable hash cleanups
r? `@nnethercote`
Add a sanity assertion in debug mode to check that the cached hashes are actually the ones we get if we compute the hash each time.
Add a new data structure that bundles all the hash-caching work to make it easier to re-use it for different interned data structures
This commit updates the signatures of all diagnostic functions to accept
types that can be converted into a `DiagnosticMessage`. This enables
existing diagnostic calls to continue to work as before and Fluent
identifiers to be provided. The `SessionDiagnostic` derive just
generates normal diagnostic calls, so these APIs had to be modified to
accept Fluent identifiers.
In addition, loading of the "fallback" Fluent bundle, which contains the
built-in English messages, has been implemented.
Each diagnostic now has "arguments" which correspond to variables in the
Fluent messages (necessary to render a Fluent message) but no API for
adding arguments has been added yet. Therefore, diagnostics (that do not
require interpolation) can be converted to use Fluent identifiers and
will be output as before.
`MultiSpan` contains labels, which are more complicated with the
introduction of diagnostic translation and will use types from
`rustc_errors` - however, `rustc_errors` depends on `rustc_span` so
`rustc_span` cannot use types like `DiagnosticMessage` without
dependency cycles. Introduce a new `rustc_error_messages` crate that can
contain `DiagnosticMessage` and `MultiSpan`.
Signed-off-by: David Wood <david.wood@huawei.com>
Better suggestions for `Fn`-family trait selection errors
1. Suppress suggestions to add `std::ops::Fn{,Mut,Once}` bounds when a type already implements `Fn{,Mut,Once}`
2. Add a note that points out that a type does in fact implement `Fn{,Mut,Once}`, but the arguments vary (either by number or by actual arguments)
3. Add a note that points out that a type does in fact implement `Fn{,Mut,Once}`, but not the right one (e.g. implements `FnMut`, but `Fn` is required).
Fixes#95147
Lazy type-alias-impl-trait take two
### user visible change 1: RPIT inference from recursive call sites
Lazy TAIT has an insta-stable change. The following snippet now compiles, because opaque types can now have their hidden type set from wherever the opaque type is mentioned.
```rust
fn bar(b: bool) -> impl std::fmt::Debug {
if b {
return 42
}
let x: u32 = bar(false); // this errors on stable
99
}
```
The return type of `bar` stays opaque, you can't do `bar(false) + 42`, you need to actually mention the hidden type.
### user visible change 2: divergence between RPIT and TAIT in return statements
Note that `return` statements and the trailing return expression are special with RPIT (but not TAIT). So
```rust
#![feature(type_alias_impl_trait)]
type Foo = impl std::fmt::Debug;
fn foo(b: bool) -> Foo {
if b {
return vec![42];
}
std::iter::empty().collect() //~ ERROR `Foo` cannot be built from an iterator
}
fn bar(b: bool) -> impl std::fmt::Debug {
if b {
return vec![42]
}
std::iter::empty().collect() // Works, magic (accidentally stabilized, not intended)
}
```
But when we are working with the return value of a recursive call, the behavior of RPIT and TAIT is the same:
```rust
type Foo = impl std::fmt::Debug;
fn foo(b: bool) -> Foo {
if b {
return vec![];
}
let mut x = foo(false);
x = std::iter::empty().collect(); //~ ERROR `Foo` cannot be built from an iterator
vec![]
}
fn bar(b: bool) -> impl std::fmt::Debug {
if b {
return vec![];
}
let mut x = bar(false);
x = std::iter::empty().collect(); //~ ERROR `impl Debug` cannot be built from an iterator
vec![]
}
```
### user visible change 3: TAIT does not merge types across branches
In contrast to RPIT, TAIT does not merge types across branches, so the following does not compile.
```rust
type Foo = impl std::fmt::Debug;
fn foo(b: bool) -> Foo {
if b {
vec![42_i32]
} else {
std::iter::empty().collect()
//~^ ERROR `Foo` cannot be built from an iterator over elements of type `_`
}
}
```
It is easy to support, but we should make an explicit decision to include the additional complexity in the implementation (it's not much, see a721052457cf513487fb4266e3ade65c29b272d2 which needs to be reverted to enable this).
### PR formalities
previous attempt: #92007
This PR also includes #92306 and #93783, as they were reverted along with #92007 in #93893fixes#93411fixes#88236fixes#89312fixes#87340fixes#86800fixes#86719fixes#84073fixes#83919fixes#82139fixes#77987fixes#74282fixes#67830fixes#62742fixes#54895
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.
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.
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`
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`.
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.
Edit `rustc_trait_selection::infer::lattice` docs
Closes#94311.
Removes mentions of outdated/missing type and filename (`infer.rs` and `LatticeValue`).
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
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`
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.
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.
Only mark projection as ambiguous if GAT substs are constrained
A slightly more targeted version of #92917, where we only give up with ambiguity if we infer something about the GATs substs when probing for a projection candidate.
fixes#93874
also note (but like the previous PR, does not fix) #91762
r? `@jackh726`
cc `@nikomatsakis` who reviewed #92917
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``
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 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 `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`
This is required to avoid creating large numbers of universes from each
Chalk query, while still having enough universe information for lifetime
errors.
Improve opaque type higher-ranked region error message under NLL
Currently, any higher-ranked region errors involving opaque types
fall back to a generic "higher-ranked subtype error" message when
run under NLL. This PR adds better error message handling for this
case, giving us the same kinds of error messages that we currently
get without NLL:
```
error: implementation of `MyTrait` is not general enough
--> $DIR/opaque-hrtb.rs:12:13
|
LL | fn foo() -> impl for<'a> MyTrait<&'a str> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `MyTrait` is not general enough
|
= note: `impl MyTrait<&'2 str>` must implement `MyTrait<&'1 str>`, for any lifetime `'1`...
= note: ...but it actually implements `MyTrait<&'2 str>`, for some specific lifetime `'2`
error: aborting due to previous error
```
To accomplish this, several different refactoring needed to be made:
* We now have a dedicated `InstantiateOpaqueType` struct which
implements `TypeOp`. This is used to invoke `instantiate_opaque_types`
during MIR type checking.
* `TypeOp` is refactored to pass around a `MirBorrowckCtxt`, which is
needed to report opaque type region errors.
* We no longer assume that all `TypeOp`s correspond to canonicalized
queries. This allows us to properly handle opaque type instantiation
(which does not occur in a query) as a `TypeOp`.
A new `ErrorInfo` associated type is used to determine what
additional information is used during higher-ranked region error
handling.
* The body of `try_extract_error_from_fulfill_cx`
has been moved out to a new function `try_extract_error_from_region_constraints`.
This allows us to re-use the same error reporting code between
canonicalized queries (which can extract region constraints directly
from a fresh `InferCtxt`) and opaque type handling (which needs to take
region constraints from the pre-existing `InferCtxt` that we use
throughout MIR borrow checking).
Currently, any higher-ranked region errors involving opaque types
fall back to a generic "higher-ranked subtype error" message when
run under NLL. This PR adds better error message handling for this
case, giving us the same kinds of error messages that we currently
get without NLL:
```
error: implementation of `MyTrait` is not general enough
--> $DIR/opaque-hrtb.rs:12:13
|
LL | fn foo() -> impl for<'a> MyTrait<&'a str> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `MyTrait` is not general enough
|
= note: `impl MyTrait<&'2 str>` must implement `MyTrait<&'1 str>`, for any lifetime `'1`...
= note: ...but it actually implements `MyTrait<&'2 str>`, for some specific lifetime `'2`
error: aborting due to previous error
```
To accomplish this, several different refactoring needed to be made:
* We now have a dedicated `InstantiateOpaqueType` struct which
implements `TypeOp`. This is used to invoke `instantiate_opaque_types`
during MIR type checking.
* `TypeOp` is refactored to pass around a `MirBorrowckCtxt`, which is
needed to report opaque type region errors.
* We no longer assume that all `TypeOp`s correspond to canonicalized
queries. This allows us to properly handle opaque type instantiation
(which does not occur in a query) as a `TypeOp`.
A new `ErrorInfo` associated type is used to determine what
additional information is used during higher-ranked region error
handling.
* The body of `try_extract_error_from_fulfill_cx`
has been moved out to a new function `try_extract_error_from_region_constraints`.
This allows us to re-use the same error reporting code between
canonicalized queries (which can extract region constraints directly
from a fresh `InferCtxt`) and opaque type handling (which needs to take
region constraints from the pre-existing `InferCtxt` that we use
throughout MIR borrow checking).
Do not suggest char literal for zero-length strings
PR #92507 adds a hint to switch to single quotes when a char is expected and a single-character string literal is provided.
The check to ensure the string literal is one character long missed the 0-char case, and would incorrectly offer the hint.
This PR adds the missing check, and a test case to confirm the new behavior.
Add in ValuePair::Term
This adds in an enum when matching on positions which can either be types or consts.
It will default to emitting old special cased error messages for types.
r? `@oli-obk`
cc `@matthiaskrgr`
Fixes#93578
Lazy type-alias-impl-trait
Previously opaque types were processed by
1. replacing all mentions of them with inference variables
2. memorizing these inference variables in a side-table
3. at the end of typeck, resolve the inference variables in the side table and use the resolved type as the hidden type of the opaque type
This worked okayish for `impl Trait` in return position, but required lots of roundabout type inference hacks and processing.
This PR instead stops this process of replacing opaque types with inference variables, and just keeps the opaque types around.
Whenever an opaque type `O` is compared with another type `T`, we make the comparison succeed and record `T` as the hidden type. If `O` is compared to `U` while there is a recorded hidden type for it, we grab the recorded type (`T`) and compare that against `U`. This makes implementing
* https://github.com/rust-lang/rfcs/pull/2515
much simpler (previous attempts on the inference based scheme were very prone to ICEs and general misbehaviour that was not explainable except by random implementation defined oddities).
r? `@nikomatsakis`
fixes#93411fixes#88236
Suggest 1-tuple parentheses on exprs without existing parens
A follow-on from #86116, split out from #90677.
This alters the suggestion to add a trailing comma to create a 1-tuple - previously we would only apply this if the relevant expression was parenthesised. We now make the suggestion regardless of parentheses, which reduces the fragility of the check (w.r.t formatting).
e.g.
```rust
let a: Option<(i32,)> = Some(3);
```
gets the below suggestion:
```rust
let a: Option<(i32,)> = Some((3,));
// ^ ^^
```
This change also improves the suggestion in other ways, such as by only making the suggestion if the types would match after the suggestion is applied and making the suggestion a multipart suggestion.
This adds in an enum when matching on positions which can either be types or consts.
It will default to emitting old special cased error messages for types.
This means we stop supporting the case where a locally defined trait has only a single impl so we can always use that impl (see nested-tait-inference.rs).
by using an opaque type obligation to bubble up comparisons between opaque types and other types
Also uses proper obligation causes so that the body id works, because out of some reason nll uses body ids for logic instead of just diagnostics.
Continue work on associated const equality
This actually implements some more complex logic for assigning associated consts to values.
Inside of projection candidates, it now defers to a separate function for either consts or
types. To reduce amount of code, projections are now generic over T, where T is either a Type or
a Const. I can add some comments back later, but this was the fastest way to implement it.
It also now finds the correct type of consts in type_of.
---
The current main TODO is finding the const of the def id for the LeafDef.
Right now it works if the function isn't called, but once you use the trait impl with the bound it fails inside projection.
I was hoping to get some help in getting the `&'tcx ty::Const<'tcx>`, in addition to a bunch of other `todo!()`s which I think may not be hit.
r? `@oli-obk`
Updates #92827
Store a `Symbol` instead of an `Ident` in `AssocItem`
This is the same idea as #92533, but for `AssocItem` instead
of `VariantDef`/`FieldDef`.
With this change, we no longer have any uses of
`#[stable_hasher(project(...))]`
This is the same idea as #92533, but for `AssocItem` instead
of `VariantDef`/`FieldDef`.
With this change, we no longer have any uses of
`#[stable_hasher(project(...))]`
Fix ICEs related to `Deref<Target=[T; N]>` on newtypes
1. Stash a const infer's type into the canonical var during canonicalization, so we can recreate the fresh const infer with that same type.
For example, given `[T; _]` we know `_` is a `usize`. If we go from infer => canonical => infer, we shouldn't forget that variable is a usize.
Fixes#92626Fixes#83704
2. Don't stash the autoderef'd slice type that we get from method lookup, but instead recreate it during method confirmation. We need to do this because the type we receive back after picking the method references a type variable that does not exist after probing is done.
Fixes#92637
... A better solution for the second issue would be to actually _properly_ implement `Deref` for `[T; N]` instead of fixing this autoderef hack to stop leaking inference variables. But I actually looked into this, and there are many complications with const impls.
Replace use of `ty()` on term and use it in more places. This will allow more flexibility in the
future, but slightly worried it allows items which are consts which only accept types.
ProjectionPredicate should be able to handle both associated types and consts so this adds the
first step of that. It mainly just pipes types all the way down, not entirely sure how to handle
consts, but hopefully that'll come with time.
Include Projections when elaborating TypeOutlives
Fixes#92280
In `Elaborator`, we elaborate that `Foo<<Bar as Baz>::Assoc>: 'a` -> `<Bar as Baz>::Assoc: 'a`. This is the same rule that would be applied to any other `Param`. If there are escaping vars, we continue to do nothing.
r? `@nikomatsakis`
Prefer projection candidates instead of param_env candidates for Sized predicates
Fixes#89352
Also includes some drive by logging and verbose printing changes that I found useful when debugging this, but I can remove this if needed.
This is a little hacky - but imo no more than the rest of `candidate_should_be_dropped_in_favor_of`. Importantly, in a Chalk-like world, both candidates should be completely compatible.
r? ```@nikomatsakis```
Don't fall back to crate-level opaque type definitions.
That would just hide bugs, as it works accidentally if the opaque type is defined at the crate level.
Only works after #90948 which worked by accident for our entire test suite.
Welcome opaque types into the fold
r? ```@nikomatsakis``` because idk who else to bug on the type_op changes
The commits have explanations in them. The TLDR is that
* 5c46002273 stops the "recurse and replace" scheme that replaces opaque types with their canonical inference var by just doing that ahead of time
* bdeeb07bf6 does not affect anything on master afaict, but since opaque types generate obligations when instantiated, and lazy TAIT instantiates opaque types *everywhere*, we need to properly handle obligations here instead of just hoping no problematic obligations ever come up.
Store a `Symbol` instead of an `Ident` in `VariantDef`/`FieldDef`
The field is also renamed from `ident` to `name`. In most cases,
we don't actually need the `Span`. A new `ident` method is added
to `VariantDef` and `FieldDef`, which constructs the full `Ident`
using `tcx.def_ident_span()`. This method is used in the cases
where we actually need an `Ident`.
This makes incremental compilation properly track changes
to the `Span`, without all of the invalidations caused by storing
a `Span` directly via an `Ident`.
The field is also renamed from `ident` to `name. In most cases,
we don't actually need the `Span`. A new `ident` method is added
to `VariantDef` and `FieldDef`, which constructs the full `Ident`
using `tcx.def_ident_span()`. This method is used in the cases
where we actually need an `Ident`.
This makes incremental compilation properly track changes
to the `Span`, without all of the invalidations caused by storing
a `Span` directly via an `Ident`.
Instead of special-casing mutable pointers/references, we
now support general generic types (currently, we handle
`ty::Ref`, `ty::RawPtr`, and `ty::Adt`)
When a `ty::Adt` is involved, we show an additional note
explaining which of the type's generic parameters is
invariant (e.g. the `T` in `Cell<T>`). Currently, we don't
explain *why* a particular generic parameter ends up becoming
invariant. In the general case, this could require printing
a long 'backtrace' of types, so doing this would be
more suitable for a follow-up PR.
We still only handle the case where our variance switches
to `ty::Invariant`.
This makes `Obligation` two words bigger, but avoids allocating a lot of
the time.
I previously tried this in #73983 and it didn't help much, but local
timings look more promising now.
Remove `in_band_lifetimes` from `rustc_infer`
See #91867 for more information.
This crate actually had a typo `'ctx` in one of its functions:
```diff
-pub fn same_type_modulo_infer(a: Ty<'tcx>, b: Ty<'ctx>) -> bool {
+pub fn same_type_modulo_infer<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
```
Also, I wasn't entirely sure about the lifetimes in `suggest_new_region_bound`:
```diff
pub fn suggest_new_region_bound(
- tcx: TyCtxt<'tcx>,
+ tcx: TyCtxt<'_>,
err: &mut DiagnosticBuilder<'_>,
fn_returns: Vec<&rustc_hir::Ty<'_>>,
```
Should all of those lifetimes really be distinct?
Remove `SymbolStr`
This was originally proposed in https://github.com/rust-lang/rust/pull/74554#discussion_r466203544. As well as removing the icky `SymbolStr` type, it allows the removal of a lot of `&` and `*` occurrences.
Best reviewed one commit at a time.
r? `@oli-obk`
Instead of clearing out the cache entirely, we store
the intermediate evaluation result into the cache entry.
This accomplishes several things:
* We avoid the performance hit associated with re-evaluating
the sub-obligations
* We avoid causing issues with incremental compilation, since
the final evaluation result is always the same
* We avoid affecting other uses of the same `InferCtxt` which
might care about 'side effects' from processing the sub-obligations
(e,g. region constraints). Only code that is specifically aware
of the new 'complete' code is affected
This crate actually had a typo `'ctx` in one of its functions:
```diff
-pub fn same_type_modulo_infer(a: Ty<'tcx>, b: Ty<'ctx>) -> bool {
+pub fn same_type_modulo_infer<'tcx>(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
```
Tweak assoc type obligation spans
* Point at RHS of associated type in obligation span
* Point at `impl` assoc type on projection error
* Reduce verbosity of recursive obligations
* Point at source of binding lifetime obligation
* Tweak "required bound" note
* Tweak "expected... found opaque (return) type" labels
* Point at set type in impl assoc type WF errors
r? `@oli-obk`
This is a(n uncontroversial) subset of #85799.
* Point at RHS of associated type in obligation span
* Point at `impl` assoc type on projection error
* Reduce verbosity of recursive obligations
* Point at source of binding lifetime obligation
* Tweak "required bound" note
* Tweak "expected... found opaque (return) type" labels
* Point at set type in impl assoc type WF errors
```
error[E0759]: `self` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement
--> $DIR/issue-72312.rs:10:24
|
LL | pub async fn start(&self) {
| ^^^^^ this data with an anonymous lifetime `'_`...
...
LL | require_static(async move {
| -------------- ...is required to live as long as `'static` here...
LL | &self;
| ----- ...and is captured here
|
note: `'static` lifetime requirement introduced by this trait bound
--> $DIR/issue-72312.rs:2:22
|
LL | fn require_static<T: 'static>(val: T) -> T {
| ^^^^^^^
error: aborting due to previous error
For more information about this error, try `rustc --explain E0759`.
```
Fix#72312.
Only shown relevant type params in E0283 label
When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.
Remove a dead code path.
It is neither documented nor can I see any way it could ever be reached.
Also, no tests fail when turning that arm into an ICE
When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.
Cleanup: Eliminate ConstnessAnd
This is almost a behaviour-free change and purely a refactoring. "almost" because we appear to be using the wrong ParamEnv somewhere already, and this is now exposed by failing a test using the unstable `~const` feature.
We most definitely need to review all `without_const` and at some point should probably get rid of many of them by using `TraitPredicate` instead of `TraitRef`.
This is a continuation of https://github.com/rust-lang/rust/pull/90274.
r? `@oli-obk`
cc `@spastorino` `@ecstatic-morse`
Make `TypeFolder::fold_*` return `Result`
Implements rust-lang/compiler-team#432.
Initially this is just a rebase of `@LeSeulArtichaut's` work in #85469 (abandoned; see https://github.com/rust-lang/rust/pull/85485#issuecomment-908781112). At that time, it caused a regression in performance that required some further exploration... with this rebased PR bors can hopefully report some perf analysis from which we can investigate further (if the regression is indeed still present).
r? `@jackh726` cc `@nikomatsakis`
Rollup of 4 pull requests
Successful merges:
- #91008 (Adds IEEE 754-2019 minimun and maximum functions for f32/f64)
- #91070 (Make `LLVMRustGetOrInsertGlobal` always return a `GlobalVariable`)
- #91097 (Add spaces in opaque `impl Trait` with more than one trait)
- #91098 (Don't suggest certain fixups (`.field`, `.await`, etc) when reporting errors while matching on arrays )
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Point at source of trait bound obligations in more places
Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).
In particular, we now point at trait bounds on method calls.
We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).
We no longer point at associated items (`ImplObligation`), as they didn't
add any user actionable information, they just added noise.
Address part of #89418.
Be more thorough in using `ItemObligation` and `BindingObligation` when
evaluating obligations so that we can point at trait bounds that
introduced unfulfilled obligations. We no longer incorrectly point at
unrelated trait bounds (`substs-ppaux.verbose.stderr`).
In particular, we now point at trait bounds on method calls.
We no longer point at "obvious" obligation sources (we no longer have a
note pointing at `Trait` saying "required by a bound in `Trait`", like
in `associated-types-no-suitable-supertrait*`).
Address part of #89418.
Fix span for non-satisfied trivial trait bounds
The spans for "trait bound not satisfied" errors in trivial trait bounds referenced the entire item (fn, impl, struct) before.
Now they only reference the obligation itself (`String: Copy`)
Address #90869
Improve diagnostics when a static lifetime is expected
Makes progress towards https://github.com/rust-lang/rust/issues/90600
The diagnostics here were previously entirely removed due to giving a misleading suggestion but if we instead provide an informative label in that same location it should better help the user understand the situation.
I included the example from the issue as it demonstrates an area where the diagnostics are still lacking.
Happy to remove that if its just adding noise atm.
The spans for "trait bound not satisfied" errors in trivial trait bounds referenced the entire item (fn, impl, struct) before.
Now they only reference the obligation itself (`String: Copy`)
Address #90869
Type inference for inline consts
Fixes#78132Fixes#78174Fixes#81857Fixes#89964
Perform type checking/inference of inline consts in the same context as the outer def, similar to what is currently done to closure.
Doing so would require `closure_base_def_id` of the inline const to return the outer def, and since `closure_base_def_id` can be called on non-local crate (and thus have no HIR available), a new `DefKind` is created for inline consts.
The type of the generated anon const can capture lifetime of outer def, so we couldn't just use the typeck result as the type of the inline const's def. Closure has a similar issue, and it uses extra type params `CK, CS, U` to capture closure kind, input/output signature and upvars. I use a similar approach for inline consts, letting it have an extra type param `R`, and then `typeof(InlineConst<[paremt generics], R>)` would just be `R`. In borrowck region requirements are also propagated to the outer MIR body just like it's currently done for closure.
With this PR, inline consts in expression position are quitely usable now; however the usage in pattern position is still incomplete -- since those does not remain in the MIR borrowck couldn't verify the lifetime there. I have left an ignored test as a FIXME.
Some disucssions can be found on [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/260443-project-const-generics/topic/inline.20consts.20typeck).
cc `````@spastorino````` `````@lcnr`````
r? `````@nikomatsakis`````
`````@rustbot````` label A-inference F-inline_const T-compiler
Implementation of GATs outlives lint
See #87479 for background. Closes#87479
The basic premise of this lint/error is to require the user to write where clauses on a GAT when those bounds can be implied or proven from any function on the trait returning that GAT.
## Intuitive Explanation (Attempt) ##
Let's take this trait definition as an example:
```rust
trait Iterable {
type Item<'x>;
fn iter<'a>(&'a self) -> Self::Item<'a>;
}
```
Let's focus on the `iter` function. The first thing to realize is that we know that `Self: 'a` because of `&'a self`. If an impl wants `Self::Item` to contain any data with references, then those references must be derived from `&'a self`. Thus, they must live only as long as `'a`. Furthermore, because of the `Self: 'a` implied bound, they must live only as long as `Self`. Since it's `'a` is used in place of `'x`, it is reasonable to assume that any value of `Self::Item<'x>`, and thus `'x`, will only be able to live as long as `Self`. Therefore, we require this bound on `Item` in the trait.
As another example:
```rust
trait Deserializer<T> {
type Out<'x>;
fn deserialize<'a>(&self, input: &'a T) -> Self::Out<'a>;
}
```
The intuition is similar here, except rather than a `Self: 'a` implied bound, we have a `T: 'a` implied bound. Thus, the data on `Self::Out<'a>` is derived from `&'a T`, and thus it is reasonable to expect that the lifetime `'x` will always be less than `T`.
## Implementation Algorithm ##
* Given a GAT `<P0 as Trait<P1..Pi>>::G<Pi...Pn>` declared as `trait T<A1..Ai> for A0 { type G<Ai...An>; }` used in return type of one associated function `F`
* Given env `E` (including implied bounds) for `F`
* For each lifetime parameter `'a` in `P0...Pn`:
* For each other type parameter `Pi != 'a` in `P0...Pn`: // FIXME: this include of lifetime parameters too
* If `E => (P: 'a)`:
* Require where clause `Ai: 'a`
## Follow-up questions ##
* What should we do when we don't pass params exactly?
For this example:
```rust
trait Des {
type Out<'x, D>;
fn des<'z, T>(&self, data: &'z Wrap<T>) -> Self::Out<'z, Wrap<T>>;
}
```
Should we be requiring a `D: 'x` clause? We pass `Wrap<T>` as `D` and `'z` as `'x`, and should be able to prove that `Wrap<T>: 'z`.
r? `@nikomatsakis`
Suggest adding a new lifetime parameter when two elided lifetimes should match up but don't
Issue #90170
This also changes the tests introduced by the previous commits because of another rustc issue (#90258)
Revert "Add rustc lint, warning when iterating over hashmaps"
Fixes perf regressions introduced in https://github.com/rust-lang/rust/pull/90235 by temporarily reverting the relevant PR.
Implement coherence checks for negative trait impls
The main purpose of this PR is to be able to [move Error trait to core](https://github.com/rust-lang/project-error-handling/issues/3).
This feature is necessary to handle the following from impl on box.
```rust
impl From<&str> for Box<dyn Error> { ... }
```
Without having negative traits affect coherence moving the error trait into `core` and moving that `From` impl to `alloc` will cause the from impl to no longer compiler because of a potential future incompatibility. The compiler indicates that `&str` _could_ introduce an `Error` impl in the future, and thus prevents the `From` impl in `alloc` that would cause overlap with `From<E: Error> for Box<dyn Error>`. Adding `impl !Error for &str {}` with the negative trait coherence feature will disable this error by encoding a stability guarantee that `&str` will never implement `Error`, making the `From` impl compile.
We would have this in `alloc`:
```rust
impl From<&str> for Box<dyn Error> {} // A
impl<E> From<E> for Box<dyn Error> where E: Error {} // B
```
and this in `core`:
```rust
trait Error {}
impl !Error for &str {}
```
r? `@nikomatsakis`
This PR was built on top of `@yaahc` PR #85764.
Language team proposal: to https://github.com/rust-lang/lang-team/issues/96
Remove hir::map::blocks and use FnKind instead
The principal tool is `FnLikeNode`, which is not often used and can be easily implemented using `rustc_hir::intravisit::FnKind`.
Adopt let_else across the compiler
This performs a substitution of code following the pattern:
```
let <id> = if let <pat> = ... { identity } else { ... : ! };
```
To simplify it to:
```
let <pat> = ... { identity } else { ... : ! };
```
By adopting the `let_else` feature (cc #87335).
The PR also updates the syn crate because the currently used version of the crate doesn't support `let_else` syntax yet.
Note: Generally I'm the person who *removes* usages of unstable features from the compiler, not adds more usages of them, but in this instance I think it hopefully helps the feature get stabilized sooner and in a better state. I have written a [comment](https://github.com/rust-lang/rust/issues/87335#issuecomment-944846205) on the tracking issue about my experience and what I feel could be improved before stabilization of `let_else`.
Remove redundant member-constraint check
impl trait will, for each lifetime in the hidden type, register a "member constraint" that says the lifetime must be equal or outlive one of the lifetimes of the impl trait. These member constraints will be solved by borrowck
But, as you can see in the big red block of removed code, there was an ad-hoc check for member constraints happening at the site where they get registered. This check had some minor effects on diagnostics, but will fall down on its feet with my big type alias impl trait refactor. So we removed it and I pulled the removal out into a (hopefully) reviewable PR that works on master directly.
This performs a substitution of code following the pattern:
let <id> = if let <pat> = ... { identity } else { ... : ! };
To simplify it to:
let <pat> = ... { identity } else { ... : ! };
By adopting the let_else feature.
Show detailed expected/found types in error message when trait paths are the same
Fixes#65230.
### Issue solved by this PR
```rust
trait T {
type U;
fn f(&self) -> Self::U;
}
struct X<'a>(&'a mut i32);
impl<'a> T for X<'a> {
type U = &'a i32;
fn f(&self) -> Self::U {
self.0
}
}
fn main() {}
```
Compiler generates the following note:
```
note: ...so that the types are compatible
--> test.rs:10:28
|
10 | fn f(&self) -> Self::U {
| ____________________________^
11 | | self.0
12 | | }
| |_____^
= note: expected `T`
found `T`
```
This note is not useful since the expected type and the found type are the same.
### How this PR solve the issue
When the expected type and the found type are exactly the same in string representation, the note falls back to the detailed string representation of trait ref:
```
note: ...so that the types are compatible
--> test.rs:10:28
|
10 | fn f(&self) -> Self::U {
| ____________________________^
11 | | self.0
12 | | }
| |_____^
= note: expected `<X<'a> as T>`
found `<X<'_> as T>`
```
So that a user can notice what was different between the expected one and the found one.
Add two inline annotations for hot functions
These two functions are essentially no-ops (and compile to just a load and
return), but show up in process_obligations profiles with a high call count --
so worthwhile to try and inline them. This is not normally possible as they're
non-generic, so they don't get offered for inlining by our current algorithm.
These two functions are essentially no-ops (and compile to just a load and
return), but show up in process_obligations profiles with a high call count --
so worthwhile to try and inline them away.
This PR has several interconnected pieces:
1. In some of the NLL region error code, we now pass
around an `ObligationCause`, instead of just a plain `Span`.
This gets forwarded into `fulfill_cx.register_predicate_obligation`
during error reporting.
2. The general InferCtxt error reporting code is extended to
handle `ObligationCauseCode::BindingObligation`
3. A new enum variant `ConstraintCategory::Predicate` is added.
We try to avoid using this as the 'best blame constraint' - instead,
we use it to enhance the `ObligationCause` of the `BlameConstraint`
that we do end up choosing.
As a result, several NLL error messages now contain the same
"the lifetime requirement is introduced here" message as non-NLL
errors.
Having an `ObligationCause` available will likely prove useful
for future improvements to NLL error messages.
Be explicit about using Binder::dummy
This is somewhat of a late followup to the binder refactor PR. It removes `ToPredicate` and `ToPolyTraitImpls` that hide the use of `Binder::dummy`. While this does make code a bit more verbose, it allows us be more careful about where we create binders.
Another alternative here might be to add a new trait `ToBinder` or something with a `dummy()` fn. Which could still allow grepping but allows doing something like `trait_ref.dummy()` (but I also wonder if longer-term, it would be better to be even more explicit with a `bind_with_vars(ty::List::empty())` *but* that's not clear yet.
r? ``@nikomatsakis``
Revise never type fallback algorithm
This is a rebase of https://github.com/rust-lang/rust/pull/84573, but dropping the stabilization of never type (and the accompanying large test diff).
Each commit builds & has tests updated alongside it, and could be reviewed in a more or less standalone fashion. But it may make more sense to review the PR as a whole, I'm not sure. It should be noted that tests being updated isn't really a good indicator of final behavior -- never_type_fallback is not enabled by default in this PR, so we can't really see the full effects of the commits here.
This combines the work by Niko, which is [documented in this gist](https://gist.github.com/nikomatsakis/7a07b265dc12f5c3b3bd0422018fa660), with some additional rules largely derived to target specific known patterns that regress with the algorithm solely derived by Niko. We build these from an intuition that:
* In general, fallback to `()` is *sound* in all cases
* But, in general, we *prefer* fallback to `!` as it accepts more code, particularly that written to intentionally use `!` (e.g., Result's with a Infallible/! variant).
When evaluating Niko's proposed algorithm, we find that there are certain cases where fallback to `!` leads to compilation failures in real-world code, and fallback to `()` fixes those errors. In order to allow for stabilization, we need to fix a good portion of these patterns.
The final rule set this PR proposes is that, by default, we fallback from `?T` to `!`, with the following exceptions:
1. `?T: Foo` and `Bar::Baz = ?T` and `(): Foo`, then fallback to `()`
2. Per [Niko's algorithm](https://gist.github.com/nikomatsakis/7a07b265dc12f5c3b3bd0422018fa660#proposal-fallback-chooses-between--and--based-on-the-coercion-graph), the "live" `?T` also fallback to `()`.
The first rule is necessary to address a fairly common pattern which boils down to something like the snippet below. Without rule 1, we do not see the closure's return type as needing a () fallback, which leads to compilation failure.
```rust
#![feature(never_type_fallback)]
trait Bar { }
impl Bar for () { }
impl Bar for u32 { }
fn foo<R: Bar>(_: impl Fn() -> R) {}
fn main() {
foo(|| panic!());
}
```
r? `@jackh726`
Migrate in-tree crates to 2021
This replaces #89075 (cherry picking some of the commits from there), and closes#88637 and fixes#89074.
It excludes a migration of the library crates for now (see tidy diff) because we have some pending bugs around macro spans to fix there.
I instrumented bootstrap during the migration to make sure all crates moved from 2018 to 2021 had the compatibility warnings applied first.
Originally, the intent was to support cargo fix --edition within bootstrap, but this proved fairly difficult to pull off. We'd need to architect the check functionality to support running cargo check and cargo fix within the same x.py invocation, and only resetting sysroots on check. Further, it was found that cargo fix doesn't behave too well with "not quite workspaces", such as Clippy which has several crates. Bootstrap runs with --manifest-path ... for all the tools, and this makes cargo fix only attempt migration for that crate. We can't use e.g. --workspace due to needing to maintain sysroots for different phases of compilation appropriately.
It is recommended to skip the mass migration of Cargo.toml's to 2021 for review purposes; you can also use `git diff d6cd2c6c87 -I'^edition = .20...$'` to ignore the edition = 2018/21 lines in the diff.
This just applies the suggested fixes from the compatibility warnings,
leaving any that are in practice spurious in. This is primarily intended to
provide a starting point to identify possible fixes to the migrations (e.g., by
avoiding spurious warnings).
A secondary commit cleans these up where they are false positives (as is true in
many of the cases).
The comment seems incorrect. Testing revealed that the examples in
question still work (as well as some variants) even without the
special casing here.
We now fallback type variables using the following rules:
* Construct a coercion graph `A -> B` where `A` and `B` are unresolved
type variables or the `!` type.
* Let D be those variables that are reachable from `!`.
* Let N be those variables that are reachable from a variable not in
D.
* All variables in (D \ N) fallback to `!`.
* All variables in (D & N) fallback to `()`.
Remove concept of 'completion' from the projection cache
Fixes#88910
When we initially store a `NormalizedTy` in the projection cache,
we discard all obligations that we can (while ensuring that we
don't cause any issues with incremental compilation).
Marking a projection cache entry as 'completed' discards all
obligations associated with it. This can only cause problems,
since any obligations stored in the cache are there for a reason
(e.g. they evaluate to `EvaluatedToOkModuloRegions`).
This commit removes `complete` and `complete_normalized` entirely.
Point at argument instead of call for their obligations
When an obligation is introduced by a specific `fn` argument, point at
the argument instead of the `fn` call if the obligation fails to be
fulfilled.
Move the information about pointing at the call argument expression in
an unmet obligation span from the `FulfillmentError` to a new
`ObligationCauseCode`.
When giving an error about an obligation introduced by a function call
that an argument doesn't fulfill, and that argument is a block, add a
span_label pointing at the innermost tail expression.
Current output:
```
error[E0425]: cannot find value `x` in this scope
--> f10.rs:4:14
|
4 | Some(x * 2)
| ^ not found in this scope
error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>`
--> f10.rs:2:31
|
2 | let p = Some(45).and_then({
| ______________________--------_^
| | |
| | required by a bound introduced by this call
3 | | |x| println!("doubling {}", x);
4 | | Some(x * 2)
| | -----------
5 | | });
| |_____^ expected an `FnOnce<({integer},)>` closure, found `Option<_>`
|
= help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>`
```
Previous output:
```
error[E0425]: cannot find value `x` in this scope
--> f10.rs:4:14
|
4 | Some(x * 2)
| ^ not found in this scope
error[E0277]: expected a `FnOnce<({integer},)>` closure, found `Option<_>`
--> f10.rs:2:22
|
2 | let p = Some(45).and_then({
| ^^^^^^^^ expected an `FnOnce<({integer},)>` closure, found `Option<_>`
|
= help: the trait `FnOnce<({integer},)>` is not implemented for `Option<_>`
```
Partially address #27300. Will require rebasing on top of #88546.
Move the information about pointing at the call argument expression in
an unmet obligation span from the `FulfillmentError` to a new
`ObligationCauseCode`.
