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

Support record defaultable fields with closure values for runtime #40579

Conversation

HindujaB
Copy link
Contributor

@HindujaB HindujaB commented Jun 2, 2023

Purpose

Describe the problems, issues, or needs driving this feature/fix and include links to related issues.

Fixes #40805
Fixes #40214
Fixes #37351
Fixes #40943
Fixes #41234
Fixes #41378
Fixes #41219
Fixes #33771
Fixes #32012
Fixes #38558

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

  • 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

…ina-lang into closures-for-record-default-values

� Conflicts:
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/BIRPackageSymbolEnter.java
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/JvmDesugarPhase.java
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SemanticAnalyzer.java
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SymbolResolver.java
…ina-lang into closures-for-record-default-values

� Conflicts:
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/SemanticAnalyzer.java
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/util/ImmutableTypeCloner.java
�	tests/jballerina-debugger-integration-test/src/test/java/org/ballerinalang/debugger/test/adapter/variables/VariableVisibilityTest.java
�	tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/DependentlyTypedFunctionsTest.java
…ina-lang into closures-for-record-default-values

� Conflicts:
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/semantics/analyzer/TypeChecker.java
�	misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/evaluation/engine/action/RemoteMethodCallActionEvaluator.java
�	misc/debug-adapter/modules/debug-adapter-core/src/main/java/org/ballerinalang/debugadapter/utils/PackageUtils.java
…github.com/ballerina-platform/ballerina-lang into closures-for-record-default-values

� Conflicts:
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/Desugar.java
�	tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/javainterop/DependentlyTypedFunctionsTest.java
…ina-lang into closures-for-record-default-values

� Conflicts:
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/split/creators/JvmRecordCreatorGen.java
…ina-lang into closures-for-record-default-values

� Conflicts:
�	compiler/ballerina-lang/src/main/java/org/wso2/ballerinalang/compiler/tree/types/BLangObjectTypeNode.java
…ina-lang into closures-for-record-default-values

� Conflicts:
�	.github/workflows/pull_request_windows_build.yml
@chiranSachintha chiranSachintha force-pushed the closures-for-record-default-values branch from 4d8ac48 to e4d495a Compare November 1, 2023 04:57
- id: type_inclusions_count
type: s4
- id: type_inclusions_cp_index
type: s4
repeat: expr
repeat-expr: type_inclusions_count
- id: default_values
Copy link
Member

Choose a reason for hiding this comment

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

Shall we update the BIR version?

Copy link
Member

Choose a reason for hiding this comment

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

Updated f0dcb27

return !hasMissingRequiredFields;
}

private void findDefaultValuesFromEnclosingPackage(List<BLangTypeDefinition> typeDefinitions,
Copy link
Member

Choose a reason for hiding this comment

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

Ping on this.

@@ -319,6 +302,7 @@ public static void populateStructureFields(Types types, SymbolTable symTable,
long flag, boolean isImmutable) {
BTypeSymbol structureSymbol = structureType.tsymbol;
LinkedHashMap<String, BField> fields = new LinkedHashMap<>();
structureType.typeInclusions = origStructureType.typeInclusions;
Copy link
Member

Choose a reason for hiding this comment

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

Why did we need to add this? May have to rename the method too, since it does more than populate structure fields.

Copy link
Member

Choose a reason for hiding this comment

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

type Foo record {|
    int[] x = [1];
    int[] y = [];
|};

type Bar record {|
    *Foo;
    int[] & readonly x = [2];
|};

public function  main() {
    Bar & readonly b1 = {};
    io:println(b1);
}

When considering the above scenario , we need to create an immutable type for 'Bar.' When doing that, we need to include type inclusions into the newly generated type. For that I do the above change.

Copy link
Member

Choose a reason for hiding this comment

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

Change function name as populateStructureFieldsAndTypeInclusions

…ina-lang into closures-for-record-default-values

# Conflicts:
#	langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibValueTest.java
Copy link
Member

@MaryamZi MaryamZi left a comment

Choose a reason for hiding this comment

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

Approving based on several reviews of mostly the compiler changes and testing.

Copy link
Contributor

@Nadeeshan96 Nadeeshan96 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@warunalakshitha warunalakshitha left a comment

Choose a reason for hiding this comment

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

LGTM

@chiranSachintha chiranSachintha merged commit 5ae9d10 into ballerina-platform:master Dec 6, 2023
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team/jBallerina All the issues related to BIR, JVM backend code generation and runtime
Projects
None yet
7 participants