trait solver: Resolve region vars in max universe#157921
Conversation
The `MaxUniverse` region visitor matched on `ReVar` terms and unwrapped `universe_of_lt`, which returns `None` for a variable that has already been unified with another region. Such resolved variables can reach the visitor while computing region assumptions under `-Zassumptions-on-binders`, causing an ICE. Resolve the variable first and inspect whatever it points at instead of assuming it is still an unresolved inference variable.
|
r? @wesleywiser rustbot has assigned @wesleywiser. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
r? BoxyUwU |
universe_of_lt on an already-resolved region var2dc4357 to
a390036
Compare
|
|
||
| fn check/*<T>*/() | ||
| where | ||
| [(); Parent::CT::<T>]:, |
There was a problem hiding this comment.
does adding the generic parameter T to check and introducing the lifetime 'a to check (and using it) cause this test to stop testing what it should? also is it possible to get a test case that doesn't depend on inherent_associated_types, the combo of mGCA and IATs is fairly brittle and likely to change significantly in the short future which will probably make this test stop testing anything 😅
| TyKind::Placeholder(p) => self.max_universe = self.max_universe.max(p.universe), | ||
| TyKind::Infer(InferTy::TyVar(inf)) => { | ||
| // Unlike `visit_region`, we don't resolve the variable first: callers | ||
| // computing assumptions bail on any non-region inference variable |
There was a problem hiding this comment.
we use MaxUniverse when rewriting constraints to a lower universe which can happen on different terms than what we compute assumptions for (though there is significant overlap) 🤔 I would expect there to possibly be cases where we have unresolved type/const infer vars but im not sure.
I would honestly probably just put a resolve_vars_if_possible call in the max_universe function just to be safe and then say we do that here, instead of trying to rely on some kind of global reasoning for why stuff must be resolved
|
thanks, that makes sense. imo putting for the test, yeah, i don't love the iat + mgca dependency either. i'll try to reduce it to something that doesn't use iats. idk yet if the same resolved-region-var shape shows up without them, but i'll check before updating the test. ltm this is the part most likely to get brittle later. |
This comment has been minimized.
This comment has been minimized.
|
i couldn't get a non-iat version to hit the same bug. the closest things i tried were trait associated consts, with both explicit and inferred lifetimes, and a gat-ish associated type. they all just reported the expected user error and never reached the old imo that's still better than landing a cleaner-looking test that doesn't cover the panic. idk if there is another way to force the same resolved-region-var constraint without iat, but i don't have one atm. also, i moved |
|
cool thanks, yeah if you can't find a test case without IATs then this is fine as is :) @bors r+ |
…ders-placeholder-ice, r=BoxyUwU trait solver: Resolve region vars in max universe Fixes rust-lang#157862 The ICE comes from computing the max universe of a region constraint after a region var has already been unified with something else. In that state, asking universe_of_lt directly can hit None and panic. This makes the region visitor opportunistically resolve ReVars first, then inspect the resolved region. Type and const infer vars stay as-is because this path already bails on non-region inference before it needs more structure from them.
…uwer Rollup of 16 pull requests Successful merges: - #156885 (Fix misattributed type inference error span for index expressions) - #157271 (simplify some `proc_macro` things) - #157883 (Remove strict invariant node_type on hir_type during ty privacy visit) - #157921 (trait solver: Resolve region vars in max universe) - #157960 (delegation: add support for infers in generics) - #158105 (Extract all instance shim variants into new `ShimKind` enum) - #158207 (Resolver: local/external split of `resolve_ident_in_module_non_globs_unadjusted` ) - #158279 (Follow goto and drop when linting unreachable code) - #157807 (don't ice on non-lifetime binders under `-Zassumptions-on-binders`) - #158020 (Update mingw-w64 C toolchain) - #158222 (format: ignore println newline in foreign format hints) - #158223 (Move target checking for #[lang] to the attribute parser) - #158252 (Use `cfg_select` in `std::os`) - #158257 ( fix escaping placeholder check in next solver normalization folder) - #158274 (triagebot: Stop pinging myself) - #158282 (slice_split_once: bounds check optimization note) Failed merges: - #158256 (Avoid parser panics bubbling out to proc macros)
…ders-placeholder-ice, r=BoxyUwU trait solver: Resolve region vars in max universe Fixes rust-lang#157862 The ICE comes from computing the max universe of a region constraint after a region var has already been unified with something else. In that state, asking universe_of_lt directly can hit None and panic. This makes the region visitor opportunistically resolve ReVars first, then inspect the resolved region. Type and const infer vars stay as-is because this path already bails on non-region inference before it needs more structure from them.
…ders-placeholder-ice, r=BoxyUwU trait solver: Resolve region vars in max universe Fixes rust-lang#157862 The ICE comes from computing the max universe of a region constraint after a region var has already been unified with something else. In that state, asking universe_of_lt directly can hit None and panic. This makes the region visitor opportunistically resolve ReVars first, then inspect the resolved region. Type and const infer vars stay as-is because this path already bails on non-region inference before it needs more structure from them.
…uwer Rollup of 23 pull requests Successful merges: - #158315 (`rust-analyzer` subtree update) - #155739 (Add temporary scope to assert_eq and assert_ne) - #156885 (Fix misattributed type inference error span for index expressions) - #157271 (simplify some `proc_macro` things) - #157883 (Remove strict invariant node_type on hir_type during ty privacy visit) - #157921 (trait solver: Resolve region vars in max universe) - #157960 (delegation: add support for infers in generics) - #157983 (Lift the same-signature restriction for `extern "tail"`) - #158105 (Extract all instance shim variants into new `ShimKind` enum) - #158207 (Resolver: local/external split of `resolve_ident_in_module_non_globs_unadjusted` ) - #158279 (Follow goto and drop when linting unreachable code) - #157527 (Move derive tests into their dedicated folder) - #157807 (don't ice on non-lifetime binders under `-Zassumptions-on-binders`) - #158020 (Update mingw-w64 C toolchain) - #158222 (format: ignore println newline in foreign format hints) - #158223 (Move target checking for #[lang] to the attribute parser) - #158252 (Use `cfg_select` in `std::os`) - #158257 ( fix escaping placeholder check in next solver normalization folder) - #158263 (Only load the feature list once in the entire resolver) - #158274 (triagebot: Stop pinging myself) - #158282 (slice_split_once: bounds check optimization note) - #158300 (Improve unknown crate_type diagnostic suggestions) - #158304 (mailmap: update mu001999) Failed merges: - #158256 (Avoid parser panics bubbling out to proc macros)
…ders-placeholder-ice, r=BoxyUwU trait solver: Resolve region vars in max universe Fixes rust-lang#157862 The ICE comes from computing the max universe of a region constraint after a region var has already been unified with something else. In that state, asking universe_of_lt directly can hit None and panic. This makes the region visitor opportunistically resolve ReVars first, then inspect the resolved region. Type and const infer vars stay as-is because this path already bails on non-region inference before it needs more structure from them.
Rollup of 27 pull requests Successful merges: - #158315 (`rust-analyzer` subtree update) - #155739 (Add temporary scope to assert_eq and assert_ne) - #156885 (Fix misattributed type inference error span for index expressions) - #157271 (simplify some `proc_macro` things) - #157883 (Remove strict invariant node_type on hir_type during ty privacy visit) - #157921 (trait solver: Resolve region vars in max universe) - #157960 (delegation: add support for infers in generics) - #157983 (Lift the same-signature restriction for `extern "tail"`) - #158053 (Optimize network address parser) - #158105 (Extract all instance shim variants into new `ShimKind` enum) - #158207 (Resolver: local/external split of `resolve_ident_in_module_non_globs_unadjusted` ) - #158279 (Follow goto and drop when linting unreachable code) - #157527 (Move derive tests into their dedicated folder) - #157807 (don't ice on non-lifetime binders under `-Zassumptions-on-binders`) - #158020 (Update mingw-w64 C toolchain) - #158039 (c-variadic: test that we use equality up to free lifetimes) - #158222 (format: ignore println newline in foreign format hints) - #158223 (Move target checking for #[lang] to the attribute parser) - #158252 (Use `cfg_select` in `std::os`) - #158257 ( fix escaping placeholder check in next solver normalization folder) - #158263 (Only load the feature list once in the entire resolver) - #158267 (FromUtf8Error::into_utf8_lossy better example and suggest use) - #158274 (triagebot: Stop pinging myself) - #158282 (slice_split_once: bounds check optimization note) - #158300 (Improve unknown crate_type diagnostic suggestions) - #158304 (mailmap: update mu001999) - #158309 (Update `rustc-literal-escaper` version to `0.0.8`) Failed merges: - #158256 (Avoid parser panics bubbling out to proc macros)
…ders-placeholder-ice, r=BoxyUwU trait solver: Resolve region vars in max universe Fixes rust-lang#157862 The ICE comes from computing the max universe of a region constraint after a region var has already been unified with something else. In that state, asking universe_of_lt directly can hit None and panic. This makes the region visitor opportunistically resolve ReVars first, then inspect the resolved region. Type and const infer vars stay as-is because this path already bails on non-region inference before it needs more structure from them.
…ders-placeholder-ice, r=BoxyUwU trait solver: Resolve region vars in max universe Fixes rust-lang#157862 The ICE comes from computing the max universe of a region constraint after a region var has already been unified with something else. In that state, asking universe_of_lt directly can hit None and panic. This makes the region visitor opportunistically resolve ReVars first, then inspect the resolved region. Type and const infer vars stay as-is because this path already bails on non-region inference before it needs more structure from them.
Fixes #157862
The ICE comes from computing the max universe of a region constraint after a region var has already been unified with something else. In that state, asking universe_of_lt directly can hit None and panic.
This makes the region visitor opportunistically resolve ReVars first, then inspect the resolved region. Type and const infer vars stay as-is because this path already bails on non-region inference before it needs more structure from them.