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

Add compiler changes for enabling Error BP in on fail clause #40293

Merged
merged 11 commits into from
Jul 18, 2023
Merged

Add compiler changes for enabling Error BP in on fail clause #40293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b3f361a
Merge branch 'on-fail-fix' of https://github.com/suleka96/ballerina-l…
suleka96 Apr 19, 2023
983e08b
Add nodeBuilder change
suleka96 Apr 19, 2023
0a735b1
Add semantic analyzer changes
suleka96 Apr 21, 2023
6edaf15
Add dataFlow, codeAnalyzer and desugar changes
suleka96 Apr 24, 2023
9e46ae6
Remove import
suleka96 Apr 25, 2023
6644e9a
Merge branch 'on-fail-fix' of https://github.com/suleka96/ballerina-l…
suleka96 Apr 25, 2023
702e132
Add all tests
suleka96 May 19, 2023
d275b77
Update desugar code
suleka96 May 22, 2023
664c9a2
Update Do tests
suleka96 May 23, 2023
c2f300b
Merge branch 'on-fail-fix' of https://github.com/suleka96/ballerina-l…
suleka96 Jun 8, 2023
563e1d4
Merge branch 'on-fail-error-bp' of https://github.com/ballerina-platf…
suleka96 Jun 8, 2023
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
4 changes: 3 additions & 1 deletion ...r/ballerina-lang/src/main/java/org/ballerinalang/util/diagnostic/DiagnosticErrorCode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,9 @@ public enum DiagnosticErrorCode implements DiagnosticCode {
EMPTY_REGEXP_STRING_DISALLOWED(
"BCS4044", "empty.regexp.string.disallowed"),
UNSUPPORTED_EMPTY_CHARACTER_CLASS(
"BCS4045", "unsupported.empty.character.class")
"BCS4045", "unsupported.empty.character.class"),

INVALID_BINDING_PATTERN_IN_ON_FAIL("BCS4046", "invalid.binding.pattern.in.on.fail")
suleka96 marked this conversation as resolved.
Show resolved Hide resolved
;

private String diagnosticId;
Expand Down
16 changes: 16 additions & 0 deletions .../ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/ASTBuilderUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.wso2.ballerinalang.compiler.tree.BLangService;
import org.wso2.ballerinalang.compiler.tree.BLangSimpleVariable;
import org.wso2.ballerinalang.compiler.tree.BLangTupleVariable;
import org.wso2.ballerinalang.compiler.tree.BLangVariable;
import org.wso2.ballerinalang.compiler.tree.bindingpatterns.BLangBindingPattern;
import org.wso2.ballerinalang.compiler.tree.bindingpatterns.BLangCaptureBindingPattern;
import org.wso2.ballerinalang.compiler.tree.clauses.BLangMatchClause;
Expand Down Expand Up @@ -552,6 +553,21 @@ static BLangRecordVariableDef createRecordVariableDef(Location pos, BLangRecordV
return variableDef;
}

static BLangErrorVariable createErrorVariable(Location pos, BType type, BLangExpression expr,
BLangSimpleVariable message, BLangVariable cause,
BLangSimpleVariable restDetail,
List<BLangErrorVariable.BLangErrorDetailEntry> detail) {
final BLangErrorVariable errVariable = (BLangErrorVariable) TreeBuilder.createErrorVariableNode();
errVariable.pos = pos;
errVariable.setBType(type);
errVariable.expr = expr;
errVariable.message = message;
errVariable.cause = cause;
errVariable.restDetail = restDetail;
errVariable.detail = detail;
return errVariable;
}

static BLangErrorVariableDef createErrorVariableDef(Location pos, BLangErrorVariable variable) {
final BLangErrorVariableDef variableDef =
(BLangErrorVariableDef) TreeBuilder.createErrorVariableDefinitionNode();
Expand Down
56 changes: 34 additions & 22 deletions compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5157,18 +5157,8 @@ private BLangFail createOnFailInvocation(BLangOnFailClause onFailClause, BLangFa
onFailBody.stmts.addAll(onFailClause.body.stmts);
env.scope.entries.putAll(onFailClause.body.scope.entries);
onFailBody.failureBreakMode = onFailClause.body.failureBreakMode;
VariableDefinitionNode onFailVarDefNode = onFailClause.variableDefinitionNode;

if (onFailVarDefNode != null) {
BVarSymbol onFailErrorVariableSymbol =
((BLangSimpleVariableDef) onFailVarDefNode).var.symbol;
BLangSimpleVariable errorVar = ASTBuilderUtil.createVariable(onFailErrorVariableSymbol.pos,
onFailErrorVariableSymbol.name.value, onFailErrorVariableSymbol.type, rewrite(fail.expr, env),
onFailErrorVariableSymbol);
BLangSimpleVariableDef errorVarDef = ASTBuilderUtil.createVariableDef(onFailClause.pos, errorVar);
env.scope.define(onFailErrorVariableSymbol.name, onFailErrorVariableSymbol);
onFailBody.stmts.add(0, errorVarDef);
}
handleOnFailErrorVarDefNode(onFailClause.variableDefinitionNode, onFailBody, fail);

if (onFailClause.isInternal && fail.exprStmt != null) {
if (fail.exprStmt instanceof BLangPanic) {
Expand All @@ -5187,6 +5177,38 @@ onFailErrorVariableSymbol.name.value, onFailErrorVariableSymbol.type, rewrite(fa
return fail;
}

private void handleOnFailErrorVarDefNode(VariableDefinitionNode onFailVarDefNode, BLangBlockStmt onFailBody,
BLangFail fail) {
if (onFailVarDefNode != null) {
suleka96 marked this conversation as resolved.
Show resolved Hide resolved
BLangStatement stmt;
suleka96 marked this conversation as resolved.
Show resolved Hide resolved
BLangVariable variableNode = (BLangVariable) onFailVarDefNode.getVariable();
if (variableNode.getKind() == NodeKind.ERROR_VARIABLE) {
stmt = handleErrorBPInOnFail((BLangErrorVariable) variableNode, fail);
} else {
stmt = handleCaptureBPInOnFail((BLangSimpleVariable) variableNode, fail);
}
onFailBody.stmts.add(0, stmt);
}
}

private BLangStatement handleErrorBPInOnFail (BLangErrorVariable varNode, BLangFail fail) {
suleka96 marked this conversation as resolved.
Show resolved Hide resolved
BLangErrorVariable errorVar = ASTBuilderUtil.createErrorVariable(varNode.pos,
varNode.getBType(), rewrite(fail.expr, env), varNode.message, varNode.cause, varNode.restDetail,
varNode.detail);
BLangErrorVariableDef errorVarDef = ASTBuilderUtil.createErrorVariableDef(onFailClause.pos, errorVar);
suleka96 marked this conversation as resolved.
Show resolved Hide resolved
return rewrite(errorVarDef, env);
}

private BLangStatement handleCaptureBPInOnFail(BLangSimpleVariable varNode, BLangFail fail) {
BVarSymbol onFailErrorVariableSymbol = varNode.symbol;
BLangSimpleVariable errorVar = ASTBuilderUtil.createVariable(onFailErrorVariableSymbol.pos,
onFailErrorVariableSymbol.name.value, onFailErrorVariableSymbol.type, rewrite(fail.expr, env),
onFailErrorVariableSymbol);
BLangSimpleVariableDef errorVarDef = ASTBuilderUtil.createVariableDef(onFailClause.pos, errorVar);
env.scope.define(onFailErrorVariableSymbol.name, onFailErrorVariableSymbol);
return errorVarDef;
}

private BLangBlockStmt rewriteNestedOnFail(BLangOnFailClause onFailClause, BLangFail fail) {
BLangOnFailClause currentOnFail = this.onFailClause;

Expand All @@ -5195,18 +5217,8 @@ private BLangBlockStmt rewriteNestedOnFail(BLangOnFailClause onFailClause, BLang
onFailBody.scope = onFailClause.body.scope;
onFailBody.mapSymbol = onFailClause.body.mapSymbol;
onFailBody.failureBreakMode = onFailClause.body.failureBreakMode;
VariableDefinitionNode onFailVarDefNode = onFailClause.variableDefinitionNode;

if (onFailVarDefNode != null) {
BVarSymbol onFailErrorVariableSymbol =
((BLangSimpleVariableDef) onFailVarDefNode).var.symbol;
BLangSimpleVariable errorVar = ASTBuilderUtil.createVariable(onFailErrorVariableSymbol.pos,
onFailErrorVariableSymbol.name.value, onFailErrorVariableSymbol.type, rewrite(fail.expr, env),
onFailErrorVariableSymbol);
BLangSimpleVariableDef errorVarDef = ASTBuilderUtil.createVariableDef(onFailClause.pos, errorVar);
onFailBody.scope.define(onFailErrorVariableSymbol.name, onFailErrorVariableSymbol);
onFailBody.stmts.add(0, errorVarDef);
}
handleOnFailErrorVarDefNode(onFailClause.variableDefinitionNode, onFailBody, fail);

int currentOnFailIndex = this.enclosingOnFailClause.indexOf(this.onFailClause);
int enclosingOnFailIndex = currentOnFailIndex <= 0 ? this.enclosingOnFailClause.size() - 1
Expand Down
16 changes: 7 additions & 9 deletions ...ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/parser/BLangNodeBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3921,15 +3921,13 @@ public BLangNode transform(OnFailClauseNode onFailClauseNode) {
BLangOnFailClause onFailClause = (BLangOnFailClause) TreeBuilder.createOnFailClauseNode();
onFailClause.pos = pos;
onFailClauseNode.typedBindingPattern().ifPresent(typedBindingPatternNode -> {
if (typedBindingPatternNode.bindingPattern().kind() == SyntaxKind.CAPTURE_BINDING_PATTERN) {
VariableDefinitionNode variableDefinitionNode =
createBLangVarDef(getPosition(typedBindingPatternNode), typedBindingPatternNode,
Optional.empty(), Optional.empty());
onFailClause.isDeclaredWithVar =
typedBindingPatternNode.typeDescriptor().kind() == SyntaxKind.VAR_TYPE_DESC;
markVariableWithFlag((BLangVariable) variableDefinitionNode.getVariable(), Flag.FINAL);
onFailClause.variableDefinitionNode = variableDefinitionNode;
}
VariableDefinitionNode variableDefinitionNode =
createBLangVarDef(getPosition(typedBindingPatternNode), typedBindingPatternNode,
Optional.empty(), Optional.empty());
onFailClause.isDeclaredWithVar =
typedBindingPatternNode.typeDescriptor().kind() == SyntaxKind.VAR_TYPE_DESC;
markVariableWithFlag((BLangVariable) variableDefinitionNode.getVariable(), Flag.FINAL);
onFailClause.variableDefinitionNode = variableDefinitionNode;
});
BLangBlockStmt blockNode = (BLangBlockStmt) transform(onFailClauseNode.blockStatement());
blockNode.pos = pos;
Expand Down
70 changes: 6 additions & 64 deletions ...a-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/CodeAnalyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,7 +579,6 @@ public void visit(BLangTransaction transactionNode, AnalyzerData data) {
boolean onFailExists = transactionNode.onFailClause != null;
boolean failureHandled = data.failureHandled;
if (onFailExists) {
data.errorTypes.push(new LinkedHashSet<>());
data.failureHandled = true;
}
analyzeNode(transactionNode.transactionBody, data);
Expand Down Expand Up @@ -654,7 +653,6 @@ public void visit(BLangRetry retryNode, AnalyzerData data) {
boolean onFailExists = retryNode.onFailClause != null;
boolean failureHandled = data.failureHandled;
if (onFailExists) {
data.errorTypes.push(new LinkedHashSet<>());
data.failureHandled = true;
}
visitNode(retryNode.retrySpec, data);
Expand Down Expand Up @@ -771,7 +769,6 @@ public void visit(BLangMatchStatement matchStatement, AnalyzerData data) {
boolean onFailExists = matchStatement.onFailClause != null;
boolean failureHandled = data.failureHandled;
if (onFailExists) {
data.errorTypes.push(new LinkedHashSet<>());
data.failureHandled = true;
}
List<BLangMatchClause> matchClauses = matchStatement.matchClauses;
Expand Down Expand Up @@ -1471,7 +1468,6 @@ public void visit(BLangForeach foreach, AnalyzerData data) {
boolean onFailExists = foreach.onFailClause != null;
boolean failureHandled = data.failureHandled;
if (onFailExists) {
data.errorTypes.push(new LinkedHashSet<>());
data.failureHandled = true;
}
data.loopCount++;
Expand All @@ -1492,7 +1488,6 @@ public void visit(BLangWhile whileNode, AnalyzerData data) {
boolean failureHandled = data.failureHandled;

if (onFailExists) {
data.errorTypes.push(new LinkedHashSet<>());
data.failureHandled = true;
}
data.loopCount++;
Expand All @@ -1511,7 +1506,6 @@ public void visit(BLangDo doNode, AnalyzerData data) {
boolean onFailExists = doNode.onFailClause != null;
boolean failureHandled = data.failureHandled;
if (onFailExists) {
data.errorTypes.push(new LinkedHashSet<>());
data.failureHandled = true;
}
analyzeNode(doNode.body, data);
Expand All @@ -1529,29 +1523,20 @@ public void visit(BLangFail failNode, AnalyzerData data) {
return;
}
typeChecker.checkExpr(failNode.expr, data.env);
if (!data.errorTypes.empty()) {
data.errorTypes.peek().add(getErrorTypes(failNode.expr.getBType()));
}
if (!data.failureHandled) {
BType exprType = data.env.enclInvokable.getReturnTypeNode().getBType();
data.returnTypes.peek().add(exprType);
if (!types.isAssignable(getErrorTypes(failNode.expr.getBType()), exprType)) {
if (!types.isAssignable(types.getErrorTypes(failNode.expr.getBType()), exprType)) {
dlog.error(failNode.pos, DiagnosticErrorCode.FAIL_EXPR_NO_MATCHING_ERROR_RETURN_IN_ENCL_INVOKABLE);
}
}
}

private BLangBlockStmt.FailureBreakMode getPossibleBreakMode(boolean possibleFailurePresent) {
return possibleFailurePresent ? BLangBlockStmt.FailureBreakMode.BREAK_TO_OUTER_BLOCK
: BLangBlockStmt.FailureBreakMode.NOT_BREAKABLE;
}

@Override
public void visit(BLangLock lockNode, AnalyzerData data) {
boolean onFailExists = lockNode.onFailClause != null;
boolean failureHandled = data.failureHandled;
if (onFailExists) {
data.errorTypes.push(new LinkedHashSet<>());
data.failureHandled = true;
}
boolean previousWithinLockBlock = data.withinLockBlock;
Expand Down Expand Up @@ -3188,7 +3173,7 @@ public void visit(BLangCheckedExpr checkedExpr, AnalyzerData data) {
return;
}

BType exprErrorTypes = getErrorTypes(checkedExpr.expr.getBType());
BType exprErrorTypes = types.getErrorTypes(checkedExpr.expr.getBType());
BType initMethodReturnType = initializerFunc.type.retType;
if (!types.isAssignable(exprErrorTypes, initMethodReturnType)) {
dlog.error(checkedExpr.pos, DiagnosticErrorCode
Expand All @@ -3205,7 +3190,7 @@ public void visit(BLangCheckedExpr checkedExpr, AnalyzerData data) {

BType exprType = enclInvokable.getReturnTypeNode().getBType();
BType checkedExprType = checkedExpr.expr.getBType();
BType errorType = getErrorTypes(checkedExprType);
BType errorType = types.getErrorTypes(checkedExprType);

if (errorType == symTable.semanticError) {
return;
Expand All @@ -3217,9 +3202,6 @@ public void visit(BLangCheckedExpr checkedExpr, AnalyzerData data) {
dlog.error(checkedExpr.pos,
DiagnosticErrorCode.CHECKED_EXPR_NO_MATCHING_ERROR_RETURN_IN_ENCL_INVOKABLE);
}
if (!data.errorTypes.empty()) {
data.errorTypes.peek().add(getErrorTypes(checkedExpr.expr.getBType()));
}

BType errorTypes;
if (exprType.tag == TypeTags.UNION) {
Expand Down Expand Up @@ -3350,15 +3332,11 @@ public void visit(BLangOnFailClause onFailClause, AnalyzerData data) {
VariableDefinitionNode onFailVarDefNode = onFailClause.variableDefinitionNode;

if (onFailVarDefNode != null) {
BLangVariable onFailVarNode = (BLangVariable) onFailVarDefNode.getVariable();
for (BType errorType : data.errorTypes.peek()) {
if (!types.isAssignable(errorType, onFailVarNode.getBType())) {
dlog.error(onFailVarNode.pos, DiagnosticErrorCode.INCOMPATIBLE_ON_FAIL_ERROR_DEFINITION, errorType,
onFailVarNode.getBType());
}
BLangNode onFailVarNode = (BLangNode) onFailClause.getVariableDefinitionNode().getVariable();
if (onFailVarNode != null) {
analyzeNode(onFailVarNode, data);
}
}
data.errorTypes.pop();
analyzeNode(onFailClause.body, data);
onFailClause.bodyContainsFail = data.failVisited;
data.failVisited = currentFailVisited;
Expand All @@ -3367,7 +3345,6 @@ public void visit(BLangOnFailClause onFailClause, AnalyzerData data) {
private void analyseOnFailAndUpdateBreakMode(boolean onFailExists, BLangBlockStmt blockStmt,
BLangOnFailClause onFailClause, AnalyzerData data) {
if (onFailExists) {
blockStmt.failureBreakMode = getPossibleBreakMode(!data.errorTypes.peek().isEmpty());
analyzeOnFailClause(onFailClause, data);
}
}
Expand Down Expand Up @@ -3863,40 +3840,6 @@ private void validateModuleInitFunction(BLangFunction funcNode) {
types.validateErrorOrNilReturn(funcNode, DiagnosticErrorCode.MODULE_INIT_RETURN_SHOULD_BE_ERROR_OR_NIL);
}

private BType getErrorTypes(BType bType) {
if (bType == null) {
return symTable.semanticError;
}

BType errorType = symTable.semanticError;

int tag = bType.tag;
if (tag == TypeTags.TYPEREFDESC) {
return getErrorTypes(Types.getReferredType(bType));
}
if (tag == TypeTags.ERROR) {
errorType = bType;
} else if (tag == TypeTags.READONLY) {
errorType = symTable.errorType;
} else if (tag == TypeTags.UNION) {
LinkedHashSet<BType> errTypes = new LinkedHashSet<>();
Set<BType> memTypes = ((BUnionType) bType).getMemberTypes();
for (BType memType : memTypes) {
BType memErrType = getErrorTypes(memType);

if (memErrType != symTable.semanticError) {
errTypes.add(memErrType);
}
}

if (!errTypes.isEmpty()) {
errorType = errTypes.size() == 1 ? errTypes.iterator().next() : BUnionType.create(null, errTypes);
}
}

return errorType;
}

/**
* This class contains the state machines for a set of workers.
*/
Expand Down Expand Up @@ -4177,7 +4120,6 @@ public static class AnalyzerData {
boolean withinQuery;
Types.QueryConstructType queryConstructType;
Stack<LinkedHashSet<BType>> returnTypes = new Stack<>();
Stack<LinkedHashSet<BType>> errorTypes = new Stack<>();
DefaultValueState defaultValueState = DefaultValueState.NOT_IN_DEFAULT_VALUE;
}
}
9 changes: 6 additions & 3 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 @@ -2269,9 +2269,12 @@ public void visit(BLangRecordVariableDef bLangRecordVariableDef) {
public void visit(BLangErrorVariable bLangErrorVariable) {
analyzeNode(bLangErrorVariable.typeNode, env);
populateUnusedVariableMapForNonSimpleBindingPatternVariables(this.unusedLocalVariables, bLangErrorVariable);
this.currDependentSymbolDeque.push(bLangErrorVariable.symbol);
analyzeNode(bLangErrorVariable.expr, env);
this.currDependentSymbolDeque.pop();
// When we visit this path from on fail clause the symbol is null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this true for capture binding patterns?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for on fail error msg kind of scenarios the flow doesn't go into this method right

if (bLangErrorVariable.symbol != null) {
this.currDependentSymbolDeque.push(bLangErrorVariable.symbol);
analyzeNode(bLangErrorVariable.expr, env);
this.currDependentSymbolDeque.pop();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused variable errors are not logged for something like

public function main() {
    do {

    } on fail Error error(i = i) {

    }
}

type Error error<record {| int i; |}>;

}

@Override
Expand Down
Loading