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

suleka96 · 2023-04-24T22:21:46Z

Purpose

Add necessary compiler changes and tests to facilitate error bp in on fail clause as a part of fixing #29183

Check List

into on-fail-compiler

compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java

...rina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/CodeAnalyzer.java

...-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SemanticAnalyzer.java

github-actions · 2023-05-13T19:33:13Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

codecov · 2023-05-24T05:40:15Z

Codecov Report

Patch coverage: 88.94% and project coverage change: +0.02 🎉

Comparison is base (29ba158) 77.16% compared to head (563e1d4) 77.19%.

Additional details and impacted files

@@                  Coverage Diff                   @@
##             on-fail-error-bp   #40293      +/-   ##
======================================================
+ Coverage               77.16%   77.19%   +0.02%     
- Complexity              54244    54272      +28     
======================================================
  Files                    3340     3340              
  Lines                  201659   201697      +38     
  Branches                25914    25926      +12     
======================================================
+ Hits                   155603   155690      +87     
+ Misses                  37599    37553      -46     
+ Partials                 8457     8454       -3

Impacted Files	Coverage Δ
...ina/runtime/internal/values/TypedescValueImpl.java	`84.84% <ø> (ø)`
...in/java/io/ballerina/cli/cmd/DeprecateCommand.java	`10.76% <0.00%> (ø)`
...ballerinalang/central/client/CentralAPIClient.java	`44.53% <0.00%> (+0.31%)`	⬆️
...allerinalang/compiler/bir/codegen/JvmValueGen.java	`93.10% <ø> (ø)`
...lerinalang/compiler/desugar/AnnotationDesugar.java	`95.62% <ø> (ø)`
.../langserver/common/constants/CommandConstants.java	`90.00% <ø> (ø)`
...rc/main/java/io/ballerina/cli/cmd/TestCommand.java	`46.25% <33.33%> (ø)`
...rinalang/langserver/hover/HoverObjectResolver.java	`82.79% <50.00%> (ø)`
.../main/java/io/ballerina/cli/task/RunTestsTask.java	`82.59% <55.55%> (ø)`
...ions/providers/context/MappingContextProvider.java	`80.55% <81.25%> (ø)`
... and 32 more

... and 12 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

into on-fail-compiler

…orm/ballerina-lang into on-fail-compiler

...iler/ballerina-lang/src/main/java/org/ballerinalang/util/diagnostic/DiagnosticErrorCode.java

compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java

...st/src/test/java/org/ballerinalang/langlib/test/statements/foreach/ForeachNegativeTests.java

MaryamZi · 2023-06-12T04:53:34Z

langlib/langlib-test/src/test/resources/test-src/statements/foreach/foreach-on-fail.bal

+        testErrorCode = errCode;
+        testErrorReason = errReason;
+        testMessage = msg;
+    }
+    assertEquality(testMessage, "error!");
+    assertEquality(testErrorCode, 20);
+    assertEquality(testErrorReason, "deadlock condition");
+    assertEquality(testErrorCode is int, true);
+    assertEquality(testErrorReason is string, true);


I wouldn't use module-level variables, won't allow us to properly run these tests concurrently.

How about asserting withing the on fail block and returning there. Then outside we can have something like an assert fail to make sure control was transferred to the on fail block.

MaryamZi · 2023-06-12T04:54:09Z

...test/src/test/resources/test-src/statements/retrystmt/retry_on_fail_negative_unreachable.bal

+   return str;
+}
+
+function testOnFailWithUnion () returns string {


Suggested change

function testOnFailWithUnion () returns string {

function testOnFailWithUnion() returns string {

MaryamZi · 2023-06-12T04:54:51Z

...-test/src/test/java/org/ballerinalang/test/statements/retrystmt/RetryStmtWithOnFailTest.java


    @BeforeClass
    public void setup() {
        programFile = BCompileUtil.compile("test-src/statements/retrystmt/retry_stmt_with_on_fail.bal");
-        negativeFile = BCompileUtil.compile("test-src/statements/retrystmt/retry_on_fail_negative.bal");
+        negativeFile1 = BCompileUtil.compile("test-src/statements/retrystmt/retry_on_fail_negative.bal");
+        negativeFile2 = BCompileUtil.compile("test-src/statements/retrystmt/retry_on_fail_negative_unreachable.bal");


Used only in one test method, right? Use a local variable instead.

MaryamZi · 2023-06-12T04:56:02Z

...test/src/test/resources/test-src/statements/retrystmt/retry_on_fail_negative_unreachable.bal

Add licence header.

How is this file related to your changes?

IMO, the tests are overly complex for the errors we are trying to test. I understand that these are similar to existing tests, but not the easiest to read.

github-actions · 2023-06-30T19:33:28Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

github-actions · 2023-07-04T19:33:37Z

Closed PR due to inactivity for more than 18 days.

MaryamZi · 2023-07-19T02:55:50Z

Review suggestions will be addressed separately.

Need to also fix

type ErrorTypeA distinct error;

type ErrorTypeB distinct error;

function fn() {
    transaction {
        var res = checkpanic commit;
        fail error ErrorTypeA("oops!", message = "Type A");
    } on fail ErrorTypeA e {

    }
}

Fails with

ERROR [temp.bal:(9:15,9:27)] incompatible error definition type: 'error' will not be matched to 'ErrorTypeA'
ERROR [temp.bal:(9:15,9:25)] incompatible types: expected '(error|ErrorTypeA)', found 'ErrorTypeA'

suleka96 added 6 commits April 19, 2023 14:27

Merge branch 'on-fail-fix' of https://github.com/suleka96/ballerina-lang

b3f361a

into on-fail-compiler

Add nodeBuilder change

983e08b

Add semantic analyzer changes

0a735b1

Add dataFlow, codeAnalyzer and desugar changes

6edaf15

Remove import

9e46ae6

Merge branch 'on-fail-fix' of https://github.com/suleka96/ballerina-lang

6644e9a

into on-fail-compiler

MaryamZi reviewed Apr 28, 2023

View reviewed changes

github-actions bot added the Stale label May 13, 2023

suleka96 removed the Stale label May 16, 2023

suleka96 added 3 commits May 19, 2023 16:36

Add all tests

702e132

Update desugar code

d275b77

Update Do tests

664c9a2

suleka96 added 2 commits June 8, 2023 10:29

Merge branch 'on-fail-fix' of https://github.com/suleka96/ballerina-lang

c2f300b

into on-fail-compiler

Merge branch 'on-fail-error-bp' of https://github.com/ballerina-platf…

563e1d4

…orm/ballerina-lang into on-fail-compiler

suleka96 marked this pull request as ready for review June 8, 2023 05:08

suleka96 requested review from sameerajayasoma and hasithaa as code owners June 8, 2023 05:08

MaryamZi requested review from pcnfernando, dulajdilshan and chiranSachintha June 8, 2023 07:34

MaryamZi reviewed Jun 12, 2023

View reviewed changes

github-actions bot added the Stale label Jun 30, 2023

github-actions bot closed this Jul 4, 2023

MaryamZi removed the Stale label Jul 5, 2023

MaryamZi reopened this Jul 5, 2023

MaryamZi merged commit ff22128 into ballerina-platform:on-fail-error-bp Jul 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

suleka96 commented Apr 24, 2023

github-actions bot commented May 13, 2023

codecov bot commented May 24, 2023 •

edited

Loading

MaryamZi Jun 12, 2023

MaryamZi Jun 12, 2023

MaryamZi Jun 12, 2023

MaryamZi Jun 12, 2023

MaryamZi Jun 12, 2023

github-actions bot commented Jun 30, 2023

github-actions bot commented Jul 4, 2023

MaryamZi commented Jul 19, 2023

	function testOnFailWithUnion () returns string {
	function testOnFailWithUnion() returns string {

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

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

Conversation

suleka96 commented Apr 24, 2023

Purpose

Check List

github-actions bot commented May 13, 2023

codecov bot commented May 24, 2023 • edited Loading

Codecov Report

MaryamZi Jun 12, 2023

Choose a reason for hiding this comment

MaryamZi Jun 12, 2023

Choose a reason for hiding this comment

MaryamZi Jun 12, 2023

Choose a reason for hiding this comment

MaryamZi Jun 12, 2023

Choose a reason for hiding this comment

MaryamZi Jun 12, 2023

Choose a reason for hiding this comment

github-actions bot commented Jun 30, 2023

github-actions bot commented Jul 4, 2023

MaryamZi commented Jul 19, 2023

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

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

codecov bot commented May 24, 2023 •

edited

Loading