ballerina-platform / ballerina-spec

Ballerina Language and Platform Specifications
Other
167 stars 53 forks source link

Using `check` in the default value expression of a parameter #1301

Closed MaryamZi closed 4 months ago

MaryamZi commented 7 months ago

Description: We've disallowed using check in record field default values. The commit for that change specifically says that it applies only for default-expressions of record fields (and not function parameters).

But, shouldn't this apply to function parameter defaults also? IMO, with functions also we have the same issue as records - not being able to represent the possibility of failure in the expression's type.

Code sample that shows issue:

import ballerina/io;

public function main() {
    int value = f1(); // panics in jBallerina atm
    io:println(value);
}

function f1(int a = check f2()) returns int => a;

function f2() returns int|error => error("err!");

Related Issues: https://github.com/ballerina-platform/ballerina-spec/issues/886

jclark commented 7 months ago

Hmm. I thought about the default function parameter case, and it seemed to me that it ought to be possible to handle check. But as you say, it's basically the same problem as record field defaults.

Let's consider the default function parameter case, specifically your example. It's the caller that is responsible for inserting the default in the function call, and it does so based on the function signature. It's the function signature that tells the caller that the parameter has a default. But is it not the case that the function signature equally tells the caller that there is the default expression that has the possibility of a check failure with type error? Therefore, this line

int value = f1();

should get a compile-time error, because the default for f1 may cause a check failure that the return type of main does not allow.

Doesn't this equally apply to #886? (Need to think more about that.)

MaryamZi commented 6 months ago

I'm not sure if the function signature can always convey that a default value expression can complete with a check-fail, since we may only have access to the default value closure (we'll have type information, but not the actual default value expression)?

Assuming we only have access to the default value closure, for

function f1(int a = check f2()) returns int => a;

if the return type of the default value closure (say aClosure) is int|error (not int), I guess we can use check with the closure and require it to be handled.

This would require error? in the main function's return type in the original example, and will call the closure function with check, as suggested.

function aClosure() returns int|error { // generated closure
    return check f2();
}

public function main() returns error? {
    int value = f1(check aClosure());
}

But this wouldn't be possible when the type of the parameter also includes error? We can't differentiate between f1 and f2 in

function fn() returns int|error => 1;

function f1(int|error a = fn()) {

}

function f2(int|error a = check fn()) {

}

Based on just the default value closures, we wouldn't know when to use check vs when not to?

We'll have similar concerns at runtime, with langlib functions that use record/parameter default value closures at runtime. There, we wouldn't be able to propagate the error from check as usual either.

jclark commented 6 months ago

You would need more than the default value closure; you would also have the type of possible check fail. By signature, I mean what's in the syntactic function-signature.

I'm not saying there's nothing to fix here in the spec here. But I think the better way to fix it is to make it work rather than make it a compile-time error.

jclark commented 6 months ago

The difficulty in my argument is that we are turning the default value expression into a closure, but there's no way for a closure to do what a check fail does (because it is local to a function). To make this work we would have to turn the default value expression into a closure in more complicated way that wraps the value and the check fail separately, but I think that's too much complexity.