Avoid rustdoc auto-trait ICEs with the global next solver#158346
Avoid rustdoc auto-trait ICEs with the global next solver#158346Jamesbarford wants to merge 2 commits into
Conversation
8806252 to
52e368f
Compare
|
|
||
| let (infcx, orig_env) = tcx.infer_ctxt().build_with_typing_env(typing_env); | ||
| let mut selcx = SelectionContext::new(&infcx); | ||
| for polarity in [ty::PredicatePolarity::Positive, ty::PredicatePolarity::Negative] { |
There was a problem hiding this comment.
can you instead copy the behavior of
here.rn this doesn't consider there to be an explicit impl for MyType<T> if there's just impl Send for MyType<u32>
| let field_clauses = adt_def | ||
| .all_fields() | ||
| .map(|field| field.ty(tcx, args).skip_norm_wip()) | ||
| .filter(|field_ty| field_ty.has_non_region_param()) |
There was a problem hiding this comment.
why this?
| let full_user_env = ty::ParamEnv::new( | ||
| tcx.mk_clauses_from_iter(orig_env.caller_bounds().iter().chain(field_clauses)), | ||
| ); |
There was a problem hiding this comment.
hmm? 🤔 what are you trying to do/please provide a comment
There was a problem hiding this comment.
generally, can you describe the algorithm you're trying to implement in words?
This is currently somewhat wrong and I would like to know what it is that you are trying to do. Might also be useful to show the intended behavior on one or two examples
There was a problem hiding this comment.
It is meant to be deliberately coarse and simply not ICE. Not a faithful port of the old algorithm. If I'm not mistaken the old path minimises the where-clauses by repeatedly selecting and folding unimplemented param-bounds back into the ParamEnv using the old-solver internals which ICEs under the new solver.
The new path just does one rather simple pass:
- If there is a user-written impl then
ExplicitImpl. - If
tyisn't anADTNoImpl. - Otherwise assume each generic field implements the trait (
field_ty: Traitfor every field mentioning a type/const param) and add those as the where-clauses. - Verify
ty: Traitunder those assumptions- true error
NegativeImpl - ambiguity
NoImpl - clean
PositiveImpl
- true error
Example:
// foo.rs
pub struct Foo<T> {
pub data: Box<T>,
}
// previously
impl<T> Send for Foo<T>
where
T: Send,
// This PR
impl<T> Send for Foo<T>
where
Box<T>: Send,So I'm emitting the raw field bound rather than minimising to T: Send. But I understand that this is not what we are wanting to do? Instead I should be following fn disqualify_auto_trait_candidate_due_to_possible_impl(...)?
There was a problem hiding this comment.
That makes sense 👍
Treating ambiguity as NoImpl should be wrong though 🤔
struct Foo<'a, 'b, T: 'a + 'b>(&'a T, &'b T);proving Foo<'a, 'b, T>: Send given &'a T: Send and &'b T: Send results in an ambiguity error.
I think what step 4 is trying to handle is types which don't implement auto traits because of one of the non-generic fields 🤔
what we can do here is to instead replace all generic parameters with inference variables and then check whether Foo<'?0, '?1, ?t>: Send holds. If this returns an error, we emit a NegativeImpl. We never emit NoImpl for now
I have some ideas on how to refine this in the future, but I think as an initial impl this is good enough :>
The way to replace generic params with infer vars is e.g.
rust/compiler/rustc_hir_typeck/src/demand.rs
Lines 981 to 983 in c397dae
and then you take the Adt type, wrap it in an EarlyBinder and instantiate it
…t-bounds, r=fmease Fix: auto trait, const trait bound Split from rust-lang#158346. I was fiddling about with the code to understand the behaviour of the snippet below, got confused, and somehow fixed something 🤷 ```rust #![feature(const_trait_impl)] pub struct Outer<T>(Inner<T>); struct Inner<T>(T); impl<T> Unpin for Inner<T> where T: const std::default::Default, {} ``` Fixes rust-lang#149281
…t-bounds, r=fmease Fix: auto trait, const trait bound Split from rust-lang#158346. I was fiddling about with the code to understand the behaviour of the snippet below, got confused, and somehow fixed something 🤷 ```rust #![feature(const_trait_impl)] pub struct Outer<T>(Inner<T>); struct Inner<T>(T); impl<T> Unpin for Inner<T> where T: const std::default::Default, {} ``` Fixes rust-lang#149281
…t-bounds, r=fmease Fix: auto trait, const trait bound Split from rust-lang#158346. I was fiddling about with the code to understand the behaviour of the snippet below, got confused, and somehow fixed something 🤷 ```rust #![feature(const_trait_impl)] pub struct Outer<T>(Inner<T>); struct Inner<T>(T); impl<T> Unpin for Inner<T> where T: const std::default::Default, {} ``` Fixes rust-lang#149281
…t-bounds, r=fmease Fix: auto trait, const trait bound Split from rust-lang#158346. I was fiddling about with the code to understand the behaviour of the snippet below, got confused, and somehow fixed something 🤷 ```rust #![feature(const_trait_impl)] pub struct Outer<T>(Inner<T>); struct Inner<T>(T); impl<T> Unpin for Inner<T> where T: const std::default::Default, {} ``` Fixes rust-lang#149281
Rollup merge of #158390 - Jamesbarford:fix/trait-solver-const-bounds, r=fmease Fix: auto trait, const trait bound Split from #158346. I was fiddling about with the code to understand the behaviour of the snippet below, got confused, and somehow fixed something 🤷 ```rust #![feature(const_trait_impl)] pub struct Outer<T>(Inner<T>); struct Inner<T>(T); impl<T> Unpin for Inner<T> where T: const std::default::Default, {} ``` Fixes #149281
-Znext-solver=globallydoes not ICE.r? @lcnr