ballerina-lang icon indicating copy to clipboard operation
ballerina-lang copied to clipboard

Implement list type as semtype in compiler

Open heshanpadmasiri opened this issue 11 months ago • 2 comments

Purpose

$subject

Fixes #42289 Fixes #42425

Approach

Describe how you are implementing the solutions along with the design details.

Samples

Provide high-level details about the samples related to this feature.

Remarks

List any other known issues, related PRs, TODO items, or any other notes related to the PR.

Check List

  • [x] Read the Contributing Guide
  • [ ] Updated Change Log
  • [ ] Checked Tooling Support (#<Issue Number>)
  • [ ] 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

heshanpadmasiri avatar Mar 24 '24 11:03 heshanpadmasiri

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Mar 28 '24 15:03 CLAassistant

Codecov Report

Attention: Patch coverage is 82.75862% with 180 lines in your changes are missing coverage. Please review.

Project coverage is 76.56%. Comparing base (0a7e4c9) to head (a83993a). Report is 646 commits behind head on nutcracker.

:exclamation: Current head a83993a differs from pull request most recent head 7eb27b6. Consider uploading reports for the commit 7eb27b6 to get more accurate results

Files Patch % Lines
.../java/io/ballerina/types/typeops/BddCommonOps.java 73.01% 16 Missing and 1 partial :warning:
...emtypes/src/main/java/io/ballerina/types/Core.java 77.46% 11 Missing and 5 partials :warning:
...llerinalang/compiler/bir/writer/BIRTypeWriter.java 73.58% 10 Missing and 4 partials :warning:
...main/java/io/ballerina/types/typeops/ListProj.java 62.16% 13 Missing and 1 partial :warning:
semtypes/src/main/java/io/ballerina/types/Env.java 59.37% 11 Missing and 2 partials :warning:
...c/main/java/io/ballerina/types/CellAtomicType.java 35.29% 6 Missing and 5 partials :warning:
...g/wso2/ballerinalang/compiler/desugar/Desugar.java 68.96% 9 Missing :warning:
...llerinalang/compiler/semantics/analyzer/Types.java 83.33% 8 Missing :warning:
...ng/compiler/semantics/analyzer/SymbolResolver.java 73.07% 7 Missing :warning:
...ang/compiler/semantics/model/types/BTupleType.java 84.09% 5 Missing and 2 partials :warning:
... and 23 more
Additional details and impacted files
@@                Coverage Diff                @@
##             nutcracker   #42397       +/-   ##
=================================================
+ Coverage          0.00%   76.56%   +76.56%     
- Complexity            0    54038    +54038     
=================================================
  Files                 9     2941     +2932     
  Lines                35   203802   +203767     
  Branches              0    26688    +26688     
=================================================
+ Hits                  0   156039   +156039     
- Misses               35    39093    +39058     
- Partials              0     8670     +8670     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 29 '24 01:03 codecov[bot]

@lochana-chathura I had to override the semtype() in BAnyType since people can (and evidently do in the case of BAnyType) simply update the public flags field, to make the type readonly after creating an instance of the type as not readonly. So the SemType we set at the time of construction is no longer valid. In retrospect we may need to do this everywhere since in theory it is possible for every type (though I saw this only when trying to build http module with any type). Do you think this is something we should address now or figure out a better workaround later since we don't have any other breaking cases? I think we can do this by not calculating the semtype at construction type and always calculating it when calling semtype()

heshanpadmasiri avatar Apr 09 '24 02:04 heshanpadmasiri

LGTM. Can merge after a rebase.

lochana-chathura avatar Apr 18 '24 05:04 lochana-chathura