Point out the known type when field doesn't satisfy bound
For file
```rust
use std::path::Path;
fn f(p: Path) { }
```
provide the following error
```nocode
error[E0277]: the trait bound `[u8]: std::marker::Sized` is not satisfied in `std::path::Path`
--> file.rs:3:6
|
3 | fn f(p: Path) { }
| ^ within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]`
|
= note: `[u8]` does not have a constant size known at compile-time
= note: required because it appears within the type `std::path::Path`
= note: all local variables must have a statically known size
```
Fix#23286.
erase lifetimes when translating specialized substs
Projections can generate lifetime variables with equality constraints,
that will not be resolved by `resolve_type_vars_if_possible`, so substs
need to be lifetime-erased after that.
Fixes#36848.
For file
```rust
use std::path::Path;
fn f(p: Path) { }
```
provide the following error
```nocode
error[E0277]: the trait bound `[u8]: std::marker::Sized` is not satisfied in `std::path::Path`
--> file.rs:3:6
|
3 | fn f(p: Path) { }
| ^ within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]`
|
= note: `[u8]` does not have a constant size known at compile-time
= note: required because it appears within the type `std::path::Path`
= note: all local variables must have a statically known size
```
Projections can generate lifetime variables with equality constraints,
that will not be resolved by `resolve_type_vars_if_possible`, so substs
need to be lifetime-erased after that.
Fixes#36848.
evaluate obligations in LIFO order during closure projection
This is an annoying gotcha with the projection cache's handling of
nested obligations.
Nested projection obligations enter the issue in this case:
```
DEBUG:rustc::traits::project: AssociatedTypeNormalizer: depth=3
normalized
<std::iter::Map<std::ops::Range<i32>,
[closure@not-a-recursion-error.rs:5:30: 5:53]> as
std::iter::IntoIterator>::Item to _#7t with 12 add'l obligations
```
Here the normalization result is the result of the nested impl
`<[closure@not-a-recursion-error.rs:5:30: 5:53] as FnMut(i32)>::Output`,
which is an additional obligation that is a part of "add'l obligations".
By itself, this is proper behaviour - the additional obligation is
returned, and the RFC 447 rules ensure that it is processed before the
output `#_7t` is used in any way.
However, the projection cache breaks this - it caches the
`<std::iter::Map<std::ops::Range<i32>,[closure@not-a-recursion-error.rs:5:30:
5:53]> as std::iter::IntoIterator>::Item = #_7t` resolution. Now
everybody else that attempts to look up the projection will just get
`#_7t` *without* any additional obligations. This obviously causes all
sorts of trouble (here a spurious `EvaluatedToAmbig` results in
specializations not being discarded
[here](9ca50bd4d5/src/librustc/traits/select.rs (L1705))).
The compiler works even with this projection cache gotcha because in most
cases during "one-pass evaluation". we tend to process obligations in LIFO
order - after an obligation is added to the cache, we process its nested
obligations before we do anything else (and if we have a cycle, we handle
it specifically) - which makes sure the inference variables are resolved
before they are used.
That "LIFO" order That was not done when projecting out of a closure, so
let's just fix that for the time being.
Fixes#38033.
Beta-nominating because regression.
r? @nikomatsakis
[9/n] rustc: move type information out of AdtDef and TraitDef.
_This is part of a series ([prev](https://github.com/rust-lang/rust/pull/37688) | [next]()) of patches designed to rework rustc into an out-of-order on-demand pipeline model for both better feature support (e.g. [MIR-based](https://github.com/solson/miri) early constant evaluation) and incremental execution of compiler passes (e.g. type-checking), with beneficial consequences to IDE support as well.
If any motivation is unclear, please ask for additional PR description clarifications or code comments._
<hr>
Both `AdtDef` and `TraitDef` contained type information (field types, generics and predicates) which was required to create them, preventing their use before that type information exists, or in the case of field types, *mutation* was required, leading to a variance-magicking implementation of `ivar`s.
This PR takes that information out and the resulting cleaner setup could even eventually end up merged with HIR, because, just like `AssociatedItem` before it, there's no dependency on types anymore.
(With one exception, variant discriminants should probably be moved into their own map later.)
This is an annoying gotcha with the projection cache's handling of
nested obligations.
Nested projection obligations enter the issue in this case:
```
DEBUG:rustc::traits::project: AssociatedTypeNormalizer: depth=3
normalized
<std::iter::Map<std::ops::Range<i32>,
[closure@not-a-recursion-error.rs:5:30: 5:53]> as
std::iter::IntoIterator>::Item to _#7t with 12 add'l obligations
```
Here the normalization result is the result of the nested impl
`<[closure@not-a-recursion-error.rs:5:30: 5:53] as FnMut(i32)>::Output`,
which is an additional obligation that is a part of "add'l obligations".
By itself, this is proper behaviour - the additional obligation is
returned, and the RFC 447 rules ensure that it is processed before the
output `#_7t` is used in any way.
However, the projection cache breaks this - it caches the
`<std::iter::Map<std::ops::Range<i32>,[closure@not-a-recursion-error.rs:5:30:
5:53]> as std::iter::IntoIterator>::Item = #_7t` resolution. Now
everybody else that attempts to look up the projection will just get
`#_7t` *without* any additional obligations. This obviously causes all
sorts of trouble (here a spurious `EvaluatedToAmbig` results in
specializations not being discarded
[here](9ca50bd4d5/src/librustc/traits/select.rs (L1705))).
The compiler works even with this projection cache gotcha because in most
cases during "one-pass evaluation". we tend to process obligations in LIFO
order - after an obligation is added to the cache, we process its nested
obligations before we do anything else (and if we have a cycle, we handle
it specifically) - which makes sure the inference variables are resolved
before they are used.
That "LIFO" order That was not done when projecting out of a closure, so
let's just fix that for the time being.
Fixes#38033.
Separate impl items from the parent impl
This change separates impl item bodies out of the impl itself. This gives incremental more resolution. In so doing, it refactors how the visitors work, and cleans up a bit of the collect/check logic (mostly by moving things out of collect that didn't really belong there, because they were just checking conditions).
However, this is not as effective as I expected, for a kind of frustrating reason. In particular, when invoking `foo.bar()` you still wind up with dependencies on private items. The problem is that the method resolution code scans that list for methods with the name `bar` -- and this winds up touching *all* the methods, even private ones.
I can imagine two obvious ways to fix this:
- separating fn bodies from fn sigs (#35078, currently being pursued by @flodiebold)
- a more aggressive model of incremental that @michaelwoerister has been advocating, in which we hash the intermediate results (e.g., the outputs of collect) so that we can see that the intermediate result hasn't changed, even if a particular impl item has changed.
So all in all I'm not quite sure whether to land this or not. =) It still seems like it has to be a win in some cases, but not with the test cases we have just now. I can try to gin up some test cases, but I'm not sure if they will be totally realistic. On the other hand, some of the early refactorings to the visitor trait seem worthwhile to me regardless.
cc #36349 -- well, this is basically a fix for that issue, I guess
r? @michaelwoerister
NB: Based atop of @eddyb's PR https://github.com/rust-lang/rust/pull/37402; don't land until that lands.
Before, when we created an AssociatedItem for impl item X, we would read
the impl item itself. Now we instead load up the impl I that contains X
and read the data from the `ImplItemRef` for X; actually, we do it for
all impl items in I pre-emptively.
This kills the last source of edges between a method X and a call to a
method Y defined in the same impl.
Fixes#37121
In general having all these different structs for "origins" is not
great, since equating types can cause obligations and vice-versa. I
think we should gradually collapse these things. We almost certainly
also need to invest a big more energy into the `error_reporting` code to
rationalize it: this PR does kind of the minimal effort in that
direction.
detect extra region requirements in impls
The current "compare method" check fails to check for the "region obligations" that accrue in the fulfillment context. This branch switches that code to create a `FnCtxt` so that it can invoke the regionck code. Previous crater runs (I haven't done one with the latest tip) have found some small number of affected crates, so I went ahead and introduced a warning cycle. I will kick off a crater run with this branch shortly.
This is a [breaking-change] because previously unsound code was accepted. The crater runs also revealed some cases where legitimate code was no longer type-checking, so the branch contains one additional (but orthogonal) change. It improves the elaborator so that we elaborate region requirements more thoroughly. In particular, if we know that `&'a T: 'b`, we now deduce that `T: 'b` and `'a: 'b`.
I invested a certain amount of effort in getting a good error message. The error message looks like this:
```
error[E0276]: impl has stricter requirements than trait
--> traits-elaborate-projection-region.rs:33:5
|
21 | fn foo() where T: 'a;
| --------------------- definition of `foo` from trait
...
33 | fn foo() where U: 'a { }
| ^^^^^^^^^^^^^^^^^^^^^^^^ impl has extra requirement `U: 'a`
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #18937 <https://github.com/rust-lang/rust/issues/18937>
note: lint level defined here
--> traits-elaborate-projection-region.rs:12:9
|
12 | #![deny(extra_requirement_in_impl)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^
```
Obviously the warning only prints if this is a _new_ error (that resulted from the bugfix). But all existing errors that fit this description are updated to follow the general template. In order to get the lint to preserve the span-labels and the error code, I separate out the core `Diagnostic` type (which encapsulates the error code, message, span, and children) from the `DiagnosticBuilder` (which layers on a `Handler` that can be used to report errors). I also extended `add_lint` with an alternative `add_lint_diagnostic` that takes in a full diagnostic (cc @jonathandturner for those changes). This doesn't feel ideal but feels like it's moving in the right direction =).
r? @pnkfelix
cc @arielb1
Fixes#18937
This seems better because I want to avoid the situation where unresolved
inference variables make it into the environment. On the other hand, I
am not 100% sure that this is correct. My assumption was that the WF
check should ensure that this normalization can succeed. But it occurs
to me that the WF checks may need to make use of the `specializes`
predicate themselves, and hence we may have a kind of cycle here (this
is a bigger problem with spec in any case that we need to resolve).
On the other hand, this should just cause extra errors I think, so it
seems like a safe thing to attempt. Certainly all tests pass.
- correct indentation
- rename `from_cause` to `from_obligation_cause`
- break up `compare_impl_method` into fns
- delete some blank lines and correct comment
Most of the Rust community agrees that the vec! macro is clearer when
called using square brackets [] instead of regular brackets (). Most of
these ocurrences are from before macros allowed using different types of
brackets.
There is one left unchanged in a pretty-print test, as the pretty
printer still wants it to have regular brackets.
Add ArrayVec and AccumulateVec to reduce heap allocations during interning of slices
Updates `mk_tup`, `mk_type_list`, and `mk_substs` to allow interning directly from iterators. The previous PR, #37220, changed some of the calls to pass a borrowed slice from `Vec` instead of directly passing the iterator, and these changes further optimize that to avoid the allocation entirely.
This change yields 50% less malloc calls in [some cases](https://pastebin.mozilla.org/8921686). It also yields decent, though not amazing, performance improvements:
```
futures-rs-test 4.091s vs 4.021s --> 1.017x faster (variance: 1.004x, 1.004x)
helloworld 0.219s vs 0.220s --> 0.993x faster (variance: 1.010x, 1.018x)
html5ever-2016- 3.805s vs 3.736s --> 1.018x faster (variance: 1.003x, 1.009x)
hyper.0.5.0 4.609s vs 4.571s --> 1.008x faster (variance: 1.015x, 1.017x)
inflate-0.1.0 3.864s vs 3.883s --> 0.995x faster (variance: 1.232x, 1.005x)
issue-32062-equ 0.309s vs 0.299s --> 1.033x faster (variance: 1.014x, 1.003x)
issue-32278-big 1.614s vs 1.594s --> 1.013x faster (variance: 1.007x, 1.004x)
jld-day15-parse 1.390s vs 1.326s --> 1.049x faster (variance: 1.006x, 1.009x)
piston-image-0. 10.930s vs 10.675s --> 1.024x faster (variance: 1.006x, 1.010x)
reddit-stress 2.302s vs 2.261s --> 1.019x faster (variance: 1.010x, 1.026x)
regex.0.1.30 2.250s vs 2.240s --> 1.005x faster (variance: 1.087x, 1.011x)
rust-encoding-0 1.895s vs 1.887s --> 1.005x faster (variance: 1.005x, 1.018x)
syntex-0.42.2 29.045s vs 28.663s --> 1.013x faster (variance: 1.004x, 1.006x)
syntex-0.42.2-i 13.925s vs 13.868s --> 1.004x faster (variance: 1.022x, 1.007x)
```
We implement a small-size optimized vector, intended to be used primarily for collection of presumed to be short iterators. This vector cannot be "upsized/reallocated" into a heap-allocated vector, since that would require (slow) branching logic, but during the initial collection from an iterator heap-allocation is possible.
We make the new `AccumulateVec` and `ArrayVec` generic over implementors of the `Array` trait, of which there is currently one, `[T; 8]`. In the future, this is likely to expand to other values of N.
Huge thanks to @nnethercote for collecting the performance and other statistics mentioned above.
Disallow Unsized Enums
Fixes#16812.
This PR is a potential fix for #16812, an issue which is reported [again](https://github.com/rust-lang/rust/issues/36801) and [again](https://github.com/rust-lang/rust/issues/36975), with over a dozen duplicates by now.
This PR is mainly meant to promoted discussion about the issue and the correct way to fix it.
This is a [breaking-change] since the error is now reported during wfchecking, so that even the definition of a (potentially) unsized enum will cause an error (whereas it would previously cause an ICE at trans time if the enum was used in an unsized manner).
