Currently a `{D,Subd}iagnosticMessage` can be created from any type that
impls `Into<String>`. That includes `&str`, `String`, and `Cow<'static,
str>`, which are reasonable. It also includes `&String`, which is pretty
weird, and results in many places making unnecessary allocations for
patterns like this:
```
self.fatal(&format!(...))
```
This creates a string with `format!`, takes a reference, passes the
reference to `fatal`, which does an `into()`, which clones the
reference, doing a second allocation. Two allocations for a single
string, bleh.
This commit changes the `From` impls so that you can only create a
`{D,Subd}iagnosticMessage` from `&str`, `String`, or `Cow<'static,
str>`. This requires changing all the places that currently create one
from a `&String`. Most of these are of the `&format!(...)` form
described above; each one removes an unnecessary static `&`, plus an
allocation when executed. There are also a few places where the existing
use of `&String` was more reasonable; these now just use `clone()` at
the call site.
As well as making the code nicer and more efficient, this is a step
towards possibly using `Cow<'static, str>` in
`{D,Subd}iagnosticMessage::{Str,Eager}`. That would require changing
the `From<&'a str>` impls to `From<&'static str>`, which is doable, but
I'm not yet sure if it's worthwhile.
Switch to `EarlyBinder` for `explicit_item_bounds`
Part of the work to finish https://github.com/rust-lang/rust/issues/105779.
This PR adds `EarlyBinder` to the return type of the `explicit_item_bounds` query and removes `bound_explicit_item_bounds`.
r? `@compiler-errors` (hope it's okay to request you, since you reviewed #110299 and #110498😃)
Break up long function in trait selection error reporting + clean up nearby code
- Move blocks of code into their own functions
- Replace a few function argument types with their type aliases
- Create "AppendConstMessage" enum to replace a nested `Option`.
Clone region var origins instead of taking them in borrowck
Fixes an issue with the new solver where reporting a borrow-checker error ICEs because it calls `InferCtxt::evaluate_obligation`.
This also removes a handful of unnecessary `tcx.infer_ctxt().build()` calls that are only there to mitigate this same exact issue, but with the old solver.
Fixescompiler-errors/next-solver-hir-issues#12.
----
This implements `@aliemjay's` solution where we just don't *take* the region constraints, but clone them. This potentially makes it easier to write a bug about taking region constraints twice or never at all, but again, not many folks are touching this code.
Add `rustc_fluent_macro` to decouple fluent from `rustc_macros`
Fluent, with all the icu4x it brings in, takes quite some time to compile. `fluent_messages!` is only needed in further downstream rustc crates, but is blocking more upstream crates like `rustc_index`. By splitting it out, we allow `rustc_macros` to be compiled earlier, which speeds up `x check compiler` by about 5 seconds (and even more after the needless dependency on `serde_json` is removed from `rustc_data_structures`).
Delay a good path bug on drop for `TypeErrCtxt` (instead of a regular delayed bug)
r? `@lcnr`
Perhaps we should just delete the `Drop` impl altogether though?
Fixesrust-lang/rust-clippy#10645
`@matthiaskrgr:` I don't know how to make a clippy test for this. Any idea? Clippy's UI tests run with `-D warnings` and I have no idea how to switch it off to make a test that triggers this ICE in the clippy test suite 🤣
Fluent, with all the icu4x it brings in, takes quite some time to
compile. `fluent_messages!` is only needed in further downstream rustc
crates, but is blocking more upstream crates like `rustc_index`. By
splitting it out, we allow `rustc_macros` to be compiled earlier, which
speeds up `x check compiler` by about 5 seconds (and even more after the
needless dependency on `serde_json` is removed from
`rustc_data_structures`).
Spelling compiler
This is per https://github.com/rust-lang/rust/pull/110392#issuecomment-1510193656
I'm going to delay performing a squash because I really don't expect people to be perfectly happy w/ my changes, I really am a human and I really do make mistakes.
r? Nilstrieb
I'm going to be flying this evening, but I should be able to squash / respond to reviews w/in a day or two.
I tried to be careful about dropping changes to `tests`, afaict only two files had changes that were likely related to the changes for a given commit (this is where not having eagerly squashed should have given me an advantage), but, that said, picking things apart can be error prone.
Various minor Idx-related tweaks
Nothing particularly exciting here, but a couple of things I noticed as I was looking for more index conversions to simplify.
cc https://github.com/rust-lang/compiler-team/issues/606
r? `@WaffleLapkin`
Remove `TypeSuper{Foldable,Visitable}` impls for `Region`.
These traits exist so that folders/visitors can recurse into types of interest: binders, types, regions, predicates, and consts. But `Region` is non-recursive and cannot contain other types of interest, so its methods in these traits are trivial.
This commit inlines and removes those trivial methods.
r? `@compiler-errors`
These traits exist so that folders/visitors can recurse into types of
interest: binders, types, regions, predicates, and consts. But `Region`
is non-recursive and cannot contain other types of interest, so its
methods in these traits are trivial.
This commit inlines and removes those trivial methods.
cleanup our region error API
- require `TypeErrCtxt` to always result in an error, closing #108810
- move `resolve_regions_and_report_errors` to the `ObligationCtxt`
- call `process_registered_region_obligations` in `resolve_regions`
- move `resolve_regions` into the `outlives` submodule
- add `#[must_use]` to functions returning lists of errors
r? types
Rollup of 8 pull requests
Successful merges:
- #110153 (Fix typos in compiler)
- #110165 (rustdoc: use CSS `overscroll-behavior` instead of JavaScript)
- #110175 (Symbol cleanups)
- #110203 (Remove `..` from return type notation)
- #110205 (rustdoc: make settings radio and checks thicker, less contrast)
- #110222 (Improve the error message when forwarding a matched fragment to another macro)
- #110237 (Split out a separate feature gate for impl trait in associated types)
- #110241 (tidy: Issue an error when UI test limits are too high)
Failed merges:
- #110218 (Remove `ToRegionVid`)
r? `@ghost`
`@rustbot` modify labels: rollup
- require `TypeErrCtxt` to always result in an error
- move `resolve_regions_and_report_errors` to the `ObligationCtxt`
- merge `process_registered_region_obligations` into `resolve_regions`
It has a single call site, and the code is clearer with all region kinds
handled in one function, instead of splitting the handling across two
functions.
The commit also changes `DescriptionCtx::new` to use a more declarative
style, instead of creating a default `DescriptionCtx` and modifying it,
which I find easier to read.
Make elaboration generic over input
Combines all the `elaborate_*` family of functions into just one, which is an iterator over the same type that you pass in (e.g. elaborating `Predicate` gives `Predicate`s, elaborating `Obligation`s gives `Obligation`s, etc.)
diagnostics: account for self type when looking for source of unsolved type variable
Fixes#109905.
When searching for the source of an unsolved infer var inside of a list of generic args, we look through the `tcx.generics_of(…).own_substs(…)` which *skips* the self type if present. However, the computed `argument_index` is later[^1] used to index into `tcx.generics_of(…).params` which may still contain the self type. In such case, we are off by one when indexing into the parameters.
From now on, we account for this immediately after calling `own_substs` which keeps things local.
This also fixes the wrong output in the preexisting UI test `inference/need_type_info/concrete-impl.rs` which was overlooked. It used to claim that the *type of type parameter `Self`* couldn't be inferred in `<Struct as Ambiguous<_>>::method()` which of course isn't true: `Self` equals `Struct` here, `A` couldn't be inferred.
`@rustbot` label A-diagnostics
[^1]: f98a271814/compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs (L471)
And while doing the updates for that, also uses `FieldIdx` in `ProjectionKind::Field` and `TypeckResults::field_indices`.
There's more places that could use it (like `rustc_const_eval` and `LayoutS`), but I tried to keep this PR from exploding to *even more* places.
Part 2/? of https://github.com/rust-lang/compiler-team/issues/606
stop special-casing `'static` in evaluation
fixes#102360
I have no idea whether this actually removed all places where `'static` matters. Without canonicalization it's very easy to accidentally rely on `'static` again. Blocked on changing the `order_dependent_trait_objects` future-compat lint to a hard error
r? `@nikomatsakis`
Don't elaborate non-obligations into obligations
It's suspicious to elaborate a `PolyTraitRef` or `Predicate` into an `Obligation`, since the former does not have a param-env associated with it, but the latter does. This is a footgun that, while not being misused *currently* in the compiler, easily could be misused by someone less familiar with the elaborator's inner workings.
This PR just changes the API -- ideally, the elaborator wouldn't even have to deal with obligations if we're not elaborating obligations, but that would require a bit more abstraction than I could be bothered with today.
Rollup of 7 pull requests
Successful merges:
- #108541 (Suppress `opaque_hidden_inferred_bound` for nested RPITs)
- #109137 (resolve: Querify most cstore access methods (subset 2))
- #109380 (add `known-bug` test for unsoundness issue)
- #109462 (Make alias-eq have a relation direction (and rename it to alias-relate))
- #109475 (Simpler checked shifts in MIR building)
- #109504 (Stabilize `arc_into_inner` and `rc_into_inner`.)
- #109506 (make param bound vars visibly bound vars with -Zverbose)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Make alias-eq have a relation direction (and rename it to alias-relate)
Emitting an "alias-eq" is too strict in some situations, since we don't always want strict equality between a projection and rigid ty. Adds a relation direction.
* I could probably just reuse this [`RelationDir`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_infer/infer/combine/enum.RelationDir.html) -- happy to uplift that struct into middle and use that instead, but I didn't feel compelled to... 🤷
* Some of the matching in `compute_alias_relate_goal` is a bit verbose -- I guess I could simplify it by using [`At::relate`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_infer/infer/at/struct.At.html#method.relate) and mapping the relation-dir to a variance.
* Alternatively, I coulld simplify things by making more helper functions on `EvalCtxt` (e.g. `EvalCtxt::relate_with_direction(T, T)` that also does the nested goal registration). No preference.
r? ```@lcnr``` cc ```@BoxyUwU``` though boxy can claim it if she wants
NOTE: first commit is all the changes, the second is just renaming stuff
Make NLL Type Relating Eager
We previously instantiated bound regions in nll type relating lazily. Making this eager is more consistent with how we handle type relating in [`higher_ranked_sub`](0a3b557d52/compiler/rustc_infer/src/infer/higher_ranked/mod.rs (L28)) and should allow us to short circuit in case there's structural equality.
new solver cleanup + implement coherence
the cleanup:
- change `Certainty::unify_and` to consider ambig + overflow to be ambig
- rename `trait_candidate_should_be_dropped_in_favor_of` to `candidate_should_be_dropped_in_favor_of`
- remove outdated fixme
For coherence I mostly just add an ambiguous candidate if the current trait ref is unknowable. I am doing the same for reservation impl where I also just add an ambiguous candidate.
a general type system cleanup
removes the helper functions `traits::fully_solve_X` as they add more complexity then they are worth. It's confusing which of these helpers should be used in which context.
changes the way we deal with overflow to always add depth in `evaluate_predicates_recursively`. It may make sense to actually fully transition to not have `recursion_depth` on obligations but that's probably a bit too much for this PR.
also removes some other small - and imo unnecessary - helpers.
r? types
Rollup of 8 pull requests
Successful merges:
- #96391 (Windows: make `Command` prefer non-verbatim paths)
- #108164 (Drop all messages in bounded channel when destroying the last receiver)
- #108729 (fix: modify the condition that `resolve_imports` stops)
- #109336 (Constrain const vars to error if const types are mismatched)
- #109403 (Avoid ICE of attempt to add with overflow in emitter)
- #109415 (Refactor `handle_missing_lit`.)
- #109441 (Only implement Fn* traits for extern "Rust" safe function pointers and items)
- #109446 (Do not suggest bounds restrictions for synthesized RPITITs)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
Constrain const vars to error if const types are mismatched
When equating two consts of different types, if either are const variables, constrain them to the correct const error kind.
This helps us avoid "successfully" matching a const against an impl signature but leaving unconstrained const vars, which will lead to incremental ICEs when we call const-eval queries during const projection.
Fixes#109296
The second commit in the stack fixes a regression in the first commit where we end up mentioning `[const error]` in an impl overlap error message. I think the error message changes for the better, but I could implement alternative strategies to avoid this without delaying the overlap error message...
r? `@BoxyUwU`
Add note for mismatched types because of circular dependencies
If you have crate A with a dependency on crate B, and crate B with a dev-dependency on A, then you might see "mismatched types" errors on types that seem to be equal. This PR adds a note that explains that the types are different, because crate B is compiled twice, one time with `cfg(test)` and one time without.
I haven't found a good way to create circular dependencies in UI tests, so I abused the incremental tests instead. As a bonus, incremental tests support "cpass" now.
related to https://github.com/rust-lang/rust/issues/22750
- only borrow the refcell once per loop
- avoid complex matches to reduce branch paths in the hot loop
- use a by-ref fast path that avoids mutations at the expense of having false negatives
Install projection from RPITIT to default trait method opaque correctly
1. For new lowering strategy `-Zlower-impl-trait-in-trait-to-assoc-ty`, install the correct default trait method projection predicates (RPITIT -> opaque). This makes default trait body tests pass!
2. Fix two WF-checking bugs -- first, we want to make sure that we're always looking for an opaque type in `check_return_position_impl_trait_in_trait_bounds`. That's because the RPITIT projections are normalized to opaques during wfcheck. Second, fix RPITIT's param-envs by not adding the projection predicates that we install on trait methods to make default RPITITs work -- I left a comment why.
3. Also, just a small drive-by for `rustc_on_unimplemented`. Not sure if it affects any tests, but can't hurt.
r? ````@spastorino,```` based off of #109140
make `define_opaque_types` fully explicit
based on the idea of #108389. Moved `define_opaque_types` into the actual operations, e.g. `eq`, instead of `infcx.at` because normalization doesn't use `define_opaque_types` and even creates it's own `At` with a different `define_opaque_types` internally.
Somewhat surprisingly, coherence actually relies on `DefineOpaqueTypes::Yes` for soundness which was revealed because I've incorrectly used `DefineOpaqueTypes::No` in `equate_impl_headers`. It feels concerning that even though this is the case, we still sometimes use `DefineOpaqueTypes::No` in coherence. I did not look into this as part of this PR as it is purely changing the structure of the code without changing behavior in any way.
r? ```@oli-obk```
tidy: enforce comment blocks to have an even number of backticks
After PR #108694, most unmatched backticks in `compiler/` comments have been eliminated. This PR adds a tidy lint to ensure no new unmatched backticks are added, and either addresses the lint in the remaining instances it found, or allows it.
Very often, backtick containing sections wrap around lines, for example:
```Rust
// This function takes a tuple `(Vec<String>,
// Box<[u8]>)` and transforms it into `Vec<u8>`.
```
The lint is implemented to work on top of blocks, counting each line with a `//` into a block, and counting if there are an odd or even number of backticks in the entire block, instead of looking at just a single line.
This makes it easier to open the messages file while developing on features.
The commit was the result of automatted changes:
for p in compiler/rustc_*; do mv $p/locales/en-US.ftl $p/messages.ftl; rmdir $p/locales; done
for p in compiler/rustc_*; do sed -i "s#\.\./locales/en-US.ftl#../messages.ftl#" $p/src/lib.rs; done
Implement goal caching with the new solver
Maybe it's wrong, idk. Opening mostly for first impressions before I go to sleep.
r? ``@lcnr,`` cc ``@cjgillot``
Don't even try to combine consts with incompatible types
~I left a more detailed explanation for why this fixes this issue in the UI test, but in general, we should not try to unify const infer vars and rigid consts if they have incompatible types. That's because we don't want something like a `ConstArgHasType` predicate to suddenly go from passing to failing, or vice versa, due to a shallow resolve.~
1. Use the `type_of` for a parameter in `try_eval_lit_or_param`, instead of the "expected" type from a `WithOptConstParam` def id.
2. Don't combine consts that have incompatible types.
Fixes#108781
Emit alias-eq when equating numeric var and projection
This doesn't fix everything having to do with projections and infer vars, but it does fix a common case I saw in HIR typeck.
r? `@lcnr`
Canonicalize root var when making response from new solver
During trait solving, if we equate two inference variables `?0` and `?1` but don't equate them with any rigid types, then `InferCtxt::probe_ty_var` will return `Err` for both of these. The canonicalizer code will then canonicalize the variables independently(!), and the response will not reflect the fact that these two variables have been made equal.
This hinders inference and I also don't think it's sound? I haven't thought too much about it past that, so let's talk about it.
r? ``@lcnr``
always resolve to universal regions if possible
`RegionConstraintCollector::opportunistic_resolve_var`, which is used in canonicalization and projection logic, doesn't resolve the region var to an equal universal region. So if we have equated `'static == '1 == '2`, it doesn't resolve `'1` or `'2` to `'static`. Now it does!
Addresses review comment https://github.com/rust-lang/rust/pull/107376#discussion_r1093233687.
r? `@lcnr`
rustc_middle: Remove trait `DefIdTree`
This trait was a way to generalize over both `TyCtxt` and `Resolver`, but now `Resolver` has access to `TyCtxt`, so this trait is no longer necessary.
rustc_infer: Consolidate obligation elaboration de-duplication
# Explanation
The obligations `Elaborator` is doing de-duplication of obligations in 3 different locations. 1 off which has a comment.
This PR consolidates the functionality and comment to a single function.
(This is a large commit. The changes to
`compiler/rustc_middle/src/ty/context.rs` are the most important ones.)
The current naming scheme is a mess, with a mix of `_intern_`, `intern_`
and `mk_` prefixes, with little consistency. In particular, in many
cases it's easy to use an iterator interner when a (preferable) slice
interner is available.
The guiding principles of the new naming system:
- No `_intern_` prefixes.
- The `intern_` prefix is for internal operations.
- The `mk_` prefix is for external operations.
- For cases where there is a slice interner and an iterator interner,
the former is `mk_foo` and the latter is `mk_foo_from_iter`.
Also, `slice_interners!` and `direct_interners!` can now be `pub` or
non-`pub`, which helps enforce the internal/external operations
division.
It's not perfect, but I think it's a clear improvement.
The following lists show everything that was renamed.
slice_interners
- const_list
- mk_const_list -> mk_const_list_from_iter
- intern_const_list -> mk_const_list
- substs
- mk_substs -> mk_substs_from_iter
- intern_substs -> mk_substs
- check_substs -> check_and_mk_substs (this is a weird one)
- canonical_var_infos
- intern_canonical_var_infos -> mk_canonical_var_infos
- poly_existential_predicates
- mk_poly_existential_predicates -> mk_poly_existential_predicates_from_iter
- intern_poly_existential_predicates -> mk_poly_existential_predicates
- _intern_poly_existential_predicates -> intern_poly_existential_predicates
- predicates
- mk_predicates -> mk_predicates_from_iter
- intern_predicates -> mk_predicates
- _intern_predicates -> intern_predicates
- projs
- intern_projs -> mk_projs
- place_elems
- mk_place_elems -> mk_place_elems_from_iter
- intern_place_elems -> mk_place_elems
- bound_variable_kinds
- mk_bound_variable_kinds -> mk_bound_variable_kinds_from_iter
- intern_bound_variable_kinds -> mk_bound_variable_kinds
direct_interners
- region
- intern_region (unchanged)
- const
- mk_const_internal -> intern_const
- const_allocation
- intern_const_alloc -> mk_const_alloc
- layout
- intern_layout -> mk_layout
- adt_def
- intern_adt_def -> mk_adt_def_from_data (unusual case, hard to avoid)
- alloc_adt_def(!) -> mk_adt_def
- external_constraints
- intern_external_constraints -> mk_external_constraints
Other
- type_list
- mk_type_list -> mk_type_list_from_iter
- intern_type_list -> mk_type_list
- tup
- mk_tup -> mk_tup_from_iter
- intern_tup -> mk_tup
Use `tcx.ty_error_with_guaranteed` in more places, rename variants
1. Use `ty_error_with_guaranteed` more so we don't delay so many span bugs
2. Rename `ty_error_with_guaranteed` to `ty_error`, `ty_error` to `ty_error_misc`. This is to incentivize using the former over the latter in cases where we already are witness to a `ErrorGuaranteed` token.
Second commit is just name replacement, so the first commit can be reviewed on its own with more scrutiny.
Use associated type bounds in some places in the compiler
Use associated type bounds for some nested `impl Trait<Assoc = impl Trait2>` cases. I'm generally keen to introduce new lang features that are more mature into the compiler, but maybe let's see what others think?
Side-note: I was surprised that the only use-cases of nested impl trait in the compiler are just iterator related?!
apply query response: actually define opaque types
not sure whether this fixes any code considering that #107891 doesn't break anything, but this is currently wrong as the `eq` there should just always fail right now.
We can definitely hit this code if we remove the `replace_opaque_types_with_inference_vars` hack. Doing so without this PR causes a few tests to ICE, e.g.
bd4a96a12d/tests/ui/impl-trait/issue-99642.rs (L1-L7)
r? `@oli-obk`
Don't delay `ReError` bug during lexical region resolve
Lexical region resolution returns a list of `RegionResolutionError` which don't necessarily correspond to diagnostics being emitted. The compiler may, validly, throw away these resolution errors and do something else. Therefore it's not valid to use `ReError` during lifetime resolution, since we may actually be on a totally fine compilation path.
For example, the `implied_bounds_entailment` lint runs region resolution twice, and only emits an error if it fails both times. If we delay a bug and create a `ReError` during this first run, then we will ICE.
Fixes#108170
----
Side-note: this is conceptually equivalent to how we can't necessarily delay bugs or create `ty::Error` during trait solving/fulfillment, since the compiler is allowed to throw away these fulfillment errors to do other things. It's only once we actually emit an error (`report_region_errors` / `report_fulfillment_errors`)
Remove type-traversal trait aliases
#107924 moved the type traversal (folding and visiting) traits into the type library, but created trait aliases in `rustc_middle` to minimise both the API churn for trait consumers and the arising boilerplate. As mentioned in that PR, an alternative approach of defining subtraits with blanket implementations of the respective supertraits was also considered at that time but was ruled out as not adding much value.
Unfortunately, it has since emerged that rust-analyzer has difficulty with these trait aliases at present, resulting in a degraded contributor experience (see the recent [r-a has become useless](https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/r-a.20has.20become.20useless) topic on the #t-compiler/help Zulip stream).
This PR removes the trait aliases, and accordingly the underlying type library traits are now used directly; they are parameterised by `TyCtxt<'tcx>` rather than just the `'tcx` lifetime, and imports have been updated to reflect the fact that the trait aliases' explicitly named traits are no longer automatically brought into scope. These changes also roll-back the (no-longer required) workarounds to #107747 that were made in b409329c62.
Since this PR is just a find+replace together with the changes necessary for compilation & tidy to pass, it's currently just one mega-commit. Let me know if you'd like it broken up.
r? `@oli-obk`
Instead of loading the Fluent resources for every crate in
`rustc_error_messages`, each crate generates typed identifiers for its
own diagnostics and creates a static which are pulled together in the
`rustc_driver` crate and provided to the diagnostic emitter.
Signed-off-by: David Wood <david.wood@huawei.com>
Better debug logs for borrowck constraint graph
It's really cumbersome to work with `RegionVar`s when trying to debug borrowck code or when trying to understand how the borrowchecker works. This PR collects some region information (behind `cfg(debug_assertions)`) for created `RegionVar`s (NLL region vars, this PR doesn't touch canonicalization) and prints the nodes and edges of the strongly connected constraints graph using representatives that use that region information (either lifetime names, locations in MIR or spans).
Name placeholder in some region errors
Also don't print `ReVar` or `ReLateBound` as debug... these error messages are super uncommon anyways, but in the case they do trigger, let's be slightly more helpful.
Deny non-lifetime bound vars in `for<..> ||` closure binders
Moves the check for illegal bound var types from astconv to resolve_bound_vars. If a binder is defined to have a type or const late-bound var that's not allowed, we'll resolve any usages to ty error or const error values, so we shouldn't ever see late-bound types or consts in places they aren't expected.
Fixes#108184Fixes#108181Fixes#108192
Switch to `EarlyBinder` for `type_of` query
Part of the work to finish #105779 and implement https://github.com/rust-lang/types-team/issues/78.
Several queries `X` have a `bound_X` variant that wraps the output in `EarlyBinder`. This adds `EarlyBinder` to the return type of the `type_of` query and removes `bound_type_of`.
r? `@lcnr`
