Optional args

[swamishiju]

Fixed optional args for LP

Removed C test for def_func_01 gives some error relating to _lfortran_strcpy not being found

[certik]

Thanks for being patient with our new workflow. I checked it out locally, and got on updating the submodule:

$ git submodule update
fatal: transport 'file' not allowed
fatal: Fetched in submodule path 'libasr', but it did not contain 8f61f6049404b0b7c46dd15ca6dbcb24022b3bc1. Direct fetching of that commit failed.

But the following two commands fixed it:

$ cd libasr
$ git remote add swamishiju [email protected]:swamishiju/lfortran
$ git fetch swamishiju
$ cd ..
$ git submodule update
Submodule path 'libasr': checked out '8f61f6049404b0b7c46dd15ca6dbcb24022b3bc1'

[certik]

@swamishiju help me understand the issue here. In LPython, the following code:

def factorial_1(x: i32, y:i32 =1) ->i32 :
    if x <= 1:
         return y
    return x * factorial_1(x-1)

def test_factorial_1():
    test_00 : i32  = factorial_1(1)
    print("test_00 is =>", test_00)
    #assert test_00 == 1

    test_01 : i32  = factorial_1(5,0)
    print("test_01 is =>", test_01)
    #assert test_01 == 120

    test_02 : i32  = factorial_1(1,5555)
    print("test_02 is =>", test_02)
    #assert test_02 == 5555

Becomes after all ASR passes:

integer(4) function factorial_1(x, y, is_y_present_) result(_lpython_return_variable)
    logical(4), intent(in) :: is_y_present_
    integer(4), intent(in) :: x
    integer(4), intent(in) :: y
    if (x <= 1) then
        _lpython_return_variable = y
        return
    end if
    _lpython_return_variable = x*factorial_1(x - 1, 1, .true.)
    return
end function factorial_1

subroutine test_factorial_1()
    integer(4) :: test_00
    integer(4) :: test_01
    integer(4) :: test_02
    test_00 = factorial_1(1, 1, .true.)
    print *, "test_00 is =>", test_00
    test_01 = factorial_1(5, 0, .true.)
    print *, "test_01 is =>", test_01
    test_02 = factorial_1(1, 5555, .true.)
    print *, "test_02 is =>", test_02
end subroutine test_factorial_1

So the is_y_present_ is not used, since the default values are simply provided in the caller. What exactly is the logic in the libasr change that you made:

--- a/src/libasr/pass/array_struct_temporary.cpp
+++ b/src/libasr/pass/array_struct_temporary.cpp
@@ -2240,8 +2240,9 @@ class TransformVariableInitialiser:
                 result_vec.push_back(al, ASRUtils::STMT(ASR::make_Associate_t(
                     al, loc, target, value)));
             } else {
-                result_vec.push_back(al, ASRUtils::STMT(make_Assignment_t_util(
-                    al, loc, target, value, nullptr, exprs_with_target)));
+                if (!(xx.m_intent == ASR::intentType::In && xx.m_presence == ASR::presenceType::Optional))
+                    result_vec.push_back(al, ASRUtils::STMT(make_Assignment_t_util(
+                        al, loc, target, value, nullptr, exprs_with_target)));
             }
             xx.m_symbolic_value = nullptr;
             xx.m_value = nullptr;

In here we skip pushing back into result_vec if the variable is intent(in), optional. This result_vec gets appended to symtab2decls, and then in transform_stmts these statements get inserted at the beginning of the function. By skipping it, we will not be assigning to the variable xx. However, this assignment doesn't seem to happen in LPython anyway. And in LFortran I think we don't have default values. So when is this being used?

[certik]

How about this fix: lfortran/lfortran#7016.

[certik]

I merged that one, and updated LPython here: #2852. Once it's merged, then go ahead and update your PR against the latest main of LPython.

[swamishiju]

@certik I found my fix while looking at C back end code if I remember correctly

def foo(bar:i32 = 5):
        pass

would give error because of the intent check, if we commented that out the generated C code was

void foo(int bar, bool _is_bar_present) {
         bar = 5;
}

I figured that something similar happens on C and LLVM back ends since commenting out the intent check for testing always gives foo(5) ( I put a print inside to check ) no matter what argument was used.

This code also failed asr_verify after the array_struct_temporary pass (this is the one that creates the assignment). The easiest way I thought was to simply not create an assignment for optional function arguments and use the symbolic value for simply book keeping and making the proper function calls.

[swamishiju]

How about this fix: lfortran/lfortran#7016.

Yes I think that makes more sense, mine was based on thinking only optional function arguments will be both Optional and In ( it does feel a bit hand-wavey ) but yes your code is definitely better and also more readable.

[swamishiju]

Thanks for being patient with our new workflow. I checked it out locally, and got on updating the submodule:

$ git submodule update
fatal: transport 'file' not allowed
fatal: Fetched in submodule path 'libasr', but it did not contain 8f61f6049404b0b7c46dd15ca6dbcb24022b3bc1. Direct fetching of that commit failed.

But the following two commands fixed it:

$ cd libasr
$ git remote add swamishiju [email protected]:swamishiju/lfortran
$ git fetch swamishiju
$ cd ..
$ git submodule update
Submodule path 'libasr': checked out '8f61f6049404b0b7c46dd15ca6dbcb24022b3bc1'

No worries I had read up on how sub-modules work so that was a plus

[swamishiju]

@certik There is also an existing problem with optional args from pre sync

def foo(bar:i32, baz:i32 = 2, raz:i32)

Will not be a valid function defn in CP I dont think there is a check against that