Deny hashing ty/re/ct inference variables
cc `@cjgillot` and https://github.com/rust-lang/rust/pull/102695#issuecomment-1275706528
r? `@lcnr`
best reviewed one commit at a time, mostly because the second commit that fixes `ClosureOutlivesRequirement` is mostly noise because of losing its `<'tcx>` lifetime parameter.
Fix missing explanation of where the borrowed reference is used when the same borrow occurs multiple times due to loop iterations
Fix#99824.
Problem of the issue:
If a borrow occurs in a loop, the borrowed reference could be invalidated at the same place at next iteration of the loop. When this happens, the point where the borrow occurs is the same as the intervening point that might invalidate the reference in the loop. This causes a problem for the current code finding the point where the resulting reference is used, so that the explanation of the cause will be missing. As the second point of "explain all errors in terms of three points" (see [leveraging intuition framing errors in terms of points"](https://rust-lang.github.io/rfcs/2094-nll.html#leveraging-intuition-framing-errors-in-terms-of-points), this explanation is very helpful for user to understand the error.
In the current implementation, the searching region for finding the location where the borrowed reference is used is limited to between the place where the borrow occurs and the place where the reference is invalidated. If those two places happen to be the same, which indicates that the borrow and invalidation occur at the same place in a loop, the search will fail.
One solution to the problem is when these two places are the same, find the terminator of the loop, and then use the location of the loop terminator instead of the location of the borrow for the region to find the place where the borrowed reference is used.
More dupe word typos
I only picked those changes (from the regex search) that I am pretty certain doesn't change meaning and is just a typo fix. Do correct me if any fix is undesirable and I can revert those. Thanks.
Use correct location for type tests in promoted constants
Previously we forgot to remap the location in a type test collected when visiting the body of a promoted constant back to the usage location, causing an ICE when trying to get span information for that type test.
Fixes https://github.com/rust-lang/rust/issues/102117
don't ICE when normalizing closure input tys
We were ICEing while rendering diagnostics because `universe_causes` is expected to track every universe created in the typeck's infcx.
`normalize_and_add_constraints` doesn't update `universe_causes`
when creating new universes, causing an ICE. Remove it!
Add spans to better track normalization constraints.
Fix couple places where `universe_causes` is not updated correctly to
track newly added universes.
Fixes#102800
~Fixess #99665~ (UPDATE: no longer true; the issue has a different failure path than when this PR was created and should be fixed by #101708, but the changes in this PR are still correct and should prevent potential future ICEs)
Remove `mir::CastKind::Misc`
As discussed in #97649 `mir::CastKind::Misc` is not clear, this PR addresses that by creating a new enum variant for every valid cast.
r? ````@oli-obk````
`normalize_and_add_constraints` doesn't add entries in `universe_causes`
when creating new universes, causing an ICE. Remove it!
Add spans to track normalization constraints.
Fix couple places where `universe_causes` is not updated correctly to
track newly added universes.
Update bootstrap compiler to 1.65.0
This PR updates the bootstrap compiler to Rust 1.65.0, removing the various `cfg(bootstrap)`s.
r? `@Mark-Simulacrum`
Neither require nor imply lifetime bounds on opaque type for well formedness
The actual hidden type can live arbitrarily longer than any individual lifetime and arbitrarily shorter than all but one of the lifetimes.
fixes#86218fixes#84305
This is a **breaking change** but it is a necessary soundness fix
Look at move place's type when suggesting mutable reborrow
Not sure why we are looking at the use site's ty instead of the move site's ty in order to suggest reborrowing the move site, but it was suppressing a perfectly valid reborrow suggestion.
r? `@estebank` who i think touched this last in 520461f1fb, though that was quite a while ago so feel free to reassign.
fix a ui test
use `into`
fix clippy ui test
fix a run-make-fulldeps test
implement `IntoQueryParam<DefId>` for `OwnerId`
use `OwnerId` for more queries
change the type of `ParentOwnerIterator::Item` to `(OwnerId, OwnerNode)`
On later stages, the feature is already stable.
Result of running:
rg -l "feature.let_else" compiler/ src/librustdoc/ library/ | xargs sed -s -i "s#\\[feature.let_else#\\[cfg_attr\\(bootstrap, feature\\(let_else\\)#"
Initial implementation of dyn*
This PR adds extremely basic and incomplete support for [dyn*](https://smallcultfollowing.com/babysteps//blog/2022/03/29/dyn-can-we-make-dyn-sized/). The goal is to get something in tree behind a flag to make collaboration easier, and also to make sure the implementation so far is not unreasonable. This PR does quite a few things:
* Introduce `dyn_star` feature flag
* Adds parsing for `dyn* Trait` types
* Defines `dyn* Trait` as a sized type
* Adds support for explicit casts, like `42usize as dyn* Debug`
* Including const evaluation of such casts
* Adds codegen for drop glue so things are cleaned up properly when a `dyn* Trait` object goes out of scope
* Adds codegen for method calls, at least for methods that take `&self`
Quite a bit is still missing, but this gives us a starting point. Note that this is never intended to become stable surface syntax for Rust, but rather `dyn*` is planned to be used as an implementation detail for async functions in dyn traits.
Joint work with `@nikomatsakis` and `@compiler-errors.`
r? `@bjorn3`
`BindingAnnotation` refactor
* `ast::BindingMode` is deleted and replaced with `hir::BindingAnnotation` (which is moved to `ast`)
* `BindingAnnotation` is changed from an enum to a tuple struct e.g. `BindingAnnotation(ByRef::No, Mutability::Mut)`
* Associated constants added for convenience `BindingAnnotation::{NONE, REF, MUT, REF_MUT}`
One goal is to make it more clear that `BindingAnnotation` merely represents syntax `ref mut` and not the actual binding mode. This was especially confusing since we had `ast::BindingMode`->`hir::BindingAnnotation`->`thir::BindingMode`.
I wish there were more symmetry between `ByRef` and `Mutability` (variant) naming (maybe `Mutable::Yes`?), and I also don't love how long the name `BindingAnnotation` is, but this seems like the best compromise. Ideas welcome.
Revert let_chains stabilization
This is the revert against master, the beta revert was already done in #100538.
Bumps the stage0 compiler which already has it reverted.
Replace `Body::basic_blocks()` with field access
Since the refactoring in #98930, it is possible to borrow the basic blocks
independently from other parts of MIR by accessing the `basic_blocks` field
directly.
Replace unnecessary `Body::basic_blocks()` method with a direct field access,
which has an additional benefit of borrowing the basic blocks only.
Suggest alternatives when trying to mutate a `HashMap`/`BTreeMap` via indexing
The error can be quite confusing to newcomers.
Fixes#100873.
I'm not so sure about the message, open to wording suggestions.
some general mir typeck cleanup
this pr contains the parts of #95763 which already work correctly.
the remaining commits of that PR have some issues which are more complex to fix.
r? types
Mention `as_mut` alongside `as_ref` in borrowck error message
Kinda fixes#99426 but I guess that really might be better staying open to see if we could make it suggest `as_mut` in a structured way. Not sure how to change borrowck to know that info tho.
Enable unused_parens for match arms
Fixes: https://github.com/rust-lang/rust/issues/92751
Currently I can't get the `stderr` to work with `./x.py test`, but this should fix the issue. Help would be appreciated!
Rollup of 6 pull requests
Successful merges:
- #98771 (Add support for link-flavor rust-lld for iOS, tvOS and watchOS)
- #98835 (relate `closure_substs.parent_substs()` to parent fn in NLL)
- #99746 (Use `TraitEngine` in more places that don't specifically need `FulfillmentContext::new_in_snapshot`)
- #99786 (Recover from C++ style `enum struct`)
- #99795 (Delay a bug when failed to normalize trait ref during specialization)
- #100029 (Prevent ICE for `doc_alias` on match arm, statement, expression)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Check that RPITs constrained by a recursive call in a closure are compatible
Fixes#99073
Adapts a similar visitor pattern to `find_opaque_ty_constraints` (that we use to check TAITs), but with some changes:
0. Only walk the "OnlyBody" children, instead of all items in the RPIT's defining scope
1. Only walk through the body's children if we found a constraining usage
2. Don't actually do any inference, just do a comparison and error if they're mismatched
----
r? `@oli-obk` -- you know all this impl-trait stuff best... is this the right approach? I can explain the underlying issue better if you'd like, in case that might reveal a better solution. Not sure if it's possible to gather up the closure's defining usages of the RPIT while borrowck'ing the outer function, that might be a better place to put this check...
Use full type name instead of just saying `impl Trait` in "captures lifetime" error
I think this is very useful, especially when there's >1 `impl Trait`, and it just means passing around a bit more info that we already have access to.
Remove the unused StableSet and StableMap types from rustc_data_structures.
The current implementation is not "stable" in the same sense that `HashStable` and `StableHasher` are stable, i.e. across compilation sessions. So, in my opinion, it's better to remove those types (which are basically unused anyway) than to give the wrong impression that these are safe for incr. comp.
I plan to provide new "stable" collection types soon that can be used to replace `FxHashMap` and `FxHashSet` in query results (see [draft](69d03ac7a7)). It's unsound that `HashMap` and `HashSet` implement `HashStable` (see https://github.com/rust-lang/rust/issues/98890 for a recent P-critical bug caused by this) -- so we should make some progress there.
Rollup of 7 pull requests
Successful merges:
- #98101 (stdlib support for Apple WatchOS)
- #99345 (Do not allow typeck children items to constrain outer RPITs)
- #99383 (Formalize defining_use_anchor)
- #99436 (Add flag to configure `noalias` on `Box<T>`)
- #99483 (Fix a numerical underflow in tuple wrap suggestion)
- #99485 (Stop injecting `#[allow(unused_qualifications)]` in generated `derive` implementations)
- #99486 (Refactor: remove a string comparison between types in `check_str_addition`)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Formalize defining_use_anchor
This tackles issue #57961
Introduces new enum called `DefiningAnchor` that replaces `Option<LocalDefId>` of `defining_use_anchor`. Now every use of it is explicit and exhaustively matched, catching errors like one in the linked issue. This is not a perfect fix but it's a step in the right direction.
r? `@oli-obk`
`arena > Rc` for query results
The `Rc`s have to live for the whole duration as their count cannot go below 1 while stored as part of the query results.
By storing them in an arena we should save a bit of memory because we don't have as many independent allocations and also don't have to clone the `Rc` anymore.
Allow destructuring opaque types in their defining scopes
fixes#96572
Before this PR, the following code snippet failed with an incomprehensible error, and similar code just ICEd in mir borrowck.
```rust
type T = impl Copy;
let foo: T = (1u32, 2u32);
let (a, b) = foo;
```
The problem was that the last line created MIR projections of the form `foo.0` and `foo.1`, but `foo`'s type is `T`, which doesn't have fields (only its hidden type does). But the pattern supplies enough type information (a tuple of two different inference types) to bind a hidden type.
We check that there's a single level of block nesting to ensure always
correct suggestions. If we don't, then we only provide a free-form
message to avoid misleading users in cases like
`src/test/ui/nll/borrowed-temporary-error.rs`.
We could expand the analysis to suggest hoising all of the relevant
parts of the users' code to make the code compile, but that could be
too much.
Implement `for<>` lifetime binder for closures
This PR implements RFC 3216 ([TI](https://github.com/rust-lang/rust/issues/97362)) and allows code like the following:
```rust
let _f = for<'a, 'b> |a: &'a A, b: &'b B| -> &'b C { b.c(a) };
// ^^^^^^^^^^^--- new!
```
cc ``@Aaron1011`` ``@cjgillot``
Implement `SourceMap::is_span_accessible`
This patch adds `SourceMap::is_span_accessible` and replaces `span_to_snippet(span).is_ok()` and `span_to_snippet(span).is_err()` with it. This removes a `&str` to `String` conversion.
promote placeholder bounds to 'static obligations
In NLL, when we are promoting a bound out from a closure, if we have a requirement that `T: 'a` where `'a` is in a higher universe, we were previously ignoring that, which is totally wrong. We should be promoting those constraints to `'static`, since universes are not expressible across closure boundaries.
Fixes#98693
~~(Marking as WIP because I'm still running tests, haven't add the new test, etc)~~
r? ``@jackh726``
When a binding is declared without a value, borrowck verifies that all
codepaths have *one* assignment to them to initialize them fully. If
there are any cases where a condition can be met that leaves the binding
uninitialized or we attempt to initialize a field of an unitialized
binding, we emit E0381.
We now look at all the statements that initialize the binding, and use
them to explore branching code paths that *don't* and point at them. If
we find *no* potential places where an assignment to the binding might
be missing, we display the spans of all the existing initializers to
provide some context.
cleanup mir visitor for `rustc::pass_by_value`
by changing `& $($mutability)?` to `$(& $mutability)?`
I also did some formatting changes because I started doing them for the visit methods I changed and then couldn't get myself to stop xx, I hope that's still fairly easy to review.
In NLL, when we are promoting a bound out from a closure,
if we have a requirement that `T: 'a` where `'a` is in a
higher universe, we were previously ignoring that, which is
totally wrong. We should be promoting those constraints to `'static`,
since universes are not expressible across closure boundaries.
translation: lint fix + more migration
- Unfortunately, the diagnostic lints are very broken and trigger much more often than they should. This PR corrects the conditional which checks if the function call being made is to a diagnostic function so that it returns in every intended case.
- The `rustc_lint_diagnostics` attribute is used by the diagnostic translation/struct migration lints to identify calls where non-translatable diagnostics or diagnostics outwith impls are being created. Any function used in creating a diagnostic should be annotated with this attribute so this PR adds the attribute to many more functions.
- Port the diagnostics from the `rustc_privacy` crate and enable the lints for that crate.
r? ``@compiler-errors``
The `rustc_lint_diagnostics` attribute is used by the diagnostic
translation/struct migration lints to identify calls where
non-translatable diagnostics or diagnostics outwith impls are being
created. Any function used in creating a diagnostic should be annotated
with this attribute so this commit adds the attribute to many more
functions.
Signed-off-by: David Wood <david.wood@huawei.com>
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.
Create elided lifetime parameters for function-like types
Split from https://github.com/rust-lang/rust/pull/97720
This PR refactor lifetime generic parameters in bare function types and parenthesized traits to introduce the additional required lifetimes as fresh parameters in a `for<>` bound.
This PR does the same to lifetimes appearing in closure signatures, and as-if introducing `for<>` bounds on closures (without the associated change in semantics).
r? `@petrochenkov`
Fix erroneous span for borrowck error
I am not confident that this is the correct fix, but it does the job. Open to suggestions for a real fix instead.
Fixes#97997
The issue is that we pass a [dummy location](https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_middle/mir/visit.rs.html#302) when type-checking the ["required consts"](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/mir/struct.Body.html#structfield.required_consts) that are needed by the MIR body during borrowck. This means that when we fail to evaluate the constant, we use the span of `bb0[0]`, instead of the actual span of the constant.
There are quite a few other places that use `START_BLOCK.start_location()`, `Location::START`, etc. when calling for a random/unspecified `Location` value. This is because, unlike (for example) `Span`, we don't have a dummy/miscellaneous value to use instead. I would appreciate guidance (either in this PR, or a follow-up) on what needs to be done to clean this up in general.
Make `ExprKind::Closure` a struct variant.
Simple refactor since we both need it to introduce additional fields in `ExprKind::Closure`.
r? ``@Aaron1011``
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.
Compute `is_late_bound_map` query separately from lifetime resolution
This query is actually very simple, and is only useful for functions and method. It can be computed directly by fetching the HIR, with no need to embed it within the lifetime resolution visitor.
Based on https://github.com/rust-lang/rust/pull/96296
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```
Add suggestion for relaxing static lifetime bounds on dyn trait impls in NLL
This PR introduces suggestions for relaxing static lifetime bounds on impls of dyn trait items for NLL similar to what is already available in lexical region diagnostics.
Fixes https://github.com/rust-lang/rust/issues/95701
r? `@estebank`
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`
Check hidden types for well formedness at the definition site instead of only at the opaque type itself
work towards #90409 . We'll need to look into closure and generator bodies of closures and generators nested inside the hidden type in order to fix that. In hindsight this PR is not necessary for that, but it may be a bit easier with it and we'll get better diagnostics from it on its own.
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
turn `append_place_to_string` from recursion into iteration
This PR fixes the FIXME in the impl of `append_place_to_string` which turns `append_place_to_string` from recursion into iteration, meanwhile simplifying the code relatively.
Remove mutable_borrow_reservation_conflict lint and allow the code pattern
This was the only breaking issue with the NLL stabilization PR. Lang team decided to go ahead and allow this.
r? `@nikomatsakis`
Closes#59159Closes#56254
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`.
Enforce Copy bounds for repeat elements while considering lifetimes
fixes https://github.com/rust-lang/rust/issues/95477
this is a breaking change in order to fix a soundness bug.
Before this PR we only checked whether the repeat element type had an `impl Copy`, but not whether that impl also had the appropriate lifetimes. E.g. if the impl was for `YourType<'static>` and not a general `'a`, then copying any type other than a `'static` one should have been rejected, but wasn't.
r? `@lcnr`
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>
Recover most `impl Trait` and `dyn Trait` lifetime bound suggestions under NLL
This is done by replacing the duplicated (and very partial) implementation from borrowck with one inspsired from `NiceRegionError::try_report_static_impl_trait` and by re-using `suggest_new_region_bound`.
Fixes#96277
r? ```@jackh726```
Improve span for `consider adding an explicit lifetime bound` suggestions under NLL
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.
This helps with #96332
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.
Make the lifetime accurate which is used in the region constraints part
This PR fixes the FIXME about lifetime using in the region constraints part.
We cannot write `<'graph, 'tcx, D>` because the definition of `Successors<'0, '1, D>` requires `'1 : '0`.
We cannot add bound to `'graph` either because `'graph` is required to be an arbitrary value in the definition of `WithSuccessors`
So the most accurate way is to use `<'s, 'tcx, D>`.
cc `@Aaron1011` who added this FIXME in #85343
Previously, we would retrieve the span from the `Body` using
the `locations` field. However, we may end up changing the
`locations` field when moving a constraint from a promoted
to a different body.
We now store the original `Span` in a dedication field, so that
changes to the `locations` do not affect the quality of our
diagnostics.
Rollup of 7 pull requests
Successful merges:
- #95566 (Avoid duplication of doc comments in `std::char` constants and functions)
- #95784 (Suggest replacing `typeof(...)` with an actual type)
- #95807 (Suggest adding a local for vector to fix borrowck errors)
- #95849 (Check for git submodules in non-git source tree.)
- #95852 (Fix missing space in lossy provenance cast lint)
- #95857 (Allow multiple derefs to be splitted in deref_separator)
- #95868 (rustdoc: Reduce allocations in a `html::markdown` function)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Note invariance reason for FnDef types
Fixes#95272. Is it worthwhile even printing a variance explanation here? Or should I try to track down which function parameter is responsible for the invariance?
r? ``@Aaron1011`` since you wrote #89336
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>
Spellchecking compiler comments
This PR cleans up the rest of the spelling mistakes in the compiler comments. This PR does not change any literal or code spelling issues.
Remember mutability in `DefKind::Static`.
This allows to compute the `BodyOwnerKind` from `DefKind` only, and
removes a direct dependency of some MIR queries onto HIR.
As a side effect, it also simplifies metadata, since we don't need 4
flavours of `EntryKind::*Static` any more.
Rollup of 5 pull requests
Successful merges:
- #95294 (Document Linux kernel handoff in std::io::copy and std::fs::copy)
- #95443 (Clarify how `src/tools/x` searches for python)
- #95452 (fix since field version for termination stabilization)
- #95460 (Spellchecking compiler code)
- #95461 (Spellchecking some comments)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
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
This allows to compute the `BodyOwnerKind` from `DefKind` only, and
removes a direct dependency of some MIR queries onto HIR.
As a side effect, it also simplifies metadata, since we don't need 4
flavours of `EntryKind::*Static` any more.
Remove `Session::one_time_diagnostic`
This is untracked mutable state, which modified the behaviour of queries.
It was used for 2 things: some full-blown errors, but mostly for lint declaration notes ("the lint level is defined here" notes).
It is replaced by the diagnostic deduplication infra which already exists in the diagnostic emitter.
A new diagnostic level `OnceNote` is introduced specifically for lint notes, to deduplicate subdiagnostics.
As a drive-by, diagnostic emission takes a `&mut` to allow dropping the `SubDiagnostic`s.
Remove redundant code from copy-suggestions
Follow up to #94375, just remove some code that is not necessary anymore. This may make the perf of such suggestions a little bit worse, but I don't think this is significant.
r? `@estebank`
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.
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.
Adt copy suggestions
Previously we've only suggested adding `Copy` bounds when the type being moved/copied is a type parameter (generic). With this PR we also suggest adding bounds when a type
- Can be copy
- All predicates that need to be satisfied for that are based on type params
i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest anything for `Option<String>`.
An example:
```rust
fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) {
(t, t)
}
```
New error (current compiler doesn't provide `help`:):
```text
error[E0382]: use of moved value: `t`
--> t.rs:2:9
|
1 | fn duplicate<T>(t: Option<T>) -> (Option<T>, Option<T>) {
| - move occurs because `t` has type `Option<T>`, which does not implement the `Copy` trait
2 | (t, t)
| - ^ value used here after move
| |
| value moved here
|
help: consider restricting type parameter `T`
|
1 | fn duplicate<T: Copy>(t: Option<T>) -> (Option<T>, Option<T>) {
| ++++++
```
Fixes#93623
r? ``````````@estebank``````````
``````````@rustbot`````````` label +A-diagnostics +A-suggestion-diagnostics +C-enhancement
----
I'm not at all sure if this is the right implementation for this kind of suggestion, but it seems to work :')
Previously we've only suggested adding `Copy` bounds when the type being
moved/copied is a type parameter (generic). With this commit we also
suggest adding bounds when a type
- Can be copy
- All predicates that need to be satisfied for that are based on type
params
i.e. we will suggest `T: Copy` for `Option<T>`, but won't suggest
anything for `Option<String>`.
Future work: it would be nice to also suggest adding `.clone()` calls
Populate liveness facts when calling `get_body_with_borrowck_facts` without `-Z polonius`
For a new feature of [Flowistry](https://github.com/willcrichton/flowistry), a static-analysis tool, we need to obtain a `mir::Body`'s liveness facts using `get_body_with_borrowck_facts` (added in #86977). We'd like to do this without passing `-Z polonius` as a compiler arg to avoid borrow checking the entire crate.
Support for doing this was added in #88983, but the Polonius input facts used for liveness analysis are empty. This happens because the liveness input facts are populated in `liveness::generate` depending only on the value of `AllFacts::enabled` (which is toggled via compiler args).
This PR propagates the [`use_polonius`](8b09ba6a5d/compiler/rustc_borrowck/src/nll.rs (L168)) flag to `liveness::generate` to support populating liveness facts without requiring the `-Z polonius` flag.
This fix is somewhat patchy - if it'd be better to add more widely-accessible state (like `AllFacts::enabled`) I'd be open to ideas!
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.
Introduce `ChunkedBitSet` and use it for some dataflow analyses.
This reduces peak memory usage significantly for some programs with very
large functions.
r? `@ghost`
This reduces peak memory usage significantly for some programs with very
large functions, such as:
- `keccak`, `unicode_normalization`, and `match-stress-enum`, from
the `rustc-perf` benchmark suite;
- `http-0.2.6` from crates.io.
The new type is used in the analyses where the bitsets can get huge
(e.g. 10s of thousands of bits): `MaybeInitializedPlaces`,
`MaybeUninitializedPlaces`, and `EverInitializedPlaces`.
Some refactoring was required in `rustc_mir_dataflow`. All existing
analysis domains are either `BitSet` or a trivial wrapper around
`BitSet`, and access in a few places is done via `Borrow<BitSet>` or
`BorrowMut<BitSet>`. Now that some of these domains are `ClusterBitSet`,
that no longer works. So this commit replaces the `Borrow`/`BorrowMut`
usage with a new trait `BitSetExt` containing the needed bitset
operations. The impls just forward these to the underlying bitset type.
This required fiddling with trait bounds in a few places.
The commit also:
- Moves `static_assert_size` from `rustc_data_structures` to
`rustc_index` so it can be used in the latter; the former now
re-exports it so existing users are unaffected.
- Factors out some common "clear excess bits in the final word"
functionality in `bit_set.rs`.
- Uses `fill` in a few places instead of loops.
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.
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.
Inherit lifetimes for async fn instead of duplicating them.
The current desugaring of `async fn foo<'a>(&usize) -> &u8` is equivalent to
```rust
fn foo<'a, '0>(&'0 usize) -> foo<'static, 'static>::Opaque<'a, '0, '_>;
type foo<'_a, '_0>::Opaque<'a, '0, '1> = impl Future<Output = &'1 u8>;
```
following the RPIT model.
Duplicating all the inherited lifetime parameters and setting the inherited version to `'static` makes lowering more complex and causes issues like #61949. This PR removes the duplication of inherited lifetimes to directly use
```rust
fn foo<'a, '0>(&'0 usize) -> foo<'a, '0>::Opaque<'_>;
type foo<'a, '0>::Opaque<'1> = impl Future<Output = &'1 u8>;
```
following the TAIT model.
Fixes https://github.com/rust-lang/rust/issues/61949
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).
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
