ballerina-platform / ballerina-lang

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

[Improvement]: Refactor `BUnionType`'s `getMemberTypes()` usages #43344

Open lochana-chathura opened 1 month ago

lochana-chathura commented 1 month ago

Description

$subject. We need to get rid of getMemberTypes() and getOriginalMemberTypes() usages in the current compiler, in order to do #43343.

This issue depends on #43126 which is in PR sent state now.

lochana-chathura commented 3 weeks ago

Fixed with 4f137ee

lochana-chathura commented 3 weeks ago

When converting isSameType() usages to semtypes, here at https://github.com/ballerina-platform/ballerina-lang/blob/d93ec613f5e22f67a868593a66493119d860f201/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/ServiceDesugar.java#L271

the same BObjectType instance with one inside a BUnionType wrapper, evaluate not to be the same type, in the current compiler.

Screenshot 2024-10-02 at 07 50 51

We are comparing BUnionType 5262 to BObjectType 5231

With semTypes, they both identified to be the same type, causing NPE at later stage

lochana-chathura commented 3 weeks ago

When rewriting isDistinctAllowedOnType() using semtypes, it was found that the same method is called twice for an BObjectType causing the wrong resolution of distinct type IDs for the semtype. When calling [1], BTypeIdSet info is not available, resulting in the wrong semtype resolution.

Two places: [1] https://github.com/ballerina-platform/ballerina-lang/blob/fe0509bed414ca8e1014f0bbb28e1f7a492de09d/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeResolver.java#L1098

[2] https://github.com/ballerina-platform/ballerina-lang/blob/fe0509bed414ca8e1014f0bbb28e1f7a492de09d/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeResolver.java#L729

Removing [1] fixed the issue.

lochana-chathura commented 2 weeks ago

Found another issue related to isDistinctAllowedOnType(). When calling it at Symbol Resolver, the BObject type's symbol does not have the attachedFuncs values updated, causing a wrong resolution of semtype for the object type.

type Obj0 distinct object {
    function apply() returns int;
};

type Obj2 object {
    function apply() returns int;
};

function foo(Obj0 x) {
    Obj2 y = x; // Should have no error
}
lochana-chathura commented 2 weeks ago

After rewriting isAllReadonlyTypes() using semtypes at, https://github.com/ballerina-platform/ballerina-lang/blob/fe0509bed414ca8e1014f0bbb28e1f7a492de09d/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java#L3588

the following test that used compile will now give an error,

class P1 {
    Q1 q;
    string p1;

    function init() {
        self.q = new;
        self.p1 = "P1";
    }
}

class Q1 {
    P1 p;
    string q1;

    function init() {
        self.p = new;
        self.q1 = "Q1";
    }
}

function testArraysOfCyclicDependentTypes3() returns P1[] {
    P1[] arr = [];
    arr[3] = new; // ERROR [test_parser.bal:(23:5,23:11)] cannot update 'readonly' value of type 'P1[]'
    return arr;
}

function testArraysOfCyclicDependentTypes4() returns Q1[] {
    Q1[] arr = [];
    arr[3] = new; // ERROR [test_parser.bal:(29:5,29:11)] cannot update 'readonly' value of type 'Q1[]'
    return arr;
}

This is because in semtype P1 and Q1 are considered to have an empty value space making them equivalent to never

lochana-chathura commented 2 weeks ago
const int[2]|[int, int] ERR_TUPLE5 = []; // ambiguous types

With the use of isSameType() semtype based API, the above will no longer give the ambiguous type error.

lochana-chathura commented 2 weeks ago

Sample:

function testBitWiseOperationsForNullable() {
    byte? x = 251;
    int:Signed8? y = -123;

    var ans7 = x >> y;
}

Replacing the following isSameType() with semtype-based API would crash as function (int:Unsigned8?,int:Signed8?) returns (int:Unsigned8?) considered to be resolved operator symbol rather than function (byte?,int:Signed8?) returns (byte?).

https://github.com/ballerina-platform/ballerina-lang/blob/b67b420361dc9db1d8860c8a17b3efadebd60aa0/compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SymbolResolver.java#L2205

BIR diff:

Screenshot 2024-10-08 at 15 29 03
crash log (click to expand) ``` org.ballerinalang.compiler.BLangCompilerException: error while generating method 'testBitWiseOperationsForNullable' in class 'test_parser' at org.wso2.ballerinalang.compiler.bir.codegen.JvmCodeGenUtil.visitMaxStackForMethod(JvmCodeGenUtil.java:760) at org.wso2.ballerinalang.compiler.bir.codegen.methodgen.MethodGen.genJMethodForBFunc(MethodGen.java:390) at org.wso2.ballerinalang.compiler.bir.codegen.methodgen.MethodGen.generateMethod(MethodGen.java:186) at org.wso2.ballerinalang.compiler.bir.codegen.JvmPackageGen.lambda$generateModuleClasses$0(JvmPackageGen.java:420) at java.base/java.util.HashMap.forEach(HashMap.java:1421) at org.wso2.ballerinalang.compiler.bir.codegen.JvmPackageGen.generateModuleClasses(JvmPackageGen.java:377) at org.wso2.ballerinalang.compiler.bir.codegen.JvmPackageGen.generate(JvmPackageGen.java:765) at org.wso2.ballerinalang.compiler.bir.codegen.CodeGenerator.generate(CodeGenerator.java:86) at org.wso2.ballerinalang.compiler.bir.codegen.CodeGenerator.generate(CodeGenerator.java:63) at io.ballerina.projects.JBallerinaBackend.performCodeGen(JBallerinaBackend.java:377) at io.ballerina.projects.ModuleContext.generateCodeInternal(ModuleContext.java:458) at io.ballerina.projects.ModuleCompilationState$4.generatePlatformSpecificCode(ModuleCompilationState.java:132) at io.ballerina.projects.ModuleContext.generatePlatformSpecificCode(ModuleContext.java:358) at io.ballerina.projects.JBallerinaBackend.performCodeGen(JBallerinaBackend.java:178) at io.ballerina.projects.JBallerinaBackend.(JBallerinaBackend.java:144) ```
lochana-chathura commented 1 week ago

When replacing the types.isSameBIRShape() call in BIRPackageSymbolEnter with semtype based isSameType(), encountered with the following crash.

stacktrace (click to expand) ``` ballerina-new-parser-test-suite > jballerina-test > org.ballerinalang.test.bala.types.TypeNameWithSpecialCharsBalaTest > testTypeNameWithSpecialChars FAILED org.ballerinalang.compiler.BLangCompilerException: failed to load the module 'specialchars/pkg:0.0.0' from its BIR due to: Cannot invoke "org.wso2.ballerinalang.compiler.semantics.model.types.BType.semType()" because "this.referredType" is null at app//org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter.definePackage(BIRPackageSymbolEnter.java:239) at app//org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter.definePackage(BIRPackageSymbolEnter.java:215) at app//io.ballerina.projects.ModuleContext.loadPackageSymbolInternal(ModuleContext.java:528) at app//io.ballerina.projects.ModuleCompilationState$6.compile(ModuleCompilationState.java:178) at app//io.ballerina.projects.ModuleContext.compile(ModuleContext.java:354) at app//io.ballerina.projects.PackageCompilation.compileModulesInternal(PackageCompilation.java:211) at app//io.ballerina.projects.PackageCompilation.compileModules(PackageCompilation.java:195) at app//io.ballerina.projects.PackageCompilation.compile(PackageCompilation.java:102) at app//io.ballerina.projects.PackageCompilation.from(PackageCompilation.java:97) at app//io.ballerina.projects.PackageContext.getPackageCompilation(PackageContext.java:243) at app//io.ballerina.projects.Package.getCompilation(Package.java:154) at app//org.ballerinalang.test.BCompileUtil.jBallerinaBackend(BCompileUtil.java:224) at app//org.ballerinalang.test.BCompileUtil.compile(BCompileUtil.java:99) at app//org.ballerinalang.test.bala.types.TypeNameWithSpecialCharsBalaTest.testTypeNameWithSpecialChars(TypeNameWithSpecialCharsBalaTest.java:43) Caused by: java.lang.NullPointerException: Cannot invoke "org.wso2.ballerinalang.compiler.semantics.model.types.BType.semType()" because "this.referredType" is null at org.wso2.ballerinalang.compiler.semantics.model.types.BTypeReferenceType.semType(BTypeReferenceType.java:44) at org.wso2.ballerinalang.compiler.semantics.model.types.BRecordType.semType(BRecordType.java:171) at org.wso2.ballerinalang.compiler.semantics.analyzer.SemTypeHelper.semType(SemTypeHelper.java:165) at org.wso2.ballerinalang.compiler.semantics.analyzer.SemTypeHelper.semType(SemTypeHelper.java:109) at org.wso2.ballerinalang.compiler.semantics.analyzer.Types.isSameTypeWithTags(Types.java:406) at org.wso2.ballerinalang.compiler.semantics.analyzer.Types.isSameBIRShape(Types.java:2498) at org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter.getType(BIRPackageSymbolEnter.java:2166) at org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter$BIRTypeReader.readType(BIRPackageSymbolEnter.java:1364) at org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter.readBType(BIRPackageSymbolEnter.java:715) at org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter$BIRTypeReader.readTypeFromCp(BIRPackageSymbolEnter.java:1185) at org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter$BIRTypeReader.readType(BIRPackageSymbolEnter.java:1551) at org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter.readBType(BIRPackageSymbolEnter.java:715) at org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter$BIRTypeReader.readTypeFromCp(BIRPackageSymbolEnter.java:1185) at org.wso2.ballerinalang.compiler.BIRPackageSymbolEnter$BIRTypeReader.readType(BIRPackageSymbolEnter.java:1385) ```
public type Bdd Node|boolean;

public type Node readonly & record {|
    int atom;
    Bdd left;
    Bdd middle;
    Bdd right;
|};

The reason is in the Node record type, the referred types for left, middle and right are null when calling at BIRPackageSymbolEnter. However, via the previous API since we don't access the type(referred type's symbol is used instead) it was not a problem.

Failing test: TypeNameWithSpecialCharsBalaTest

lochana-chathura commented 2 days ago

The reason is in the Node record type, the referred types for left, middle and right are null when calling at BIRPackageSymbolEnter. However, via the previous API since we don't access the type(referred type's symbol is used instead) it was not a problem.

The reason for null was that the type was still resolving. For BReferenceType we update the referredType after creating the instance. It turned out isSameBIRShape() was a temporary workaround(See: https://github.com/ballerina-platform/ballerina-lang/issues/35872#issuecomment-1108144686) and the original issue has been fixed with PR #36168.

Hence, isSameBIRShape() call was removed.

lochana-chathura commented 2 days ago
type X [(), (), (), (), int, (), (), string];
type Y [5.0];