added early check to eliminate candidates from solution set

[TomerStarkware]

No description provided.

[reviewable-StarkWare]

This change is 

[orizi]

Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 245 at r1 (raw file):

        impl_type_bounds: Arc<BTreeMap<ImplTypeById, TypeId>>,
    ) -> InferenceResult<CandidateSolver> {
        if !can_conform_generic_args(

can you add an initial check with:

if candidate.concrete_trait(db).is_fully_concrete(db) &&
    canonical_trait.id.is_var_free(db) {
   if candidate.concrete_trait(db) != canonical_trait.id {
       return Err(super::ErrorSet);
   }
}

Code quote:

        if !can_conform_generic_args(

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 361 at r1 (raw file):

        can_conform_generic_arg(*garg_candidate, *garg1, impl_type_bounds, db)
    })
}

Suggestion:

/// Checks if the generic arguments of the candidate could be conformed to the generic args of the trait.
fn can_conform_generic_args(
    db: &dyn SemanticGroup,
    candidate_args: &[GenericArgumentId],
    target_args: &[GenericArgumentId],
    impl_type_bounds: &Arc<BTreeMap<ImplTypeById, TypeId>>,
) -> bool {
    zip_eq(candidate_args, target_args).all(|(candidate_arg, target_arg)| {
        can_conform_generic_arg(*candidate_arg, *target_arg, impl_type_bounds, db)
    })
}

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 397 at r1 (raw file):

        }
    }
}

Suggestion:

    db: &dyn SemanticGroup,
    candidate_arg: GenericArgumentId,
    target_arg: GenericArgumentId,
    impl_type_bounds: &Arc<BTreeMap<ImplTypeById, TypeId>>,
) -> bool {
    if candidate_arg == target_arg {
        return true;
    }
    match (candidate_arg, target_arg) {
        (GenericArgumentId::Type(candidate), GenericArgumentId::Type(target)) => {
            can_conform_ty(db, candidate, target, impl_type_bounds)
        }
        (GenericArgumentId::Constant(candidate), GenericArgumentId::Constant(target)) => {
            can_conform_const(db, candidate, target, impl_type_bounds)
        }
        (GenericArgumentId::Impl(candidate), GenericArgumentId::Impl(target)) => {
            can_conform_impl(db, candidate, target, impl_type_bounds)
        }
        (GenericArgumentId::NegImpl, GenericArgumentId::NegImpl) => true,
        _ => false,
    }
}

[TomerStarkware]

Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @orizi)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 245 at r1 (raw file):

Previously, orizi wrote…

can you add an initial check with:

if candidate.concrete_trait(db).is_fully_concrete(db) &&
    canonical_trait.id.is_var_free(db) {
   if candidate.concrete_trait(db) != canonical_trait.id {
       return Err(super::ErrorSet);
   }
}

Done.

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 361 at r1 (raw file):

        can_conform_generic_arg(*garg_candidate, *garg1, impl_type_bounds, db)
    })
}

Done.

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 397 at r1 (raw file):

        }
    }
}

Done.

[orizi]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion

[orizi]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 378 at r3 (raw file):

        return true;
    }
    if candidate_arg.is_fully_concrete(db) && target_arg.is_fully_concrete(db) {

Suggestion:

is_var_free

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 514 at r3 (raw file):

// TODO(Tomerstarkware) add consts to early candidate eliminiation
fn can_conform_const(

doc.
also - isn't the implementation here just as simple?

[orizi]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 279 at r5 (raw file):

                }
            }
        }

Suggestion:

        // Describe what you actually do here.
        if let UninferredImpl::GenericParam(_) = candidate {
            for (trait_arg, candidate_arg) in zip_eq(
                &canonical_trait.id.generic_args(db),
                &candidate_concrete_trait.generic_args(db),
            )  {
                if let (GenericArgumentId::Type(ty0), GenericArgumentId::Type(ty1)) =
                    (trait_arg, candidate_arg)
                {
                    if ty0.is_var_free(db) && ty1.is_var_free(db) && *ty0 != ty1 {
                        return Err(super::ErrorSet);
                    }
                }
            }
        }

[TomerStarkware]

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @orizi)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 514 at r3 (raw file):

Previously, orizi wrote…

doc.
also - isn't the implementation here just as simple?

Done.

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 279 at r5 (raw file):

                }
            }
        }

Done.

[orizi]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 261 at r6 (raw file):

        // If the candidate is a generic param, we do not substitute the generic types, so if the
        // types are not equal so the candidate can be eliminated.

i don't understand.

Code quote:

        // If the candidate is a generic param, we do not substitute the generic types, so if the
        // types are not equal so the candidate can be eliminated.

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 270 at r6 (raw file):

                    (trait_arg, candidate_arg)
                {
                    if ty0.is_var_free(db) && ty1.is_var_free(db) && *ty0 != *ty1 {

isn't the candidate always var free?

Code quote:

ty1.is_var_free(db) 

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 416 at r6 (raw file):

            let target_args = target_concrete.generic_args(db);
            can_conform_generic_args(db, &candidate_args, &target_args)
        }

consider:
(and similarly elsewhere)

Suggestion:

        (TypeLongId::Concrete(candidate), TypeLongId::Concrete(target)) => {
            candidate.generic_type(db) == target.generic_type(db) 
            && can_conform_generic_args(db, &candidate.generic_args(db), &target.generic_args(db))
            
        }

[TomerStarkware]

Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @orizi)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 261 at r6 (raw file):

Previously, orizi wrote…

i don't understand.

removed

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 270 at r6 (raw file):

Previously, orizi wrote…

isn't the candidate always var free?

I am not sure what happens with impl types

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 416 at r6 (raw file):

Previously, orizi wrote…

consider:
(and similarly elsewhere)

Done.

[orizi]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 259 at r7 (raw file):

        ) {
            return Err(super::ErrorSet);
        }

Suggestion:

        let candidate_concrete_trait = candidate.concrete_trait(db).unwrap();
        if !can_conform_generic_args(
            db,
            (&candidate_concrete_trait.generic_args(db),
             matches!(candidate, UninferredImpl::GenericParam(_)) || candidate_concrete_trait.is_fully_concrete(db),
            (&canonical_trait.id.generic_args(db), canonical_trait.id.is_var_free(db)),
        ) {
            return Err(super::ErrorSet);
        }

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 348 at r7 (raw file):

    zip_eq(candidate_args, target_args).all(|(candidate_arg, target_arg)| {
        can_conform_generic_arg(db, *candidate_arg, *target_arg, substitute_generic_params)
    })

Suggestion:

    db: &dyn SemanticGroup,
    (candidate_args, candidate_final): (&[GenericArgumentId], bool),
    (target_args, target_final): (&[GenericArgumentId], bool),
) -> bool {
    zip_eq(candidate_args, target_args).all(|(candidate_arg, target_arg)| {
        can_conform_generic_arg(db, (*candidate_arg, candidate_final), (*target_arg, target_final), substitute_generic_params)
    })

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 377 at r7 (raw file):

        (GenericArgumentId::NegImpl, GenericArgumentId::NegImpl) => true,
        _ => false,
    }

Suggestion:

fn can_conform_generic_arg(
    db: &dyn SemanticGroup,
    (candidate_arg, mut candidate_final): (GenericArgumentId, bool),
    (target_arg, mut target_final): (GenericArgumentId, bool),
) -> bool {
    if candidate_arg == target_arg {
        return true;
    }
    candidate_final = candidate_final || candidate_arg.is_fully_concrete(db);
    target_final = target_final || target_arg.is_var_free(db);
    if candidate_final && target_final {
        return false;
    }
    match (candidate_arg, target_arg) {
        (GenericArgumentId::Type(candidate), GenericArgumentId::Type(target)) => {
            can_conform_ty(db, (candidate, candidate_final), (target, target_final))
        }
        (GenericArgumentId::Constant(candidate), GenericArgumentId::Constant(target)) => {
            can_conform_const(db, (candidate, candidate_final), (target, target_final))
        }
        (GenericArgumentId::Impl(candidate), GenericArgumentId::Impl(target)) => {
            can_conform_impl(db, (candidate, candidate_final), (target, target_final))
        }
        (GenericArgumentId::NegImpl, GenericArgumentId::NegImpl) => true,
        _ => false,
    }

[TomerStarkware]

Reviewable status: 0 of 1 files reviewed, 3 unresolved discussions (waiting on @orizi)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 259 at r7 (raw file):

        ) {
            return Err(super::ErrorSet);
        }

Why remove the first part

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 348 at r7 (raw file):

    zip_eq(candidate_args, target_args).all(|(candidate_arg, target_arg)| {
        can_conform_generic_arg(db, *candidate_arg, *target_arg, substitute_generic_params)
    })

Done.

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 377 at r7 (raw file):

        (GenericArgumentId::NegImpl, GenericArgumentId::NegImpl) => true,
        _ => false,
    }

Done.

[orizi]

Reviewed 1 of 1 files at r8.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 259 at r7 (raw file):

Previously, TomerStarkware wrote…

Why remove the first part

New suggestion instead.

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 262 at r8 (raw file):

        ) {
            return Err(super::ErrorSet);
        }

Suggestion:

        let candidate_concrete_trait = candidate.concrete_trait(db).unwrap();
        let candidate_final = matches!(candidate, UninferredImpl::GenericParam(_))
         || candidate_concrete_trait.is_fully_concrete(db);
        let target_final = canonical_trait.id.is_var_free(db);
        if candidate_final && target_final {
            if candidate_concrete_trait != canonical_trait.id {
                return Err(super::ErrorSet);
            }
        } else !can_conform_generic_args(
            db,
            (&candidate_concrete_trait.generic_args(db), candidate_final),
            (&canonical_trait.id.generic_args(db), target_final),
        ) {
            return Err(super::ErrorSet);
        }

[orizi]

Reviewed 1 of 1 files at r9, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware)

[orizi]

Reviewed all commit messages.
Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 247 at r10 (raw file):

        let candidate_concrete_trait = candidate.concrete_trait(db).unwrap();
        let candidate_final = matches!(candidate, UninferredImpl::GenericParam(_))
            && candidate_concrete_trait.is_var_free(db)

explain why in a comment.

Code quote:

            && candidate_concrete_trait.is_var_free(db)

[TomerStarkware]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @orizi)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 247 at r10 (raw file):

Previously, orizi wrote…

explain why in a comment.

Done.

[orizi]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 247 at r10 (raw file):

Previously, TomerStarkware wrote…

Done.

what is "a generic that is var_free"? - that is the point that was unclear.

[TomerStarkware]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @orizi)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 247 at r10 (raw file):

Previously, orizi wrote…

what is "a generic that is var_free"? - that is the point that was unclear.

a generic imp param is usually var_free, the exceptions are

Code snippet:

<+MyTrait,+Copy<MyTrait::Inner>

[orizi]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 247 at r10 (raw file):

Previously, TomerStarkware wrote…

a generic imp param is usually var_free, the exceptions are

this still seems var free - at least in expression context.

[TomerStarkware]

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @orizi)

---

crates/cairo-lang-semantic/src/expr/inference/solver.rs line 247 at r10 (raw file):

Previously, orizi wrote…

this still seems var free - at least in expression context.

If there are Impl type bounds, we need inference to determine the associated type

[orizi]

Reviewed 1 of 1 files at r12, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @TomerStarkware)