Extend push_auto_trait_impl to built-in types #612
Conversation
|
I am not entirely sure, but it seems like we can do the same thing in chalk as we do in rustc: get a list of constituent types for a type and push a clause |
|
otherwise looks more or less what i've expected |
|
oh I missed it at first, but thats not how you spell "constituent" :P |
|
Whoops ^^' |
memoryleak47
changed the title
tests/test/auto_traits.rs
Outdated
| enum ExtEnum { GoodVariant, BadVariant(Ext) } | ||
| } | ||
|
|
||
| goal { |
There was a problem hiding this comment.
Comments on these would be nice.
Can we also get a test of a tuple that does impl the auto trait.
There was a problem hiding this comment.
Oh, Haha, that's below.
Clever, but I would feel better about splitting it up a bit.
tests/test/auto_traits.rs
Outdated
| } | ||
|
|
||
| goal { | ||
| forall<'a> { (fn(), [(); 1], [()], u32, *const (), str, !, Struct, Enum, func, good_closure, &'a ()): AutoTrait } |
chalk-solve/src/clauses.rs
Outdated
| TypeName::Adt(adt_id) => { | ||
| let adt_datum = &db.adt_datum(adt_id); | ||
| let adt_datum_bound = adt_datum.binders.substitute(interner, &app_ty.substitution); | ||
| adt_datum_bound | ||
| .variants | ||
| .into_iter() | ||
| .flat_map(|variant| variant.fields.into_iter()) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
PhantomData<T> should have T as a constituent type
jackh726
left a comment
jackh726
left a comment
There was a problem hiding this comment.
A few comments but only one small actually suggestion.
| } | ||
| }); | ||
|
|
||
| // we only compare the `TypeName`s as |
There was a problem hiding this comment.
I wonder if it's worth passing the substitution to this function, even if unused? Theoretically, if we wanted to match rustc's behavior in the integration, we could.
There was a problem hiding this comment.
I think the answer is no. Interesting thought, but let's not overcomplicate.
chalk-solve/src/clauses.rs
Outdated
| assert!(builder.placeholders_in_scope().is_empty()); | ||
|
|
||
| // auto traits are not implemented for foreign types | ||
| if let TypeName::Foreign(_) = app_ty.name { |
There was a problem hiding this comment.
Hmm, I feel like it would be simpler to make constituent_types return an Option and then have it return None for this.
chalk-solve/src/clauses.rs
Outdated
| ); | ||
| }); | ||
| // closures require binders, while the other types do not | ||
| if let TypeName::Closure(closure_id) = app_ty.name { |
There was a problem hiding this comment.
Alternatively, just move the Foreign check here.
|
@bors r+ |
|
📌 Commit c35689c has been approved by |
|
☀️ Test successful - checks-actions |
4 participants
cc #604.