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.

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

Fix records with function calls for default values failing code generation #43563

Conversation

Thushara-Piyasekara
Copy link
Contributor

@Thushara-Piyasekara Thushara-Piyasekara commented Nov 9, 2024

Purpose

Fixes #43531

Remarks

The codegen failure was due to corrupted BIRInstructions originated from BIRRecordValueOptimizer. BIRRecordValueOptimizer tries to optimize default functions with other function invocations inside their bodies. This was fixed by updating the logic used to identify functions with only constant loads.

Check List

  • Read the Contributing Guide
  • Updated Change Log
  • Checked Tooling Support (#)
  • Added necessary tests
    • Unit Tests
    • Spec Conformance Tests
    • Integration Tests
    • Ballerina By Example Tests
  • Increased Test Coverage
  • Added necessary documentation
    • API documentation
    • Module documentation in Module.md files
    • Ballerina By Examples

@Thushara-Piyasekara
Copy link
Contributor Author

@HindujaB could you point out the location where the tests for the following class is located?

I tried searching but couldn't find a place. In case there are no tests for that class, could you suggest a place to add the sample test mentioned in #43531?

@HindujaB
Copy link
Contributor

@HindujaB could you point out the location where the tests for the following class is located?

I tried searching but couldn't find a place. In case there are no tests for that class, could you suggest a place to add the sample test mentioned in #43531?

As this is covered in default value creation for record fields, we didn't add specific tests.
You can add unit tests here - https://github.com/ballerina-platform/ballerina-lang/blob/master/tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/record/RecordWithClosureTest.java

Copy link

codecov bot commented Nov 11, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.56%. Comparing base (06e7fa7) to head (6a957cc).
Report is 392 commits behind head on master.

Files with missing lines Patch % Lines
...ompiler/bir/optimizer/BIRRecordValueOptimizer.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #43563      +/-   ##
============================================
- Coverage     77.56%   77.56%   -0.01%     
  Complexity    58741    58741              
============================================
  Files          3448     3448              
  Lines        219678   219679       +1     
  Branches      28918    28919       +1     
============================================
- Hits         170394   170391       -3     
- Misses        39892    39897       +5     
+ Partials       9392     9391       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Thushara-Piyasekara
Copy link
Contributor Author

The 1 line that is missing coverage is not a change done in this PR. @rdulmina do we have to add tests for that as well?

Also could you try rerunning the stdlib level 6? It is passing in other PRs now. Whatever was failing in that repo might be fixed now.

Copy link
Contributor

@HindujaB HindujaB left a comment

Choose a reason for hiding this comment

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

LGTM

@rdulmina rdulmina merged commit f1c60dc into ballerina-platform:master Nov 28, 2024
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: JvmCodeGenUtil reaches error while generating method for the below sample
3 participants