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

Jump to bottom

Fix issue with global variable as default value for class field and function parameter #40774

Merged
merged 9 commits into from
Aug 15, 2023
Merged

Fix issue with global variable as default value for class field and function parameter #40774

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f6c3695
Update closure variable logic
chiranSachintha Jun 19, 2023
c7764ca
Add unit tests
chiranSachintha Jun 20, 2023
116537a
Fix review suggestions
chiranSachintha Aug 1, 2023
9cbd31d
Merge branch 'master' of https://github.com/ballerina-platform/baller…
chiranSachintha Aug 1, 2023
95ed36b
Fix review suggestions
chiranSachintha Aug 14, 2023
86d73de
Fix review suggestions
chiranSachintha Aug 14, 2023
78ef046
Fix review suggestions
chiranSachintha Aug 14, 2023
0c23902
Fix review suggestions
chiranSachintha Aug 15, 2023
b58a604
Refactor tests
chiranSachintha Aug 15, 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
2 changes: 1 addition & 1 deletion ...allerina-lang/src/main/java/org/wso2/ballerinalang/compiler/desugar/ClosureGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1209,7 +1209,7 @@ public void visit(BLangSimpleVarRef.BLangLocalVarRef localVarRef) {
private void updateClosureVariable(BVarSymbol varSymbol, BLangInvokableNode encInvokable, Location pos) {
Set<Flag> flagSet = encInvokable.flagSet;
boolean isClosure = !flagSet.contains(Flag.QUERY_LAMBDA) && flagSet.contains(Flag.LAMBDA) &&
!flagSet.contains(Flag.ATTACHED);
!flagSet.contains(Flag.ATTACHED) && varSymbol.owner.tag != SymTag.PACKAGE;
if (!varSymbol.closure && isClosure) {
SymbolEnv encInvokableEnv = findEnclosingInvokableEnv(env, encInvokable);
BSymbol resolvedSymbol =
Expand Down
15 changes: 13 additions & 2 deletions tests/jballerina-unit-test/src/test/java/org/ballerinalang/test/object/ObjectTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,13 @@ public class ObjectTest {

private CompileResult checkInInitializerResult;
private CompileResult checkFunctionReferencesResult;
private CompileResult checkObjectWithDefaultValuesResult;

@BeforeClass
public void setUp() {
checkInInitializerResult = BCompileUtil.compile("test-src/object/object_field_initializer_with_check.bal");
checkFunctionReferencesResult = BCompileUtil.compile("test-src/object/object_function_pointer.bal");
checkObjectWithDefaultValuesResult = BCompileUtil.compile("test-src/object/object-with-defaultable-field.bal");
}

@Test(description = "Test Basic object as struct")
Expand Down Expand Up @@ -127,8 +129,7 @@ public void testObjectWithSimpleInit() {

@Test(description = "Test object with defaultable field in init function")
public void testObjectWithDefaultableField() {
CompileResult compileResult = BCompileUtil.compile("test-src/object/object-with-defaultable-field.bal");
BArray returns = (BArray) BRunUtil.invoke(compileResult, "testObjectWithSimpleInit");
BArray returns = (BArray) BRunUtil.invoke(checkObjectWithDefaultValuesResult, "testObjectWithSimpleInit");

Assert.assertEquals(returns.size(), 4);
Assert.assertSame(returns.get(0).getClass(), Long.class);
Expand Down Expand Up @@ -941,6 +942,16 @@ public void testNonPublicSymbolsWarningInServiceClass() {
Assert.assertEquals(result.getDiagnostics().length, 0);
}

@Test
public void testClassWithModuleDefaultValue() {
Copy link
Member

Choose a reason for hiding this comment

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

What does module default value mean?

Suggested change
public void testClassWithModuleDefaultValue() {
public void testClassWithModuleLevelVarAsDefaultValue() {

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. I am trying to specify this test case using a module variable as a default value.

BRunUtil.invoke(checkObjectWithDefaultValuesResult, "testClassWithModuleDefaultValue");
}

@Test
public void testObjectWithModuleDefaultValue() {
BRunUtil.invoke(checkObjectWithDefaultValuesResult, "testObjectWithModuleDefaultValue");
}

@AfterClass
public void tearDown() {
checkFunctionReferencesResult = null;
Expand Down
40 changes: 40 additions & 0 deletions ...jballerina-unit-test/src/test/resources/test-src/object/object-with-defaultable-field.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,43 @@ class Person {
self.month = val1;
}
}

final int classI = 111222;

class ModuleVariableReferencingClass {
int i = classI;
}

function value(int k = classI) returns int {
return k;
}

ModuleVariableReferencingClass c1 = new;

function testClassWithModuleDefaultValue() {
ModuleVariableReferencingClass c = new;
assertEquality(111222, c.i);
assertEquality(111222, c1.i);
}

function testObjectWithModuleDefaultValue() {
var value = object {
int i = classI;
};
assertEquality(111222, value.i);
}

const ASSERTION_ERROR_REASON = "AssertionError";

function assertEquality(anydata expected, anydata actual) {
if expected is anydata && actual is anydata && expected == actual {
Copy link
Member

Choose a reason for hiding this comment

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

Don't you get warnings for the checks now? Always true?

Copy link
Member

Choose a reason for hiding this comment

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

Please update the logic, removing unnecessary checks.

Copy link
Member Author

Choose a reason for hiding this comment

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

didn't get the warning.

Copy link
Member

Choose a reason for hiding this comment

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

Hint rather. Didn't you get unnecessary condition: expression will always evaluate to 'true'?

return;
}

if expected === actual {
return;
}

panic error(ASSERTION_ERROR_REASON,
message = "expected '" + expected.toString() + "', found '" + actual.toString () + "'");
}
Loading