Address review comments

Author: Aaron1011

chalk-solve/src/clauses.rs

@@ -358,18 +358,18 @@ fn program_clauses_that_could_match<I: Interner>(
 
                 let trait_datum = db.trait_datum(trait_id);
 
+                let self_ty = alias.self_type_parameter(interner);
+
                 // Flounder if the self-type is unknown and the trait is non-enumerable.
                 //
                 // e.g., Normalize(<?X as Iterator>::Item = u32)
-                if (alias.self_type_parameter(interner).is_var(interner))
-                    && trait_datum.is_non_enumerable_trait()
-                {
+                if (self_ty.is_var(interner)) && trait_datum.is_non_enumerable_trait() {
                     return Err(Floundered);
                 }
 
                 if let Some(well_known) = trait_datum.well_known {
                     builtin_traits::add_builtin_assoc_program_clauses(
-                        db, builder, well_known, proj,
+                        db, builder, well_known, self_ty,
                     );
                 }
 

chalk-solve/src/clauses/builtin_traits.rs

@@ -1,6 +1,6 @@
 use super::{builder::ClauseBuilder, generalize};
 use crate::{Interner, RustIrDatabase, TraitRef, WellKnownTrait};
-use chalk_ir::{AliasTy, ProjectionTy, Substitution, Ty};
+use chalk_ir::{Substitution, Ty};
 
 mod clone;
 mod copy;
@@ -48,12 +48,10 @@ pub fn add_builtin_assoc_program_clauses<I: Interner>(
     db: &dyn RustIrDatabase<I>,
     builder: &mut ClauseBuilder<'_, I>,
     well_known: WellKnownTrait,
-    proj: &ProjectionTy<I>,
+    self_ty: Ty<I>,
 ) {
     match well_known {
         WellKnownTrait::FnOnce => {
-            let interner = db.interner();
-            let self_ty = AliasTy::Projection(proj.clone()).self_type_parameter(interner);
             let trait_id = db.well_known_trait_id(well_known).unwrap();
             fn_family::add_fn_trait_program_clauses(db, builder, trait_id, self_ty);
         }

chalk-solve/src/clauses/builtin_traits/fn_family.rs

@@ -8,11 +8,14 @@ use chalk_ir::{
 };
 
 /// Handles clauses for FnOnce/FnMut/Fn.
-/// If `assoc_output` is `true`, we push a clause of the form
-/// `Normalize(<fn(A) -> B as FnOnce<(A,)>>::Output -> B) :- Implemented(fn(A) -> B as FnOnce<(A,)>`
+/// If `self_ty` is a function, we push a clause of the form
+/// `fn(A1, A2, ..., AN) -> O: FnTrait<(A1, A2, ..., AN)>`, where `FnTrait`
+/// is the trait corresponding to `trait_id` (FnOnce/FnMut/Fn)
 ///
-/// If `assoc_output` is `false`, we push a clause of the form
-/// `Implemented(fn(A) -> B as FnOnce<(A,)>)`
+/// If `trait_id` is `FnOnce`, we also push a clause for the output type of the form:
+/// `Normalize(<fn(A) -> B as FnOnce<(A,)>>::Output -> B)`
+/// We do not add the usual `Implemented(fn(A) -> b as FnOnce<(A,)>` clause
+/// as a condition, since we already called `push_fact` with it
 pub fn add_fn_trait_program_clauses<I: Interner>(
     db: &dyn RustIrDatabase<I>,
     builder: &mut ClauseBuilder<'_, I>,
@@ -25,10 +28,10 @@ pub fn add_fn_trait_program_clauses<I: Interner>(
             let (binders, orig_sub) = fn_val.into_binders_and_value(interner);
             let bound_ref = Binders::new(VariableKinds::from(interner, binders), orig_sub);
             builder.push_binders(&bound_ref, |builder, orig_sub| {
-                let all_params: Vec<_> = orig_sub.iter(interner).cloned().collect();
-
                 // The last parameter represents the function return type
-                let (arg_sub, fn_output_ty) = all_params.split_at(all_params.len() - 1);
+                let (arg_sub, fn_output_ty) = orig_sub
+                    .parameters(interner)
+                    .split_at(orig_sub.len(interner) - 1);
                 let arg_sub = Substitution::from(interner, arg_sub);
                 let fn_output_ty = fn_output_ty[0].assert_ty_ref(interner);