Report unconstrained region in hidden types lazily#152218
Conversation
|
|
||
| type MultiUse2 = impl Sized; | ||
| #[define_opaque(MultiUse2)] | ||
| fn capture_different_universals_not_on_bounds<'a, 'b, 'c>(a: &'a i32, b: &'b i32, c: &'c i32) { |
There was a problem hiding this comment.
Just make sure that we report error for each invalid capture.
In my other attempt, only one unconstrained error per opaque definition is reported.
There was a problem hiding this comment.
hmm, I personally don't actually think this is too important and we do only report one error in HIR typeck 🤔
There was a problem hiding this comment.
please move the check-pass regression test into a separate file
There was a problem hiding this comment.
I am slightly unhappy about having both a hidden_types and unconstrained_hidden_types field but think it's a fairly clean solution.
The alternative would be to change hidden_types to an enum with a variant for unconstrained_hidden_types (either a single one or a list), but I don't think that's necessarily better 🤔
|
|
||
| type MultiUse2 = impl Sized; | ||
| #[define_opaque(MultiUse2)] | ||
| fn capture_different_universals_not_on_bounds<'a, 'b, 'c>(a: &'a i32, b: &'b i32, c: &'c i32) { |
There was a problem hiding this comment.
hmm, I personally don't actually think this is too important and we do only report one error in HIR typeck 🤔
|
|
||
| type MultiUse2 = impl Sized; | ||
| #[define_opaque(MultiUse2)] | ||
| fn capture_different_universals_not_on_bounds<'a, 'b, 'c>(a: &'a i32, b: &'b i32, c: &'c i32) { |
There was a problem hiding this comment.
please move the check-pass regression test into a separate file
| unreachable!("the body should depend on opaques type if it has opaque use"); | ||
| }; | ||
| result.deferred_opaque_type_errors.push(error); | ||
| } |
There was a problem hiding this comment.
is there a reason you're return a list of errors instead of eagerly adding them t deferred_opaque_type_errors in detect_unconstrained_hidden_types (i guess then called handle_unconstrained_hidden_types)
There was a problem hiding this comment.
also, why the def_id check here? 🤔 it's not immediately clear to me whether this assert is important. We could avoid some code if we don't check htis as the def_id is otherwise unused
There was a problem hiding this comment.
Ah, you're right. Eagerly adding the errors in detect_unconstrained_hidden_types is cleaner.
The def_id is needed because we need to insert error to the body where it originates from. The region var only makes sense in its own infcx. Yeah, I should have comments for this.
| pub tcx: TyCtxt<'tcx>, | ||
| root_def_id: LocalDefId, | ||
| hidden_types: FxIndexMap<LocalDefId, ty::DefinitionSiteHiddenType<'tcx>>, | ||
| unconstrained_hidden_types: Vec<UnexpectedHiddenRegion<'tcx>>, |
a509e96 to
03e0020
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
03e0020 to
7b1cc81
Compare
|
|
||
| // Some defining uses of opaque types can't constrain captured regions to universals. | ||
| // Previouly, we eagerly report error in this case. | ||
| // Now we report error only if there's no fully defining use from all bodies of the typeck root. |
There was a problem hiding this comment.
link the issue this is a regression test for
7b1cc81 to
10301d5
Compare
|
|
||
| #![feature(type_alias_impl_trait)] | ||
|
|
||
| struct Invar<'a>(*mut &'a ()); |
There was a problem hiding this comment.
| struct Invar<'a>(*mut &'a ()); | |
| struct Inv<'a>(*mut &'a ()); |
sorry, this is the convention we generally use and Invar is confusing to me. Please also change this test to use RPIT instead of tait
| /// We handle `UnexpectedHiddenRegion` error lazily in the next solver. | ||
| /// Hope that it can be resolved by another fully defining use, which may not exist. | ||
| /// We detect eventually unconstrained region via this function after collecting defining uses from all | ||
| /// bodies. |
There was a problem hiding this comment.
| /// We handle `UnexpectedHiddenRegion` error lazily in the next solver. | |
| /// Hope that it can be resolved by another fully defining use, which may not exist. | |
| /// We detect eventually unconstrained region via this function after collecting defining uses from all | |
| /// bodies. | |
| /// We handle `UnexpectedHiddenRegion` error lazily in the next solver as | |
| /// there may be a fully defining use in another body. | |
| /// | |
| /// In case such a defining use does not exist, we register an error here. |
10301d5 to
ae57df8
Compare
ae57df8 to
20c46d6
Compare
|
@bors r=lcnr rollup |
…-handling-keep-all-errors, r=lcnr Report unconstrained region in hidden types lazily Fixes rust-lang/trait-system-refactor-initiative#264 I didn't copy the mechanism of HIR typeck as I found that we just need to be lax in the unconstrained region case. It already ignores non-defining uses and invalid params. About tests, I'm having trouble coming up with more complex ones. 🙁 This fixes `ukanren` and `codecrafters-redis-rust` but not rust-lang#151322 and rust-lang#151323. I believe they are a [different problem](rust-lang#151322 (comment)). r? @lcnr
…-handling-keep-all-errors, r=lcnr Report unconstrained region in hidden types lazily Fixes rust-lang/trait-system-refactor-initiative#264 I didn't copy the mechanism of HIR typeck as I found that we just need to be lax in the unconstrained region case. It already ignores non-defining uses and invalid params. About tests, I'm having trouble coming up with more complex ones. 🙁 This fixes `ukanren` and `codecrafters-redis-rust` but not rust-lang#151322 and rust-lang#151323. I believe they are a [different problem](rust-lang#151322 (comment)). r? @lcnr
…uwer Rollup of 14 pull requests Successful merges: - #152323 (Fix ICE in borrowck when recovering `fn_sig` for `-> _`) - #152469 (Remove unused features) - #152515 (Extract `DepKindVTable` constructors to their own module) - #152555 (Port `#[rustc_diagnostic_item]` to the new attribute parsers) - #152218 (Report unconstrained region in hidden types lazily) - #152356 (Improve the `inline_fluent!` macro) - #152392 (Fix ICE in supertrait_vtable_slot when supertrait has missing generics) - #152407 (Add regression test for type_const with unit struct ctor under mGCA) - #152440 (Fix typos and grammar in compiler and build documentation) - #152536 (bootstrap: add explicit UTF-8 encoding to text-mode open() calls) - #152554 (Remove `deprecated_safe` and its corresponding feature gate) - #152556 (doc: move riscv64a23-unknown-linux-gnu to tier 2) - #152563 (Replace "bug" with "issue" in triagebot ping messages) - #152565 (fix missleading error for tuple ctor) Failed merges: - #152512 (core: Implement feature `float_exact_integer_constants`) - #152296 (Port `rust_nonnull_optimization_guaranteed` and `rustc_do_not_const_check` to the new attribute parser)
…uwer Rollup of 14 pull requests Successful merges: - #152323 (Fix ICE in borrowck when recovering `fn_sig` for `-> _`) - #152469 (Remove unused features) - #152515 (Extract `DepKindVTable` constructors to their own module) - #152555 (Port `#[rustc_diagnostic_item]` to the new attribute parsers) - #152218 (Report unconstrained region in hidden types lazily) - #152356 (Improve the `inline_fluent!` macro) - #152392 (Fix ICE in supertrait_vtable_slot when supertrait has missing generics) - #152407 (Add regression test for type_const with unit struct ctor under mGCA) - #152440 (Fix typos and grammar in compiler and build documentation) - #152536 (bootstrap: add explicit UTF-8 encoding to text-mode open() calls) - #152554 (Remove `deprecated_safe` and its corresponding feature gate) - #152556 (doc: move riscv64a23-unknown-linux-gnu to tier 2) - #152563 (Replace "bug" with "issue" in triagebot ping messages) - #152565 (fix missleading error for tuple ctor) Failed merges: - #152512 (core: Implement feature `float_exact_integer_constants`) - #152296 (Port `rust_nonnull_optimization_guaranteed` and `rustc_do_not_const_check` to the new attribute parser)
Rollup merge of #152218 - adwinwhite:fix-mir-borrowck-opaque-handling-keep-all-errors, r=lcnr Report unconstrained region in hidden types lazily Fixes rust-lang/trait-system-refactor-initiative#264 I didn't copy the mechanism of HIR typeck as I found that we just need to be lax in the unconstrained region case. It already ignores non-defining uses and invalid params. About tests, I'm having trouble coming up with more complex ones. 🙁 This fixes `ukanren` and `codecrafters-redis-rust` but not #151322 and #151323. I believe they are a [different problem](#151322 (comment)). r? @lcnr
Fixes rust-lang/trait-system-refactor-initiative#264
I didn't copy the mechanism of HIR typeck as I found that we just need to be lax in the unconstrained region case.
It already ignores non-defining uses and invalid params.
About tests, I'm having trouble coming up with more complex ones. 🙁
This fixes
ukanrenandcodecrafters-redis-rustbut not #151322 and #151323.I believe they are a different problem.
r? @lcnr