Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revert "Consider on-fail as an execution branch when possible failure is reached" #39951

Merged
merged 1 commit into from
Mar 22, 2023
Merged

Revert "Consider on-fail as an execution branch when possible failure is reached" #39951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 28 additions & 59 deletions ...ng/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/DataflowAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,6 @@ public class DataflowAnalyzer extends BLangNodeVisitor {
private Map<BSymbol, Set<BSymbol>> globalNodeDependsOn;
private Map<BSymbol, Set<BSymbol>> functionToDependency;
private boolean flowTerminated = false;
private boolean possibleFailureReached = false;
private boolean visitingOnFailStmt = false;

private static final CompilerContext.Key<DataflowAnalyzer> DATAFLOW_ANALYZER_KEY = new CompilerContext.Key<>();
private Deque<BSymbol> currDependentSymbolDeque;
Expand Down Expand Up @@ -436,7 +434,6 @@ private void visitFunctionBodyWithDynamicEnv(BLangFunction funcNode, SymbolEnv f
// updated/marked as initialized.
this.uninitializedVars = copyUninitializedVars();
this.flowTerminated = false;
this.possibleFailureReached = false;

analyzeNode(funcNode.body, funcEnv);

Expand Down Expand Up @@ -535,7 +532,6 @@ public void visit(BLangClassDefinition classDef) {
this.unusedLocalVariables.putAll(prevUnusedLocalVariables);
this.uninitializedVars = copyUninitializedVars();
this.flowTerminated = false;
this.possibleFailureReached = false;
visitedOCE = true;
}
SymbolEnv objectEnv = SymbolEnv.createClassEnv(classDef, classDef.symbol.scope, env);
Expand Down Expand Up @@ -841,7 +837,10 @@ public void visit(BLangForeach foreach) {
}

analyzeNode(collection, env);
analyzeErrorHandlingStatement(foreach.body, foreach.onFailClause);
analyzeNode(foreach.body, env);
if (foreach.onFailClause != null) {
analyzeNode(foreach.onFailClause, env);
}
}

@Override
Expand All @@ -854,13 +853,13 @@ public void visit(BLangQueryAction queryAction) {
@Override
public void visit(BLangWhile whileNode) {
Map<BSymbol, InitStatus> prevUninitializedVars = this.uninitializedVars;
boolean prevVisitingOnFailStmt = this.visitingOnFailStmt;
if (whileNode.onFailClause != null) {
this.visitingOnFailStmt = true;
}

analyzeNode(whileNode.expr, env);
BranchResult whileResult = analyzeBranch(whileNode.body, env);
analyzeOnFailBranch(whileNode.onFailClause, whileResult);

if (whileNode.onFailClause != null) {
analyzeNode(whileNode.onFailClause, env);
}

BType constCondition = ConditionResolver.checkConstCondition(types, symTable, whileNode.expr);

Expand All @@ -875,56 +874,34 @@ public void visit(BLangWhile whileNode) {
}

this.uninitializedVars = mergeUninitializedVars(this.uninitializedVars, whileResult.uninitializedVars);
this.visitingOnFailStmt = prevVisitingOnFailStmt;
}

@Override
public void visit(BLangDo doNode) {
analyzeErrorHandlingStatement(doNode.body, doNode.onFailClause);
}

private void analyzeErrorHandlingStatement(BLangBlockStmt blockStmt, BLangOnFailClause onFailClause) {
boolean prevVisitingOnFailStmt = this.visitingOnFailStmt;
if (onFailClause != null) {
this.visitingOnFailStmt = true;
}
BranchResult doResult = analyzeBranch(blockStmt, env);
this.uninitializedVars = doResult.uninitializedVars;
analyzeOnFailBranch(onFailClause, doResult);
this.visitingOnFailStmt = prevVisitingOnFailStmt;
}

private void analyzeOnFailBranch(BLangOnFailClause onFailClause, BranchResult doResult) {
if (this.visitingOnFailStmt) {
// loop through the partial init variables from statement block
// and remove from `uninitializedVars` map if initialized in on-fail branch
BranchResult onFailResult = analyzeBranch(onFailClause, env);
Set<BSymbol> symbols = new HashSet<>();
for (BSymbol varRef: doResult.uninitializedVars.keySet()) {
InitStatus status = onFailResult.uninitializedVars.get(varRef);
if (status == null) {
symbols.add(varRef);
}
}
for (BSymbol symbol: symbols) {
doResult.uninitializedVars.remove(symbol);
}
analyzeNode(doNode.body, env);
if (doNode.onFailClause != null) {
analyzeNode(doNode.onFailClause, env);
}
}

public void visit(BLangFail failNode) {
this.possibleFailureReached = true;
analyzeNode(failNode.expr, env);
}

@Override
public void visit(BLangLock lockNode) {
analyzeErrorHandlingStatement(lockNode.body, lockNode.onFailClause);
analyzeNode(lockNode.body, this.env);
if (lockNode.onFailClause != null) {
analyzeNode(lockNode.onFailClause, env);
}
}

@Override
public void visit(BLangTransaction transactionNode) {
analyzeErrorHandlingStatement(transactionNode.transactionBody, transactionNode.onFailClause);
analyzeNode(transactionNode.transactionBody, env);
if (transactionNode.onFailClause != null) {
analyzeNode(transactionNode.onFailClause, env);
}

// marks the injected import as used
Name transactionPkgName = names.fromString(Names.DOT.value + Names.TRANSACTION_PACKAGE.value);
Expand Down Expand Up @@ -1937,9 +1914,6 @@ public void visit(BLangIsAssignableExpr assignableExpr) {

@Override
public void visit(BLangCheckedExpr checkedExpr) {
if (this.visitingOnFailStmt) {
this.possibleFailureReached = true;
}
analyzeNode(checkedExpr.expr, env);
}

Expand Down Expand Up @@ -1968,7 +1942,10 @@ public void visit(BLangAnnotationAttachment annAttachmentNode) {

@Override
public void visit(BLangRetry retryNode) {
analyzeErrorHandlingStatement(retryNode.retryBody, retryNode.onFailClause);
analyzeNode(retryNode.retryBody, env);
if (retryNode.onFailClause != null) {
analyzeNode(retryNode.onFailClause, env);
}
}

@Override
Expand Down Expand Up @@ -2326,28 +2303,25 @@ public void visit(BLangRegExpTemplateLiteral regExpTemplateLiteral) {
private BranchResult analyzeBranch(BLangNode node, SymbolEnv env) {
Map<BSymbol, InitStatus> prevUninitializedVars = this.uninitializedVars;
boolean prevFlowTerminated = this.flowTerminated;
boolean prevFailureReached = this.possibleFailureReached;

// Get a snapshot of the current uninitialized vars before visiting the node.
// This is done so that the original set of uninitialized vars will not be
// updated/marked as initialized.
this.uninitializedVars = copyUninitializedVars();
this.flowTerminated = false;
this.possibleFailureReached = false;

analyzeNode(node, env);
BranchResult branchResult = new BranchResult(this.uninitializedVars, this.flowTerminated);
BranchResult brachResult = new BranchResult(this.uninitializedVars, this.flowTerminated);

// Restore the original set of uninitialized vars
this.uninitializedVars = prevUninitializedVars;
this.flowTerminated = prevFlowTerminated;
this.possibleFailureReached = prevFailureReached;

return branchResult;
return brachResult;
}

private Map<BSymbol, InitStatus> copyUninitializedVars() {
return new LinkedHashMap<>(this.uninitializedVars);
return new HashMap<>(this.uninitializedVars);
}

private void analyzeNode(BLangNode node, SymbolEnv env) {
Expand Down Expand Up @@ -2510,12 +2484,7 @@ private void checkAssignment(BLangExpression varRef) {
addFunctionToGlobalVarDependency(owner, ((BLangSimpleVarRef) varRef).symbol);
}

BSymbol symbol = ((BLangVariableReference) varRef).symbol;
if (this.possibleFailureReached && this.uninitializedVars.containsKey(symbol)) {
this.uninitializedVars.put(symbol, InitStatus.PARTIAL_INIT);
} else {
this.uninitializedVars.remove(symbol);
}
this.uninitializedVars.remove(((BLangVariableReference) varRef).symbol);
}

private void checkFinalObjectFieldUpdate(BLangFieldBasedAccess fieldAccess) {
Expand Down
11 changes: 0 additions & 11 deletions ...na-unit-test/src/test/java/org/ballerinalang/test/statements/onfail/OnFailClauseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,6 @@ public void testOnFailClauseNegativeCaseV2() {
BAssertUtil.validateError(negativeResult, i++, "this function must return a result", 32, 1);
BAssertUtil.validateError(negativeResult, i++, "this function must return a result", 48, 1);
BAssertUtil.validateError(negativeResult, i++, "this function must return a result", 66, 1);
BAssertUtil.validateError(negativeResult, i++, "variable 'resultInt' may not have been initialized", 92, 5);
BAssertUtil.validateError(negativeResult, i++, "variable 'resultInt2' may not have been initialized", 106, 5);
BAssertUtil.validateError(negativeResult, i++, "variable 'resultInt3' may not have been initialized", 107, 5);
BAssertUtil.validateError(negativeResult, i++, "variable 'resultInt1' may not have been initialized", 121, 5);
BAssertUtil.validateError(negativeResult, i++, "variable 'resultInt2' may not have been initialized", 122, 5);
BAssertUtil.validateError(negativeResult, i++, "variable 'resultInt3' may not have been initialized", 123, 5);
BAssertUtil.validateError(negativeResult, i++, "variable 'resultInt2' may not have been initialized", 140, 5);
BAssertUtil.validateError(negativeResult, i++, "variable 'resultInt3' may not have been initialized", 141, 5);
BAssertUtil.validateError(negativeResult, i++, "variable 'resultInt3' may not have been initialized", 159, 5);
BAssertUtil.validateError(negativeResult, i++, "variable 'k' may not have been initialized", 174, 9);
BAssertUtil.validateError(negativeResult, i++, "variable 'str2' may not have been initialized", 212, 5);
Assert.assertEquals(negativeResult.getErrorCount(), i);
}

Expand Down
12 changes: 0 additions & 12 deletions ...unit-test/src/test/java/org/ballerinalang/test/types/globalvar/GlobalVarNegativeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,16 +198,4 @@ public void testConfigurableImplicitReadOnly() {
28, 22);
Assert.assertEquals(result.getErrorCount(), i);
}

@Test
public void testGlobalVariableInitWithOnFailNegative() {
CompileResult result = BCompileUtil.compile
("test-src/statements/variabledef/global_variable_init_with_onfail_negative.bal");
int i = 0;
BAssertUtil.validateError(result, i++, "uninitialized variable 'j'", 18, 1);
BAssertUtil.validateError(result, i++, "uninitialized variable 'k'", 19, 1);
BAssertUtil.validateError(result, i++, INVALID_FUNC_OR_METHOD_CALL_WITH_UNINITIALIZED_VARS_PREFIX +
"variable(s) 'j, k' not initialized", 31, 5);
Assert.assertEquals(result.getErrorCount(), i);
}
}
135 changes: 0 additions & 135 deletions ...na-unit-test/src/test/resources/test-src/statements/onfail/on-fail-clause-negative-v2.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,138 +78,3 @@ function getError() returns error {
error err = error("Custom Error");
return err;
}

function getErrorOrInt() returns int|error {
return getError();
}

public function testUnInitVars1() {
int resultInt;
do {
resultInt = check getErrorOrInt();
} on fail {
}
resultInt += 1;
}

public function testUnInitVars2() {
int resultInt1;
int resultInt2;
int resultInt3;
do {
resultInt1 = 1;
resultInt2 = check getErrorOrInt();
resultInt3 = 1;
} on fail {
}
resultInt1 += 1;
resultInt2 += 1;
resultInt3 += 1;
}

public function testUnInitVars3() {
int resultInt1;
int resultInt2;
int resultInt3;
transaction {
check commit;
resultInt1 = 1;
resultInt2 = check getErrorOrInt();
resultInt3 = 1;
} on fail {
}
resultInt1 += 1;
resultInt2 += 1;
resultInt3 += 1;
}

public function testUnInitVars4() {
int resultInt1;
int resultInt2;
int resultInt3;
transaction {
do {
resultInt1 = 1;
resultInt2 = check getErrorOrInt();
resultInt3 = 1;
}
check commit;
} on fail {
}
resultInt1 += 1;
resultInt2 += 1;
resultInt3 += 1;
}

public function testUnInitVars5() {
int resultInt1;
int resultInt2;
int resultInt3;
transaction {
do {
resultInt1 = 1;
resultInt2 = 2;
}
check commit;
resultInt3 = 1;
} on fail {
}
resultInt1 += 1;
resultInt2 += 1;
resultInt3 += 1;
}

function testUnInitVars6() {
int i;
int j;
int k;
do {
i = 0;
j = 1;
check getErrorOrNil();
k = 1;
} on fail {
i += 1;
j += 1;
k += 1;
}
}

function testUnInitVars7() {
int i;
int j;
int k;
do {
i = 0;
j = 1;
check getErrorOrNil();
k = 1;
} on fail {
k = -1;
}
i += 1;
j += 1;
k += 1;
}

function getErrorOrNil() returns error? {
return getError();
}

function testUnInitVars8(int[] data) returns string {
string str1 = "";
string str2;
foreach var i in data {
if(i < 0) {
check getErrorOrNil();
str1 = "partial init";
str2 = "partial init";
}
} on fail {
str1 += "-> error caught. Hence value returning";
return str1;
}
str2 += "-> reached end";
return str1;
}

13 changes: 0 additions & 13 deletions tests/jballerina-unit-test/src/test/resources/test-src/statements/onfail/on-fail-clause.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ function testOnFailEdgeTestcases() {
testOnFailWithinInLineServiceObj();
assertTrue(testOnFailInAnonFunctionExpr() is ());
testNoPossibleFailureWithOnFail();
testVarRefValueUpdate();
}

function testUnreachableCodeWithIf(){
Expand Down Expand Up @@ -589,18 +588,6 @@ function testOnFailInAnonFunctionExpr() returns error? {
assertEquality((<error>result2).detail().get("message"), "'string' value 'a2' cannot be converted to 'int'");
}

function testVarRefValueUpdate() {
int i = 0;
do {
i += 1;
_ = check getCheckError();
i += 1;
} on fail {
i = i - 1;
}
assertEquality(0, i);
}

//-------------------------------------------------------------------------------

type AssertionError error;
Expand Down
Loading