support for optional arguments

[ramseyharrison]

added support for optional arguments. a regular argument cannot follow an optional argument, so something like

fun foo(arg1, arg2 = true, arg3){}

raises an error. It's during the parsing of an invocation that missing arguments in the invocation are replaced with the default arguments parsed during definition. Once we determine the location of the first missing argument, all the default arguments which follow are also passed to the invocation. This is why we can't interleave optional and non-optionals.

EDIT :
In response to @b1ek 's comment, I confirm that since in the following example, the argument arg1 is generic, despite holding an optional argument, any argument type can be passed to it.

fun echo_var(arg1 = 100){
    echo a
}

So the following are valid calls : echo_var(100), echo_var("test").

If the argument however is typed,

fun echo_var(arg1 : Num = 100){
    echo a
}

Then echo_var("test") raises an error as expected.

[Mte90]

Maybe we need also a test?

[ramseyharrison]

Sure, I'll write some tests.

[Ph0enixKM]

First review looking good. Some minor changes required

[Ph0enixKM]

@b1ek can you check if this PR is ready?

[b1ek]

@b1ek can you check if this PR is ready?

sorry, i dont have the time rn

i will take a look tomorrow tho

[b1ek]

are the types supposed to be lazily evaluated? iirc that has not been done in any part of the project yet

the following code prints abc:

fun a(a = 123) {
	echo a
}

a("abc")

[b1ek]

are the types supposed to be lazily evaluated

actually, nevermind. it is not a lazy evaluation thing, its that it doesnt automatically get assigned the Int type and there is no test to assert a consistent behaviour for such type overrides

[Mte90]

So this PR can be approved or there is something pending @b1ek? Can be moved to a different ticket?

[b1ek]

So this PR can be approved or there is something pending @b1ek? Can be moved to a different ticket?

no, there is an issue with it. thats what is meant by "requested changes": either changes have to be made or the review to be dismissed

[Mte90]

I wasn't sure because of your next message.
@ramseyharrison can you add the test mentioned to the PR so we can merge it?

[ramseyharrison]

@b1ek What is the expected behavior? The problem with assigning a type to an optional argument which isn't explicitly typed is that the rest of the arguments would then have themselves to be typed. That's why I didn't enforce anything here.

[b1ek]

@b1ek What is the expected behavior? The problem with assigning a type to an optional argument is explicitly typed is that the rest of the arguments would then have themselves to be typed. That's why I didn't enforce anything here.

1. that is not mentioned in the description of the pr; therefore is undefined behavior
2. there is no test that covers this behavior
3. the function expects a number. if the user would pass "abc" to a parameter that is supposed to be an integer, it would result in a cryptic compiler errror in the middle of the function if it does integer specific operation with it (like 1 + 2)

the problem is not that i have expected it to behave like something, the problem is that the PR does not explain this behavior.

if you are adding a new feature that changes compiler behavior (or pretty much any other code change tbh), it must be well explained in the PR and tested

[ramseyharrison]

3. the function expects a number. if the user would pass "abc" to a parameter that is supposed to be an integer, it would result in a cryptic compiler errror in the middle of the function if it does integer specific operation with it (like 1 + 2)

I see. If one wrote the following (without optional arguments) :

fun a(a) {
	echo a + 2
}

a("abc")

They would get the same error as if they did this (with an optional argument)

fun a(a = 23) {
	echo a + 2
}

a("abc")

I want to make sure I'm writing the right kind of test. Do you need one which illustrates what I have just stated?

Thank you.
Ramsey

[b1ek]

@ramseyharrison explain how you want it to work with different default & actual type in the description and cover it with a test, then re request the reviews

[Ph0enixKM]

I think that this looks good enough. We can improve the erroring system for optional arguments in a separate ticket.

[b1ek]

@Ph0enixKM im not sure how exactly docs are published to the site, but please make sure that these shenanigans are properly explained