ballerina-platform / ballerina-lang

The Ballerina Programming Language
https://ballerina.io/
Apache License 2.0
3.6k stars 745 forks source link

[Bug]: Dependently-Typed Functions Return Invalid Types when Assigned to an Error #42761

Open ThisaruGuruge opened 5 months ago

ThisaruGuruge commented 5 months ago

Description

When a dependently typed function is called and assigned to a union of an invalid type and an error, the function does not return an error. Instead, it assigns the value to the variable, and when type-guard is used the error throws.

Steps to Reproduce

Consider the following code:

import ballerina/io;
import ballerina/jballerina.java;

public class Foo {
    private final map<any> attributes = {};

    public function set(string 'key, any value) {
        self.attributes.put('key, value);
    }

    function get(typedesc returnType = <>) returns returnType = @java:Method {
        'class: "io.ballerina.Sample"
    } external;
}

public function main() returns error? {
    Foo foo = new;
    foo.set("String", "Ballerina"); // Setting a string value in the map.

    int|error value = foo.get("String"); // Retrieving the value as a string. This does not return an error

    if value is int {
        io:println("Value is int"); // This prints.
        int i = value + 1; // Error throws here.
    }
    return value;
}

Following is the native method:

package io.ballerina;

public class Sample {
    public static Object get(BObject object, BString key, BTypedesc returnType) {
        BMap<BString, Object> attributes = (BMap<BString, Object>) context.getMapValue(StringUtils.fromString("attributes"));
        if (attributes.containsKey(key)) {
            return attributes.get(key);
        }
        return ErrorUtils.createError(StringUtils.fromString("Key not found"));
    }
}

Affected Version(s)

2201.9.0

OS, DB, other environment details and versions

No response

Related area

-> Runtime

Related issue(s) (optional)

No response

Suggested label(s) (optional)

No response

Suggested assignee(s) (optional)

No response

HindujaB commented 4 months ago

This happens due to an unchecked cast in the generated code. The BIR of the above code is as follows,

 bb2 {
        %19 = ConstLoad String;
        %21 = newType int|error;
        %26 = get(%1, %19, %21) -> bb3;
    }
    bb3 {
        %17 = <int|error> %26; // Compiler added type cast expression
        %27 = %17 is int;
        %27? bb4 : bb6;
    }

This is generated as an unchecked cast due to the checkTypes boolean is set to false at compilation. Therefore, it fails at the next line (int i = value + 1;).

Seems like we need to handle the union-type conversions separately when adding type-conversion.

Screenshot 2024-05-21 at 09 43 40
MaryamZi commented 3 months ago

@chiranSachintha, @HindujaB, if this isn't dependently-typed, does it result in an error at

int|error value = foo.get("String"); 

Anyway, IIRC, at least with dependently-typed functions, we expect the developer to ensure they return the value belonging to the expected type. For example, the following is valid code, and a cast (even if added) won't/can't identify the issue here.

typedesc<anydata> td = int;
anydata x = foo.get("String", td);
public class Foo {
    private final map<any> attributes = {};

    public function set(string 'key, any value) {
        self.attributes['key] = value;
    }

    function get(string str, typedesc returnType = <>) returns returnType = @java:Method {
        'class: "io.ballerina.Sample"
    } external;
}

Foo foo = new;
foo.set("String", "Ballerina"); // Setting a string value in the map.
MaryamZi commented 3 months ago

@chiranSachintha @HindujaB any update on this?

HindujaB commented 3 months ago

The issue only happens for dependently typed functions, because if it is a function with a specific type the check cast is called at the return of that function.

    function get(string key) returns int|error = @java:Method {
        'class: "org.ballerinalang.nativeimpl.jvm.runtime.api.tests.Async"
    } external; 

I agree that we expect the user to provide a value with the expected type for dependently typed functions. In both cases, it has to fail with a runtime type checking error. But, we need to discuss in which statement it should fail.

IMO, we have to fail at the int|error assignment for the first case as the returned RHS value is not that type. To identify such cases at code generation, currently, we use the 'checkTypes` boolean. @chiranSachintha is there any way to identify these cases?

int|error value = foo.get("String"); 

For anydata that is not the case since int is a subtype ofanydata.

}