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`
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 `()`.
Move the information about pointing at the call argument expression in
an unmet obligation span from the `FulfillmentError` to a new
`ObligationCauseCode`.
Detect stricter constraints on gats where clauses in impls vs trait
I might try to see if I can do a bit more to improve these diagnostics, but any initial feedback is appreciated. I can also do any additional work in a followup PR.
r? `@estebank`
Path remapping: Make behavior of diagnostics output dependent on presence of --remap-path-prefix.
This PR fixes a regression (#87745) with `--remap-path-prefix` where the flag stopped causing diagnostic messages to be remapped as well. The regression was introduced in https://github.com/rust-lang/rust/pull/83813 where we erroneously assumed that remapping of diagnostic messages was not desired anymore (because #70642 partially undid that functionality with nobody objecting).
The issue is fixed by making `--remap-path-prefix` remap diagnostic messages again, including for paths that have been remapped in upstream crates (e.g. the standard library). This means that "sysroot-localization" (implemented in #70642) is also disabled if `rustc` is invoked with `--remap-path-prefix`. The assumption is that once someone starts explicitly remapping paths they also don't want paths to their local Rust installation in their build output.
In the future we might want to give more fine-grained control over this behavior via compiler flags (see https://github.com/rust-lang/rfcs/pull/3127 for a related RFC). For now this PR is intended as a regression fix.
This PR is an alternative to https://github.com/rust-lang/rust/pull/88191, which makes diagnostic messages be remapped unconditionally. That approach, however, would effectively revert #70642.
Fixes https://github.com/rust-lang/rust/issues/87745.
cc `@cbeuw`
r? `@ghost`
canonicalize consts before calling try_unify_abstract_consts query
Fixes#88022Fixes#86953Fixes#77708Fixes#82034Fixes#85031
these ICEs were all caused by calling the `try_unify_abstract_consts` query with inference vars in substs
r? `@lcnr`
I didn't like the sub-unify code executing when a predicate was
ENQUEUED, that felt fragile. I would have preferred to move the
sub-unify code so that it only occurred during generalization, but
that impacted diagnostics, so having it also occur when we process
subtype predicates felt pretty reasonable. (I guess we only need one
or the other, but I kind of prefer both, since the generalizer
ultimately feels like the *right* place to guarantee the properties we
want.)
Matthew's work on improving NLL's "higher-ranked subtype error"s
This PR rebases `@matthewjasper's` [branch](https://github.com/matthewjasper/rust/tree/nll-hrtb-errors) which has great work to fix the obscure higher-ranked subtype errors that are tracked in #57374.
These are a blocker to turning full NLLs on, and doing some internal cleanups to remove some of the old region code.
The goal is so `@nikomatsakis` can take a look at this early, and I'll then do my best to help do the changes and followup work to land this work, and move closer to turning off the migration mode.
I've only updated the branch and made it compile, removed a warning or two.
r? `@nikomatsakis`
(Here's the [zulip topic to discuss this](https://rust-lang.zulipchat.com/#narrow/stream/122657-t-compiler.2Fwg-nll/topic/.2357374.3A.20improving.20higher-ranked.20subtype.20errors.20via.20.2386700) that Niko wanted)
This allows opaque type inference to check for defining uses without having to pass down that def id via function arguments to every method that could possibly cause an opaque type to be compared with a concrete type
When this gate is enabled, explicit generic arguments can be specified even
if `impl Trait` is used in argument position. Generic arguments can only be
specified for explicit generic parameters but not for the synthetic type
parameters from `impl Trait`
When pretty printing, name placeholders as bound regions
Split from #85499
When we see a placeholder that we are going to print, treat it as a bound var (and add it to a `for<...>`
Previously, we would 'forget' that we had `'static` regions in some
place during trait evaluation. This lead to us producing
`EvaluatedToOkModuloRegions` when we could have produced
`EvaluatedToOk`, causing us to perform unnecessary work.
This PR preserves `'static` regions when we canonicalize a predicate for
`evaluate_obligation`, and when we 'freshen' a predicate during trait
evaluation. Thie ensures that evaluating a predicate containing
`'static` regions can produce `EvaluatedToOk` (assuming that we
don't end up introducing any region dependencies during evaluation).
Building off of this improved caching, we use
`predicate_must_hold_considering_regions` during fulfillment of
projection predicates to see if we can skip performing additional work.
We already do this for trait predicates, but doing this for projection
predicates lead to mixed performance results without the above caching
improvements.
avoid temporary vectors/reuse iterators
Avoid collecting an interator just to re-iterate immediately.
Rather reuse the previous iterator. (clippy::needless_collect)
Better diagnostics with mismatched types due to implicit static lifetime
Fixes#78113
I think this is my first diagnostics PR...definitely happy to hear thoughts on the direction/implementation here.
I was originally just trying to solve the error above, where the lifetime on a GAT was causing a cryptic "mismatched types" error. But as I was writing this, I realized that this (unintentionally) also applied to a different case: `wf-in-foreign-fn-decls-issue-80468.rs`. I'm not sure if this diagnostic should get a new error code, or even reuse an existing one. And, there might be some ways to make this even more generalized. Also, the error is a bit more lengthy and verbose than probably needed. So thoughts there are welcome too.
This PR essentially ended up adding a new nice region error pass that triggers if a type doesn't match the self type of an impl which is selected because of a predicate because of an implicit static bound on that self type.
r? `@estebank`
Handle C-variadic arguments properly when reporting region errors
This pull request fixes#86053. The issue is that for a C-variadic function
```rust
#![feature(c_variadic)]
unsafe extern "C" fn foo(_: (), ...) {}
```
`foo`'s signature will contain only the first parameter (and have `c_variadic` set to `true`), whereas its body has a second argument (a `hir::Pat` for the `...`).
The code for reporting region errors iterates over the body's parameters and tries to fetch the corresponding parameter from the signature; this causes an out-of-bounds ICE for the `...` (though not in the example above, because there are no region errors to report).
I have simply restricted the iteration over the body parameters to exclude `...`, which is fine because `...` cannot cause a region error.
Fix `--remap-path-prefix` not correctly remapping `rust-src` component paths and unify handling of path mapping with virtualized paths
This PR fixes#73167 ("Binaries end up containing path to the rust-src component despite `--remap-path-prefix`") by preventing real local filesystem paths from reaching compilation output if the path is supposed to be remapped.
`RealFileName::Named` introduced in #72767 is now renamed as `LocalPath`, because this variant wraps a (most likely) valid local filesystem path.
`RealFileName::Devirtualized` is renamed as `Remapped` to be used for remapped path from a real path via `--remap-path-prefix` argument, as well as real path inferred from a virtualized (during compiler bootstrapping) `/rustc/...` path. The `local_path` field is now an `Option<PathBuf>`, as it will be set to `None` before serialisation, so it never reaches any build output. Attempting to serialise a non-`None` `local_path` will cause an assertion faliure.
When a path is remapped, a `RealFileName::Remapped` variant is created. The original path is preserved in `local_path` field and the remapped path is saved in `virtual_name` field. Previously, the `local_path` is directly modified which goes against its purpose of "suitable for reading from the file system on the local host".
`rustc_span::SourceFile`'s fields `unmapped_path` (introduced by #44940) and `name_was_remapped` (introduced by #41508 when `--remap-path-prefix` feature originally added) are removed, as these two pieces of information can be inferred from the `name` field: if it's anything other than a `FileName::Real(_)`, or if it is a `FileName::Real(RealFileName::LocalPath(_))`, then clearly `name_was_remapped` would've been false and `unmapped_path` would've been `None`. If it is a `FileName::Real(RealFileName::Remapped{local_path, virtual_name})`, then `name_was_remapped` would've been true and `unmapped_path` would've been `Some(local_path)`.
cc `@eddyb` who implemented `/rustc/...` path devirtualisation
normalize mir::Constant differently from ty::Const in preparation for valtrees
Valtrees are unable to represent many kind of constant values (this is on purpose). For constants that are used at runtime, we do not need a valtree representation and can thus use a different form of evaluation. In order to make this explicit and less fragile, I added a `fold_constant` method to `TypeFolder` and implemented it for normalization. Normalization can now, when it wants to eagerly evaluate a constant, normalize `mir::Constant` directly into a `mir::ConstantKind::Val` instead of relying on the `ty::Const` evaluation.
In the future we can get rid of the `ty::Const` in there entirely and add our own `Unevaluated` variant to `mir::ConstantKind`. This would allow us to remove the `promoted` field from `ty::ConstKind::Unevaluated`, as promoteds can never occur in the type system.
cc `@rust-lang/wg-const-eval`
r? `@lcnr`
- Add back `HirIdVec`, with a comment that it will soon be used.
- Add back `*_region` functions, with a comment they may soon be used.
- Remove `-Z borrowck_stats` completely. It didn't do anything.
- Remove `make_nop` completely.
- Add back `current_loc`, which is used by an out-of-tree tool.
- Fix style nits
- Remove `AtomicCell` with `cfg(parallel_compiler)` for consistency.
Found with https://github.com/est31/warnalyzer.
Dubious changes:
- Is anyone else using rustc_apfloat? I feel weird completely deleting
x87 support.
- Maybe some of the dead code in rustc_data_structures, in case someone
wants to use it in the future?
- Don't change rustc_serialize
I plan to scrap most of the json module in the near future (see
https://github.com/rust-lang/compiler-team/issues/418) and fixing the
tests needed more work than I expected.
TODO: check if any of the comments on the deleted code should be kept.
Add function core::iter::zip
This makes it a little easier to `zip` iterators:
```rust
for (x, y) in zip(xs, ys) {}
// vs.
for (x, y) in xs.into_iter().zip(ys) {}
```
You can `zip(&mut xs, &ys)` for the conventional `iter_mut()` and
`iter()`, respectively. This can also support arbitrary nesting, where
it's easier to see the item layout than with arbitrary `zip` chains:
```rust
for ((x, y), z) in zip(zip(xs, ys), zs) {}
for (x, (y, z)) in zip(xs, zip(ys, zs)) {}
// vs.
for ((x, y), z) in xs.into_iter().zip(ys).zip(xz) {}
for (x, (y, z)) in xs.into_iter().zip((ys.into_iter().zip(xz)) {}
```
It may also format more nicely, especially when the first iterator is a
longer chain of methods -- for example:
```rust
iter::zip(
trait_ref.substs.types().skip(1),
impl_trait_ref.substs.types().skip(1),
)
// vs.
trait_ref
.substs
.types()
.skip(1)
.zip(impl_trait_ref.substs.types().skip(1))
```
This replaces the tuple-pair `IntoIterator` in #78204.
There is prior art for the utility of this in [`itertools::zip`].
[`itertools::zip`]: https://docs.rs/itertools/0.10.0/itertools/fn.zip.html
This currently creates a field which is always false on GenericParamDefKind for future use when
consts are permitted to have defaults
Update const_generics:default locations
Previously just ignored them, now actually do something about them.
Fix using type check instead of value
Add parsing
This adds all the necessary changes to lower const-generics defaults from parsing.
Change P<Expr> to AnonConst
This matches the arguments passed to instantiations of const generics, and makes it specific to
just anonymous constants.
Attempt to fix lowering bugs
Rust contains various size checks conditional on target_arch = "x86_64",
but these checks were never intended to apply to
x86_64-unknown-linux-gnux32. Add target_pointer_width = "64" to the
conditions.
Improve anonymous lifetime note to indicate the target span
Improvement for #81650
Cc #81995
Message after this improvement:
(Improve note in the middle)
```
error[E0311]: the parameter type `T` may not live long enough
--> src/main.rs:25:11
|
24 | fn play_with<T: Animal + Send>(scope: &Scope, animal: T) {
| -- help: consider adding an explicit lifetime bound...: `T: 'a +`
25 | scope.spawn(move |_| {
| ^^^^^
|
note: the parameter type `T` must be valid for the anonymous lifetime defined on the function body at 24:40...
--> src/main.rs:24:40
|
24 | fn play_with<T: Animal + Send>(scope: &Scope, animal: T) {
| ^^^^^
note: ...so that the type `[closure@src/main.rs:25:17: 27:6]` will meet its required lifetime bounds
--> src/main.rs:25:11
|
25 | scope.spawn(move |_| {
| ^^^^^
```
r? ``````@estebank``````
name async generators something more human friendly in type error diagnostic
fixes#81457
Some details:
1. I opted to load the generator kind from the hir in TyCategory. I also use 1 impl in the hir for the descr
2. I named both the source of the future, in addition to the general type (`future`), not sure what is preferred
3. I am not sure what is required to make sure "generator" is not referred to anywhere. A brief `rg "\"generator\"" showed me that most diagnostics correctly distinguish from generators and async generator, but the `descr` of `DefKind` is pretty general (not sure how thats used)
4. should the descr impl of AsyncGeneratorKind use its display impl instead of copying the string?
Ensure valid TraitRefs are created for GATs
This fixes `ProjectionTy::trait_ref` to use the correct substs. Places that need all of the substs have been updated to not use `trait_ref`.
r? ````@jackh726````
remove useless ?s (clippy::needless_question_marks)
Example code:
```rust
fn opts() -> Option<String> {
let s: Option<String> = Some(String::new());
Some(s?) // this can just be "s"
}
```
Placeholder lifetime error cleanup
- Remove note of trait definition
- Avoid repeating the same self type
- Use original region names when possible
- Use this error kind more often
- Print closure signatures when they are suppose to implement `Fn*` traits
Works towards #57374
r? ```@nikomatsakis```
Only store a LocalDefId in some HIR nodes
Some HIR nodes are guaranteed to be HIR owners: Item, TraitItem, ImplItem, ForeignItem and MacroDef.
As a consequence, we do not need to store the `HirId`'s `local_id`, and we can directly store a `LocalDefId`.
This allows to avoid a bit of the dance with `tcx.hir().local_def_id` and `tcx.hir().local_def_id_to_hir_id` mappings.
avoid full-slicing slices
If we already have a slice, there is no need to get another full-range slice from that, just use the original.
clippy::redundant_slicing
If we have a cause containing `ValuePairs::PolyTraitRefs` but neither
TraitRef has any escaping bound regions then we report the same error as
for `ValuePairs::TraitRefs`.
Improve wording of suggestion about accessing field
Follow-up to #81504
The compiler at this moment suggests "you might have meant to use field `b` of type `B`", sounding like it's type `B` which has the field `b`.
r? ```@estebank```
Upgrade Chalk
~~Blocked on rust-lang/chalk#670~~
~~Now blocked on rust-lang/chalk#680 and release~~
In addition to the straight upgrade, I also tried to fix some tests by properly returning variables and max universes in the solution. Unfortunately, this actually triggers the same perf problem that rustc traits code runs into in `canonicalizer`. Not sure what the root cause of this problem is, or why it's supposed to be solved in chalk.
r? ```@nikomatsakis```
Refractor a few more types to `rustc_type_ir`
In the continuation of #79169, ~~blocked on that PR~~.
This PR:
- moves `IntVarValue`, `FloatVarValue`, `InferTy` (and friends) and `Variance`
- creates the `IntTy`, `UintTy` and `FloatTy` enums in `rustc_type_ir`, based on their `ast` and `chalk_ir` equilavents, and uses them for types in the rest of the compiler.
~~I will split up that commit to make this easier to review and to have a better commit history.~~
EDIT: done, I split the PR in commits of 200-ish lines each
r? `````@nikomatsakis````` cc `````@jackh726`````
Separate out a `hir::Impl` struct
This makes it possible to pass the `Impl` directly to functions, instead
of having to pass each of the many fields one at a time. It also
simplifies matches in many cases.
See `rustc_save_analysis::dump_visitor::process_impl` or `rustdoc::clean::clean_impl` for a good example of how this makes `impl`s easier to work with.
r? `@petrochenkov` maybe?
This makes it possible to pass the `Impl` directly to functions, instead
of having to pass each of the many fields one at a time. It also
simplifies matches in many cases.
