inferring closures types within higher order functions

[dean-starkware]

No description provided.

[reviewable-StarkWare]

This change is 

[orizi]

Reviewed all commit messages.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1051 at r1 (raw file):

                );
                let id = ctx.arenas.exprs.alloc(expr.clone());
                (ExprAndId { expr, id }, None)

Suggestion:

                (ExprAndId { expr, id }, Some(arg_named.name(syntax_db))

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1058 at r1 (raw file):

                )
            }
        }

extract function.

Suggestion:

        ast::ArgClause::Unnamed(arg_unnamed) => {
            (the_extracted_func(ctx, arg_unnamed.value(syntax_db)), None)
        }
        ast::ArgClause::Named(arg_named) => {
            (the_extracted_func(ctx, arg_named.value(syntax_db)), Some(arg_named.name(syntax_db)))
        }

---

crates/cairo-lang-semantic/src/items/functions.rs line 128 at r1 (raw file):

    }

    pub fn get_closure_params(

doc.

[orizi]

Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @TomerStarkware)

---

a discussion (no related file):
if there are already instances of this in the corelib iterator implementations - remove the explicit typing.

[TomerStarkware]

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @dean-starkware and @gilbens-starkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 981 at r1 (raw file):

            // Normal parameters
            let mut named_args = vec![];
            let ConcreteFunction { .. } = function.lookup_intern(db).function;

This does nothing

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2819 at r1 (raw file):

    let mut named_args = vec![NamedArg(fixed_lexpr, None, mutability)];
    // Other arguments.
    let ConcreteFunction { .. } = function_id.lookup_intern(ctx.db).function;

...

---

crates/cairo-lang-semantic/src/items/functions.rs line 128 at r1 (raw file):

    }

    pub fn get_closure_params(

Add todo! to salsa the query

---

crates/cairo-lang-semantic/src/items/functions.rs line 918 at r1 (raw file):

}

/// Query implementation of [crate::db::SemanticGroup::concrete_function_closure_params].

add the salsa method and call this function from db

[TomerStarkware]

Reviewed all commit messages.
Reviewable status: 0 of 33 files reviewed, 9 unresolved discussions (waiting on @dean-starkware and @gilbens-starkware)

---

a discussion (no related file):
something went wrong with the commits

[dean-starkware]

Reviewable status: 0 of 33 files reviewed, 9 unresolved discussions (waiting on @gilbens-starkware and @TomerStarkware)

---

a discussion (no related file):

Previously, TomerStarkware wrote…

something went wrong with the commits

yes, on it

[TomerStarkware]

Reviewed 1 of 33 files at r2, 27 of 31 files at r3, 6 of 6 files at r4, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @dean-starkware and @gilbens-starkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1713 at r4 (raw file):

        params.iter().filter(|param| param.mutability == Mutability::Reference).for_each(|param| {
            new_ctx.diagnostics.report(param.stable_ptr(ctx.db.upcast()), RefClosureParam);

why is it in this pr? why is it duplicated?

[dean-starkware]

Reviewable status: 31 of 35 files reviewed, 9 unresolved discussions (waiting on @gilbens-starkware, @orizi, and @TomerStarkware)

---

a discussion (no related file):

Previously, orizi wrote…

if there are already instances of this in the corelib iterator implementations - remove the explicit typing.

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 981 at r1 (raw file):

Previously, TomerStarkware wrote…

This does nothing

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1058 at r1 (raw file):

Previously, orizi wrote…

extract function.

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2819 at r1 (raw file):

Previously, TomerStarkware wrote…

...

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1713 at r4 (raw file):

Previously, TomerStarkware wrote…

why is it in this pr? why is it duplicated?

Done.

---

crates/cairo-lang-semantic/src/items/functions.rs line 128 at r1 (raw file):

Previously, orizi wrote…

doc.

Done.

---

crates/cairo-lang-semantic/src/items/functions.rs line 128 at r1 (raw file):

Previously, TomerStarkware wrote…

Add todo! to salsa the query

Done.

---

crates/cairo-lang-semantic/src/items/functions.rs line 918 at r1 (raw file):

Previously, TomerStarkware wrote…

add the salsa method and call this function from db

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1051 at r1 (raw file):

                );
                let id = ctx.arenas.exprs.alloc(expr.clone());
                (ExprAndId { expr, id }, None)

Done.

[orizi]

Reviewed 1 of 4 files at r5, all commit messages.
Reviewable status: 32 of 35 files reviewed, 9 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/db.rs line 1435 at r5 (raw file):

    /// Returns a `HashMap` where the key is the closure type, and the value is a
    /// vector of parameter types.

wrong doc.

Code quote:

    /// Returns a `HashMap` where the key is the closure type, and the value is a
    /// vector of parameter types.

---

crates/cairo-lang-semantic/src/db.rs line 1443 at r5 (raw file):

    /// Returns a `HashMap` where the key is the closure type, and the value is a
    /// vector of parameter types.

wrong doc.

Code quote:

    /// Returns a `HashMap` where the key is the closure type, and the value is a
    /// vector of parameter types.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1037 at r5 (raw file):

            let arg_expr = arg_named.value(syntax_db);
            if let ast::Expr::Closure(expr_closure) = arg_expr {
                (handle_closure_expr(ctx, &expr_closure, closure_param_types), None)

why is it none in the case of named closure?
and if it shouldn't be, handle the external match in the same extracted function.

Code quote:

(handle_closure_expr(ctx, &expr_closure, closure_param_types), None)

[dean-starkware]

Reviewable status: 32 of 35 files reviewed, 9 unresolved discussions (waiting on @gilbens-starkware, @orizi, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/db.rs line 1435 at r5 (raw file):

Previously, orizi wrote…

wrong doc.

Done.

---

crates/cairo-lang-semantic/src/db.rs line 1443 at r5 (raw file):

Previously, orizi wrote…

wrong doc.

Is it? Added "generic parameters".

[orizi]

Reviewed 1 of 33 files at r2, 1 of 1 files at r6, all commit messages.
Reviewable status: 33 of 35 files reviewed, 8 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/db.rs line 1443 at r5 (raw file):

Previously, dean-starkware wrote…

Is it? Added "generic parameters".

no vector is mapped.

have a similar do to the above function.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1059 at r6 (raw file):

}

fn handle_closure_expr(

doc

[dean-starkware]

Reviewable status: 32 of 35 files reviewed, 8 unresolved discussions (waiting on @gilbens-starkware, @orizi, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1037 at r5 (raw file):

Previously, orizi wrote…

why is it none in the case of named closure?
and if it shouldn't be, handle the external match in the same extracted function.

added the arg_named.name instead of None.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1059 at r6 (raw file):

Previously, orizi wrote…

doc

Done.

[orizi]

Reviewed 1 of 2 files at r7, all commit messages.
Reviewable status: 33 of 35 files reviewed, 6 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1037 at r5 (raw file):

Previously, dean-starkware wrote…

added the arg_named.name instead of None.

So why not handle both in function?

[TomerStarkware]

Reviewed 1 of 2 files at r7, all commit messages.
Reviewable status: 34 of 35 files reviewed, 6 unresolved discussions (waiting on @dean-starkware and @gilbens-starkware)

---

crates/cairo-lang-semantic/src/items/functions.rs line 151 at r7 (raw file):

                let concrete_id =
                    ConcreteTraitGenericFunctionId::new(db, concrete_trait_id, id.function);
                let substitution = GenericSubstitution::from_impl(id.impl_id);

this substitution is now redundant in specialize_function in resolve/mod.rs
remove it there

---

crates/cairo-lang-semantic/src/items/functions.rs line 1036 at r7 (raw file):

/// returns a `HashMap` where the key is the closure type, and the value is a
/// vector of parameter types.
pub fn get_closure_params(

add documentation that this is a salsa query

---

crates/cairo-lang-semantic/src/items/functions.rs line 1054 at r7 (raw file):

                    ] = *concrete_trait.generic_args(db)
                    else {
                        unreachable!()

add a reason for why its unreachable

[dean-starkware]

Reviewable status: 34 of 35 files reviewed, 6 unresolved discussions (waiting on @gilbens-starkware, @orizi, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1037 at r5 (raw file):

Previously, orizi wrote…

So why not handle both in function?

Done.
It caused some changes in expr numbers in some test files.
I'm not sure why or what it means.
Please take a look and give me your thoughts.

---

crates/cairo-lang-semantic/src/items/functions.rs line 151 at r7 (raw file):

Previously, TomerStarkware wrote…

this substitution is now redundant in specialize_function in resolve/mod.rs
remove it there

Removing it would make the specialize_function completely redundant.

---

crates/cairo-lang-semantic/src/items/functions.rs line 1036 at r7 (raw file):

Previously, TomerStarkware wrote…

add documentation that this is a salsa query

Done.

---

crates/cairo-lang-semantic/src/items/functions.rs line 1054 at r7 (raw file):

Previously, TomerStarkware wrote…

add a reason for why its unreachable

Done.

[TomerStarkware]

Reviewed 9 of 10 files at r8, all commit messages.
Reviewable status: 42 of 43 files reviewed, 7 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @orizi)

---

crates/cairo-lang-semantic/src/items/functions.rs line 1054 at r7 (raw file):

Previously, dean-starkware wrote…

Done.

remove the comments, the argument for unreachable is enough

---

crates/cairo-lang-semantic/src/items/functions.rs line 1036 at r8 (raw file):

/// returns a `HashMap` where the key is the closure type, and the value is a
/// vector of parameter types.
/// This is a salsa query.

Suggestion:

/// Query implementation of [crate::db::SemanticGroup::get_closure_params].

[orizi]

Reviewed 4 of 10 files at r8, all commit messages.
Reviewable status: 42 of 43 files reviewed, 8 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1034 at r8 (raw file):

            closure_param_types,
            Some(arg_named.name(syntax_db)),
        ),

the last param should not be a part of the function call.
and rename to be more exact.

Suggestion:

        ast::ArgClause::Unnamed(arg_unnamed) => {
            (handle_possible_closure_expr(ctx, &arg_unnamed.value(syntax_db), closure_param_types), None)
        }
        ast::ArgClause::Named(arg_named) => (
            handle_possible_closure_expr(ctx, &arg_named.value(syntax_db), closure_param_types),
            Some(arg_named.name(syntax_db)),
        ),

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1067 at r8 (raw file):

        let expr = compute_expr_semantic(ctx, expr);
        let expr = wrap_maybe_with_missing(ctx, Ok(expr.expr.clone()), expr.stable_ptr());
        let id = ctx.arenas.exprs.alloc(expr.clone());

there wasn't an allocation here before - this is probably the cause for the id change.

Code quote:

        let expr = wrap_maybe_with_missing(ctx, Ok(expr.expr.clone()), expr.stable_ptr());
        let id = ctx.arenas.exprs.alloc(expr.clone());

[dean-starkware]

Reviewable status: 42 of 43 files reviewed, 8 unresolved discussions (waiting on @gilbens-starkware, @orizi, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1034 at r8 (raw file):

Previously, orizi wrote…

the last param should not be a part of the function call.
and rename to be more exact.

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1067 at r8 (raw file):

Previously, orizi wrote…

there wasn't an allocation here before - this is probably the cause for the id change.

Yes this is indeed the cause for the change.
It seems uncessary though- as I cannot make it work without it.
Is it posiible to keep it and the followed changes?

---

crates/cairo-lang-semantic/src/items/functions.rs line 1054 at r7 (raw file):

Previously, TomerStarkware wrote…

remove the comments, the argument for unreachable is enough

Done.

---

crates/cairo-lang-semantic/src/items/functions.rs line 1036 at r8 (raw file):

/// returns a `HashMap` where the key is the closure type, and the value is a
/// vector of parameter types.
/// This is a salsa query.

Done.

[orizi]

Reviewed all commit messages.
Reviewable status: 41 of 43 files reviewed, 7 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1034 at r8 (raw file):

Previously, dean-starkware wrote…

Done.

not all - note the fact that i remove the identifier param from the function - as there's no need to pass it through it at all.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1067 at r8 (raw file):

Previously, dean-starkware wrote…

Yes this is indeed the cause for the change.
It seems uncessary though- as I cannot make it work without it.
Is it posiible to keep it and the followed changes?

what happens if you don't add these?
couldn't you have the else be just:
compute_expr_semantic(ctx, expr)
?

[dean-starkware]

Reviewable status: 41 of 43 files reviewed, 7 unresolved discussions (waiting on @gilbens-starkware, @orizi, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1067 at r8 (raw file):

Previously, orizi wrote…

what happens if you don't add these?
couldn't you have the else be just:
compute_expr_semantic(ctx, expr)
?

Done.

[orizi]

Reviewed all commit messages.
Reviewable status: 3 of 43 files reviewed, 10 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/items/functions.rs line 151 at r7 (raw file):

Previously, dean-starkware wrote…

Removing it would make the specialize_function completely redundant.

if so - it would be a good time to completely remove it.

---

crates/cairo-lang-semantic/src/items/functions.rs line 1036 at r8 (raw file):

Previously, dean-starkware wrote…

Done.

move the rest of the doc - to the query declaration.
(if already there - just remove here)

---

crates/cairo-lang-semantic/src/expr/compute.rs line 990 at r10 (raw file):

                    ctx,
                    arg_syntax,
                    closure_params.get(&ty).cloned(),

Suggestion:

                    closure_params.get(&ty).copied(),

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1014 at r10 (raw file):

    ctx: &mut ComputationContext<'_>,
    arg_syntax: ast::Arg,
    closure_param_types: Option<TypeId>,

Suggestion:

    closure_params_tuple_ty: Option<TypeId>,

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1673 at r10 (raw file):

    ctx: &mut ComputationContext<'_>,
    syntax: &ast::ExprClosure,
    param_types: Option<TypeId>,

Suggestion:

    params_tuple_ty: Option<TypeId>,

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2880 at r10 (raw file):

            ctx,
            arg_syntax,
            closure_params.get(&ty).cloned(),

Suggestion:

            closure_params.get(&ty).copied(),

[dean-starkware]

Reviewable status: 3 of 43 files reviewed, 10 unresolved discussions (waiting on @gilbens-starkware, @orizi, and @TomerStarkware)

---

crates/cairo-lang-semantic/src/items/functions.rs line 151 at r7 (raw file):

Previously, orizi wrote…

if so - it would be a good time to completely remove it.

@TomerStarkware- any objections before I proceed with this?

---

crates/cairo-lang-semantic/src/items/functions.rs line 1036 at r8 (raw file):

Previously, orizi wrote…

move the rest of the doc - to the query declaration.
(if already there - just remove here)

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 990 at r10 (raw file):

                    ctx,
                    arg_syntax,
                    closure_params.get(&ty).cloned(),

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1014 at r10 (raw file):

    ctx: &mut ComputationContext<'_>,
    arg_syntax: ast::Arg,
    closure_param_types: Option<TypeId>,

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 1673 at r10 (raw file):

    ctx: &mut ComputationContext<'_>,
    syntax: &ast::ExprClosure,
    param_types: Option<TypeId>,

Done.

---

crates/cairo-lang-semantic/src/expr/compute.rs line 2880 at r10 (raw file):

            ctx,
            arg_syntax,
            closure_params.get(&ty).cloned(),

Done.

[orizi]

Reviewed 38 of 39 files at r10, 1 of 2 files at r11.
Reviewable status: 3 of 43 files reviewed, 6 unresolved discussions (waiting on @gilbens-starkware and @TomerStarkware)

[TomerStarkware]

Reviewed 1 of 2 files at r9.
Reviewable status: 3 of 43 files reviewed, 2 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @orizi)

---

crates/cairo-lang-semantic/src/items/functions.rs line 151 at r7 (raw file):

Previously, dean-starkware wrote…

@TomerStarkware- any objections before I proceed with this?

its still calling resolve_generic_args

[orizi]

Reviewed 1 of 2 files at r11, 39 of 39 files at r12, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dean-starkware, @gilbens-starkware, and @TomerStarkware)

[TomerStarkware]

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dean-starkware, @gilbens-starkware, and @orizi)

[orizi]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @dean-starkware and @gilbens-starkware)