Skip to content

Commit

Permalink
Merge pull request #43776 from ballerina-platform/Improve-optional-fi…
Browse files Browse the repository at this point in the history 
…eld-access-desugar

[2201.10.x] Desugar field access expression to if statement expression
  • Loading branch information
@gimantha
gimantha authored Feb 6, 2025
2 parents 8a460b1 + 6743133 commit 1e12f9a
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 4 deletions.
100 changes: 96 additions & 4 deletions compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6335,12 +6335,13 @@ public void visit(BLangSimpleVarRef varRefExpr) {

@Override
public void visit(BLangFieldBasedAccess.BLangNSPrefixedFieldBasedAccess nsPrefixedFieldBasedAccess) {
rewriteFieldBasedAccess(nsPrefixedFieldBasedAccess);
rewriteFieldBasedAccess(nsPrefixedFieldBasedAccess, true);
}

private void rewriteFieldBasedAccess(BLangFieldBasedAccess fieldAccessExpr) {
private void rewriteFieldBasedAccess(BLangFieldBasedAccess fieldAccessExpr, boolean prefixedFieldBased) {
if (safeNavigate(fieldAccessExpr)) {
result = rewriteExpr(rewriteSafeNavigationExpr(fieldAccessExpr));
result = prefixedFieldBased ? rewriteExpr(rewriteSafeNavigationExpr(fieldAccessExpr)) :
rewriteExpr(rewriteSafeFieldBasedAccessExpr(fieldAccessExpr));
return;
}

Expand Down Expand Up @@ -6434,7 +6435,7 @@ private void rewriteFieldBasedAccess(BLangFieldBasedAccess fieldAccessExpr) {

@Override
public void visit(BLangFieldBasedAccess fieldAccessExpr) {
rewriteFieldBasedAccess(fieldAccessExpr);
rewriteFieldBasedAccess(fieldAccessExpr, false);
}

private BLangNode rewriteObjectMemberAccessAsField(BLangFieldBasedAccess fieldAccessExpr) {
Expand Down Expand Up @@ -10056,6 +10057,97 @@ private boolean safeNavigate(BLangAccessExpression accessExpr) {
return false;
}

private BLangExpression rewriteSafeFieldBasedAccessExpr(BLangAccessExpression accessExpr) {
/*
* Following is the structure of the safe navigation desugar.
* Before :
* type A {
* B b;
* }
* type B {
* int c;
* }
* int? res = a?.b?.c
*
* After desugar :
* any|error temp_result;
* temp_result = a;
* if temp_result !is error? {
* temp_result = (<A> temp_result).b;
* if temp_result !is error? {
* temp_result = (<B> temp_result).c;
* }
* }
* int? res = <int> temp_result;
*/
Location pos = accessExpr.pos;
// Create block statement to hold desugared statements.
BLangBlockStmt blockStmt = ASTBuilderUtil.createBlockStmt(pos);

// Create a temp variable to hold the intermediate result of the access expression.
String matchTempResultVarName = GEN_VAR_PREFIX.value + "temp_result";
BLangSimpleVariableDef tempResultVarDef = createVarDef(matchTempResultVarName, symTable.anyOrErrorType,
null, pos);
blockStmt.addStatement(tempResultVarDef);
BLangSimpleVarRef tempResultVarRef = ASTBuilderUtil.createVariableRef(pos, tempResultVarDef.var.symbol);

createNestedIfForAccessExpression(blockStmt, accessExpr, tempResultVarRef, pos);

BLangStatementExpression stmtExpr = createStatementExpression(blockStmt,
types.addConversionExprIfRequired(tempResultVarRef, accessExpr.getBType()));
stmtExpr.setBType(accessExpr.getBType());
return stmtExpr;
}

private BLangIf createNestedIfForAccessExpression(BLangBlockStmt blockStmt, BLangAccessExpression accessExpr,
BLangSimpleVarRef tempResultVarRef, Location pos) {
NodeKind kind = accessExpr.expr.getKind();
if (kind == NodeKind.FIELD_BASED_ACCESS_EXPR) {
BLangIf ifStmt = createNestedIfForAccessExpression(blockStmt, (BLangAccessExpression) accessExpr.expr,
tempResultVarRef, pos);
blockStmt = ifStmt.body;
} else {
// this else block only reach one time in the recursive execution
// this will assign the initial value for the `tempResultVarRef` eg: `temp_result = a;` in the sample
blockStmt.addStatement(ASTBuilderUtil.createAssignmentStmt(pos, tempResultVarRef, accessExpr.expr));
}

// Condition
BLangType testTypeNode = null;
if (accessExpr.errorSafeNavigation && accessExpr.nilSafeNavigation) {
testTypeNode = getErrorOrNillTypeNode();
} else if (accessExpr.errorSafeNavigation) {
testTypeNode = getErrorTypeNode();
} else if (accessExpr.nilSafeNavigation) {
testTypeNode = getNillTypeNode();
}

BLangExpression conditionExpr;
if (testTypeNode == null) {
conditionExpr = getBooleanLiteral(true);
} else {
BLangTypeTestExpr isNilOrErrorTest = createTypeCheckExpr(pos, tempResultVarRef, testTypeNode);
isNilOrErrorTest.isNegation = true;
isNilOrErrorTest.setBType(symTable.booleanType);
conditionExpr = isNilOrErrorTest;
}

// If body
BType type = types.getSafeType(accessExpr.expr.getBType(), true, true);
accessExpr.expr = types.addConversionExprIfRequired(tempResultVarRef, type);
accessExpr.errorSafeNavigation = false;
accessExpr.nilSafeNavigation = false;
BLangAssignment assignmentStmt =
ASTBuilderUtil.createAssignmentStmt(symTable.builtinPos, tempResultVarRef, accessExpr);
BLangBlockStmt ifBody = ASTBuilderUtil.createBlockStmt(symTable.builtinPos);
ifBody.addStatement(assignmentStmt);

BLangIf ifStmt = ASTBuilderUtil.createIfStmt(pos, blockStmt);
ifStmt.setCondition(conditionExpr);
ifStmt.body = ifBody;
return ifStmt;
}

private BLangExpression rewriteSafeNavigationExpr(BLangAccessExpression accessExpr) {
BType originalExprType = accessExpr.getBType();
// Create a temp variable to hold the intermediate result of the acces expression.
Expand Down
5 changes: 5 additions & 0 deletions ...na-unit-test/src/test/java/org/ballerinalang/test/expressions/access/FieldAccessTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,11 @@ public void testValidXMLmapFieldAccess() {
BRunUtil.invoke(result, "testValidXMLmapFieldAccess");
}

@Test
public void testLargeChainingFieldAccess() {
BRunUtil.invoke(result, "testLargeChainingFieldAccess");
}

@Test(dataProvider = "fieldAccessOnJsonTypedRecordFields")
public void testFieldAccessOnJsonTypedRecordFields(String function) {
BRunUtil.invoke(result, function);
Expand Down
11 changes: 11 additions & 0 deletions tests/jballerina-unit-test/src/test/resources/test-src/expressions/access/field_access.bal
View file
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,17 @@ function testValidXMLmapFieldAccess() {
assertEquals(y, null);
}

function testLargeChainingFieldAccess() {
// This test is to check the performance of the field access chaining and possible OOM and large method errors
json a = { "a": "b" };
anydata|error b = a?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b;
anydata|error c = a?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b;
anydata|error d = a?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b?.b;
assertTrue(b is ());
assertTrue(c is ());
assertTrue(d is ());
}

isolated function isEqual(anydata|error val1, anydata|error val2) returns boolean {
if (val1 is anydata && val2 is anydata) {
return (val1 == val2);
Expand Down

0 comments on commit 1e12f9a

Please sign in to comment.