Function Default Arguments

[assem2002]

fixes #2509
fixes #2109
completed the implementation of default arguments in functions.

Code Flow

• AST::functiondef has arguments object as one of its members, this object has m_defaults and n_defaults which preserve the number of default arguments and AST::expr_t of the default constants.
• When we're building the variables of the function in visit_functiondef( ) , we start pushing the ASR::expr_t of the defaults as a value for the variable when we reach the difference of n_args and n_defaults .
• When we start to call the function, we check if n_pos_args is less than func.n_args , if so, we start pushing the data from varibles in ASR::function_t to Vec<ASR::call_arg_t>,starting from the index n_pos_args.

The code handles keyword arguments.

serious issue to handle

The solution is build upon the idea of starting to make variable for the last number of parameters. If the user writes a code like this

def whatever(first :str ="lpython",second :str):
    print(second)

whatever("foo") # result => lpython

as the lpython would be matched with the last parameter.

This should arise a syntax error like how Cpython does.

[assem2002]

The errors arises from converting the lpython code into "c code" as c doesn't provide default arguments.
Should i change the process of converting lpython-> c , so it doesn't make a default arguments assignment?

[assem2002]

If the solution seems to be doing the job correctly. I would start handling the syntax error to make the solution complete without any flaws.
I just want to make sure it's the right approach to handle such a problem.

[Shaikh-Ubaid]

Shared couple comments. Rest looks good. Let's add some integration tests and check if the implementation works as expected.

[Shaikh-Ubaid]

If the solution seems to be doing the job correctly. I would start handling the syntax error to make the solution complete without any flaws. I just want to make sure it's the right approach to handle such a problem.

You can start another PR to support the semantic error. I suspect that the implementation might require updates in the parser.

[Shaikh-Ubaid]

Please mark it as "Ready for review" when ready.

[assem2002]

If the solution seems to be doing the job correctly. I would start handling the syntax error to make the solution complete without any flaws. I just want to make sure it's the right approach to handle such a problem.

You can start another PR to support the semantic error. I suspect that the implementation might require updates in the parser.

I think we can handle such a thing on the parser level as you mentioned.
we can handle it by using a default_argumets_started_ flag = false to throw an error if ==> default_argumets_started_ flag = true and the current argument being built doesn't have a default argument.

[Shaikh-Ubaid]

we can handle it by using a default_argumets_started_ flag = false to throw an error if ==> default_argumets_started_ flag = true and the current argument being built doesn't have a default argument.

Go ahead and submit a PR with your approach and I will take a look.

[Shaikh-Ubaid]

I suggest implementing the semantic error in a separate PR.

[assem2002]

I suggest implementing the semantic error in a separate PR.

I'll do right after handling this PR

[assem2002]

@Shaikh-Ubaid

I faced a test case which I can't say whether it's a valid test case or not
the following:

def default_func(x : str ="Hello" ,y : str = " ", z : str = "World") ->str:
    return x + y + z

default_func(z="lpython")

It results in the following error:

semantic error: default_func() got multiple values for argument 'z'
  --> ../../integration_tests/def_func_01.py:49:20
   |
49 |     test_06 :str = default_func(z="lpython")
   |                    ^^^^^^^^^^^^^^^^^^^^^^^^^ 

should I handle this in the parsing phase to prevent user form passing keyword arguments for the last parameters ?, instead the user should pass keyword arguments only if he filled some sequence in the beginning of the parameter list like that:

default_func(y = "///",x="Hi")  # result => "Hi///World"

This prevention still fits the convention that most programming languages use, which states that whenever you pass an argument to a default argument function,you can't pass it arbitrarily but you should follow the following pattern :
[passed arguments by user] + [default arguments to be handled by compiler]

and not -> [passed arguments by user] + [default arguments to be handled by compiler] + [passed arguments by user]

[Shaikh-Ubaid]

I faced a test case which I can't say whether it's a valid test case or not

If the test case runs in cpython then its a valid test case.

[assem2002]

I faced a test case which I can't say whether it's a valid test case or not

If the test case runs in cpython then its a valid test case.

Actually it passes this testcase. I will try to find a way to handle it.

[assem2002]

@Shaikh-Ubaid
I handled the test case.

def default_func(x : str ="Hello" ,y : str = " ", z : str = "World") ->str:
    return x + y + z

    test_09 :str = default_func(y = "++",z = "LPython")
    print("test_09 is =>",test_09)
    assert test_09 == "Hello++LPython"

    test_10 :str = default_func("Welcome",z = "LPython")
    print("test_10 is =>",test_10)
    assert test_10 == "Welcome LPython"

Solution Flow

• If only positional arguments are passed and aren't matching func->n_args, we start from the last argument in function_t and push back in Vec<args>, if there's no value to push then break.
• If there's keyword arguments passed, We reserve Vec<args> with func->n_args and then visit_expr_list() starts to place set all arguments to nullptr, then pushes back positional arguments, then sets keyword arguments in its desired index in Vec<args> , then we loop onVec<args> to see if there's a nullptr arg that we can have a matching default argument for.

[certik]

Thanks!

To implement default arguments, the following must be done:

• At the function definition time, find the default argument and put it into the argument's Variable symbolic_value into the symbol table
• At the function call site, simply lookup any default arguments in ASR, and if present, use them by explicitly inserting them

The alternative design is to not even insert them and sort it out inside the function, but I would not do this, since it is not explicit. I would always insert them at the call site.

[kmr-srbh]

@assem2002 could you please test for this case?

from lpython import i32

def fn(defarg1: str="default", arg: i32, defarg2: str="default") -> i32:
    return 1


fn()

This should throw an error.

[assem2002]

@assem2002 could you please test for this case?

from lpython import i32

def fn(defarg1: str="default", arg: i32, defarg2: str="default") -> i32:
    return 1


fn()

This should throw an error.

Yeah it should, but unfortunately it doesn't
I talked with Shaikh Ubaid about this, We agreed to push another pull request to handle this problem on the parsing stage not in the ASR stage.

[assem2002]

Thanks!

To implement default arguments, the following must be done:

* At the function definition time, find the default argument and put it into the argument's Variable `symbolic_value` into the symbol table

* At the function call site, simply lookup any default arguments in ASR, and if present, use them by explicitly inserting them

The alternative design is to not even insert them and sort it out inside the function, but I would not do this, since it is not explicit. I would always insert them at the call site.

Does that mean the AST of a call to a function with default arguments will include the default arguments passed even if they weren't provided by user? If so, isn't the whole AST is built then ASR?

Or what you mean is to edit the call to 'make_call_helper()' to include the default arguments?

[Shaikh-Ubaid]

The approach that Ondrej shared comprises of the following two steps:

Step 1

For default function arguments, their default value should be inserted in the functions symbol table inside the Variable declaration. Precisely, the default value should be inserted in the variable's symbolic_value and value in its declaration inside the symbol table of each function.

Step 2

For each function call, whenever we need to use the default argument value we can simply lookup the symbol table of the called function, access the symbolic_value (or value) for the variable whose default value we need and insert this default value in the function call.

Please let me know if this makes it clear.

[assem2002]

The approach that Ondrej shared comprises of the following two steps:

Step 1

For default function arguments, their default value should be inserted in the functions symbol table inside the Variable declaration. Precisely, the default value should be inserted in the variable's symbolic_value and value in its declaration inside the symbol table of each function.

Step 2

For each function call, whenever we need to use the default argument value we can simply lookup the symbol table of the called function, access the symbolic_value (or value) for the variable whose default value we need and insert this default value in the function call.

Please let me know if this makes it clear.

I got the first step.
does the second step should be similar to that? :

            (for size_t i = index_start_def_arguments; i < func-> n_args; i++){
                ASR::var_t* var  = ASR::down_cast<ASR::Var_t>(func->m_args[i]);
                std::string symbol_name = ASRUtils::symbol_name(arg_Var->m_v);
                ASR::symbol_t* sym = func->m_symtab->getsymbol(symbol_name);
                //then accessing the value, and pushing in call_args vector.
            }