From cb2a9e0ab1932521d055fae14d465383e123cb7c Mon Sep 17 00:00:00 2001 From: ravinperera00 Date: Mon, 10 Jun 2024 21:05:04 +0530 Subject: [PATCH 1/5] Add logic to support unshift operation at runtime Add a check to test if existing members can be rearranged without breaking inherent type Fix a bug that caused an index out of bound error in arraycopy Add test cases to cover newly added logic --- .../internal/values/TupleValueImpl.java | 19 +++++- .../ballerinalang/langlib/array/Unshift.java | 4 -- .../langlib/test/LangLibArrayTest.java | 1 + .../test/resources/test-src/arraylib_test.bal | 66 +++++++++++++++++++ 4 files changed, 84 insertions(+), 6 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java index d0939c5f7158..6f1d64f048e2 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java @@ -737,6 +737,7 @@ protected void checkFixedLength(long length) { @Override protected void unshift(long index, Object[] vals) { handleImmutableArrayValue(); + validateInherentTypeOfExistingMembers(index, vals.length); unshiftArray(index, vals.length, getCurrentArrayLength()); addToRefArray(vals, (int) index); } @@ -806,7 +807,8 @@ private void addToRefArray(Object[] vals, int startIndex) { } private void unshiftArray(long index, int unshiftByN, int arrLength) { - int lastIndex = size() + unshiftByN - 1; + int currSize = size(); + int lastIndex = currSize + unshiftByN - 1; prepareForConsecutiveMultiAdd(lastIndex, arrLength); if (index > lastIndex) { throw ErrorHelper.getRuntimeException( @@ -815,7 +817,7 @@ private void unshiftArray(long index, int unshiftByN, int arrLength) { } int i = (int) index; - System.arraycopy(this.refValues, i, this.refValues, i + unshiftByN, this.size - i); + System.arraycopy(this.refValues, i, this.refValues, i + unshiftByN, currSize - i); } private int getCurrentArrayLength() { @@ -827,4 +829,17 @@ private void resetSize(int index) { size = index + 1; } } + + private void validateInherentTypeOfExistingMembers(long index, int offset) { + Type targetType; + for (long i = index; i < this.size; i++) { + targetType = i + offset >= this.tupleType.getTupleTypes().size() ? + this.tupleType.getRestType() : this.tupleType.getTupleTypes().get((int) (i + offset)); + if (!TypeChecker.checkIsType(this.getRefValue(i), targetType)) { + throw ErrorHelper.getRuntimeException( + getModulePrefixedReason(ARRAY_LANG_LIB, INHERENT_TYPE_VIOLATION_ERROR_IDENTIFIER), + ErrorCodes.INCOMPATIBLE_TYPE, TypeChecker.getType(this.getRefValue(i)), targetType); + } + } + } } diff --git a/langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Unshift.java b/langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Unshift.java index d4c1e1e21680..d02c83e31335 100644 --- a/langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Unshift.java +++ b/langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Unshift.java @@ -18,11 +18,8 @@ package org.ballerinalang.langlib.array; -import io.ballerina.runtime.api.utils.TypeUtils; import io.ballerina.runtime.api.values.BArray; -import static org.ballerinalang.langlib.array.utils.ArrayUtils.checkIsArrayOnlyOperation; - /** * Native implementation of lang.array:unshift((any|error)[], (any|error)...). * @@ -31,7 +28,6 @@ public class Unshift { public static void unshift(BArray arr, Object... vals) { - checkIsArrayOnlyOperation(TypeUtils.getImpliedType(arr.getType()), "unshift()"); arr.unshift(vals); } } diff --git a/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibArrayTest.java b/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibArrayTest.java index 8b7a128b0f10..f8dc94bbf0e0 100644 --- a/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibArrayTest.java +++ b/langlib/langlib-test/src/test/java/org/ballerinalang/langlib/test/LangLibArrayTest.java @@ -552,6 +552,7 @@ public Object[] testFunctions() { "testLastIndexOf", "testPush", "testShiftOperation", + "testUnshiftOperation", "testSort1", "testSort2", "testSort4", diff --git a/langlib/langlib-test/src/test/resources/test-src/arraylib_test.bal b/langlib/langlib-test/src/test/resources/test-src/arraylib_test.bal index 00e2d51077f1..94d83ef33041 100644 --- a/langlib/langlib-test/src/test/resources/test-src/arraylib_test.bal +++ b/langlib/langlib-test/src/test/resources/test-src/arraylib_test.bal @@ -707,6 +707,72 @@ function testShiftOnTupleWithoutValuesForRestParameter() { assertValueEquality("shift() not supported on type '[int,int...]'", detailMessage); } +function testUnshiftOperation() { + testUnshiftTuplesPositives(); + testUnshiftTuplesNegatives(); +} + +function testUnshiftTuplesPositives() { + [int, int...] tuple1 = []; + [int, int, int...] tuple3 = [1, 3, 4, 5, 7]; + [int, int...] list = []; + + tuple1.unshift(5, 6, 7, 8); + tuple3.unshift(12, 67); + foreach int i in 0 ..< 400 { + list.unshift(i); + } + + assertValueEquality([5, 6, 7, 8, 0], tuple1); + assertValueEquality([12, 67, 1, 3, 4, 5, 7], tuple3); + assertValueEquality(401, list.length()); + assertValueEquality(399, list[0]); + assertValueEquality(199, list[200]); + assertValueEquality(0, list[399]); + assertValueEquality(0, list[400]); +} + +function testUnshiftTuplesNegatives() { + [boolean, string...] tuple1 = []; + [int, boolean, string...] tuple2 = [8, false, "hello"]; + [int, boolean, string...] tuple3 = []; + + var fn1 = function() { + tuple1.unshift("hello"); + }; + + var fn2 = function() { + tuple2.unshift(false, 78, "world"); + }; + + var fn3 = function() { + tuple3.unshift(10, false, "Hello", "World"); + }; + + error? res1 = trap fn1(); + error? res2 = trap fn2(); + error? res3 = trap fn3(); + assertTrue(res1 is error); + assertTrue(res2 is error); + assertTrue(res3 is error); + + error err1 = res1; + error err2 = res2; + error err3 = res3; + var message1 = err1.detail()["message"]; + var message2 = err2.detail()["message"]; + var message3 = err3.detail()["message"]; + string detailMessage1 = message1 is error ? message1.toString() : message1.toString(); + string detailMessage2 = message2 is error ? message2.toString() : message2.toString(); + string detailMessage3 = message3 is error ? message3.toString() : message3.toString(); + assertValueEquality("{ballerina/lang.array}InherentTypeViolation", err1.message()); + assertValueEquality("incompatible types: expected 'boolean', found 'string'", detailMessage1); + assertValueEquality("{ballerina/lang.array}InherentTypeViolation", err2.message()); + assertValueEquality("incompatible types: expected 'int', found 'string'", detailMessage2); + assertValueEquality("{ballerina/lang.array}InherentTypeViolation", err3.message()); + assertValueEquality("incompatible types: expected 'int', found 'string'", detailMessage3); +} + type Student record {| int id; string? fname; From 1ce03d1d06b73799e855ee1ad2bd76aaeee937d1 Mon Sep 17 00:00:00 2001 From: ravinperera00 Date: Wed, 12 Jun 2024 13:12:50 +0530 Subject: [PATCH 2/5] Add tests with records --- .../test/resources/test-src/arraylib_test.bal | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/langlib/langlib-test/src/test/resources/test-src/arraylib_test.bal b/langlib/langlib-test/src/test/resources/test-src/arraylib_test.bal index 94d83ef33041..81df1eef46fb 100644 --- a/langlib/langlib-test/src/test/resources/test-src/arraylib_test.bal +++ b/langlib/langlib-test/src/test/resources/test-src/arraylib_test.bal @@ -707,6 +707,8 @@ function testShiftOnTupleWithoutValuesForRestParameter() { assertValueEquality("shift() not supported on type '[int,int...]'", detailMessage); } +type UnshiftType1 record {string val1; int val2;}; +type UnshiftType2 record {string val1;}; function testUnshiftOperation() { testUnshiftTuplesPositives(); testUnshiftTuplesNegatives(); @@ -716,12 +718,14 @@ function testUnshiftTuplesPositives() { [int, int...] tuple1 = []; [int, int, int...] tuple3 = [1, 3, 4, 5, 7]; [int, int...] list = []; + [UnshiftType1, UnshiftType1...] tuple4 = [{val1:"Hello", val2:67}]; tuple1.unshift(5, 6, 7, 8); tuple3.unshift(12, 67); foreach int i in 0 ..< 400 { list.unshift(i); } + tuple4.unshift({val1:"world", val2:82}); assertValueEquality([5, 6, 7, 8, 0], tuple1); assertValueEquality([12, 67, 1, 3, 4, 5, 7], tuple3); @@ -730,12 +734,14 @@ function testUnshiftTuplesPositives() { assertValueEquality(199, list[200]); assertValueEquality(0, list[399]); assertValueEquality(0, list[400]); + assertValueEquality([{val1:"world", val2:82}, {val1:"Hello", val2:67}], tuple4); } function testUnshiftTuplesNegatives() { [boolean, string...] tuple1 = []; [int, boolean, string...] tuple2 = [8, false, "hello"]; [int, boolean, string...] tuple3 = []; + [UnshiftType1, UnshiftType2...] tuple4 = [{val1:"Hello", val2:67}]; var fn1 = function() { tuple1.unshift("hello"); @@ -749,28 +755,39 @@ function testUnshiftTuplesNegatives() { tuple3.unshift(10, false, "Hello", "World"); }; + var fn4 = function() { + tuple4.unshift({val1:"world"}); + }; + error? res1 = trap fn1(); error? res2 = trap fn2(); error? res3 = trap fn3(); + error? res4 = trap fn4(); assertTrue(res1 is error); assertTrue(res2 is error); assertTrue(res3 is error); + assertTrue(res4 is error); error err1 = res1; error err2 = res2; error err3 = res3; + error err4 = res4; var message1 = err1.detail()["message"]; var message2 = err2.detail()["message"]; var message3 = err3.detail()["message"]; + var message4 = err4.detail()["message"]; string detailMessage1 = message1 is error ? message1.toString() : message1.toString(); string detailMessage2 = message2 is error ? message2.toString() : message2.toString(); string detailMessage3 = message3 is error ? message3.toString() : message3.toString(); + string detailMessage4 = message4 is error ? message4.toString() : message4.toString(); assertValueEquality("{ballerina/lang.array}InherentTypeViolation", err1.message()); assertValueEquality("incompatible types: expected 'boolean', found 'string'", detailMessage1); assertValueEquality("{ballerina/lang.array}InherentTypeViolation", err2.message()); assertValueEquality("incompatible types: expected 'int', found 'string'", detailMessage2); assertValueEquality("{ballerina/lang.array}InherentTypeViolation", err3.message()); assertValueEquality("incompatible types: expected 'int', found 'string'", detailMessage3); + assertValueEquality("{ballerina/lang.array}InherentTypeViolation", err4.message()); + assertValueEquality("incompatible types: expected 'UnshiftType1', found 'UnshiftType2'", detailMessage4); } type Student record {| From dced2a0275f713d7245f3efc0d035a16830df37c Mon Sep 17 00:00:00 2001 From: Ravin Perera <62183347+ravinperera00@users.noreply.github.com> Date: Wed, 19 Jun 2024 10:29:09 +0530 Subject: [PATCH 3/5] Apply suggestions from code review Co-authored-by: Gabilan Ganeshwaran --- .../io/ballerina/runtime/internal/values/TupleValueImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java index 6f1d64f048e2..c112a8778384 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java @@ -833,7 +833,7 @@ private void resetSize(int index) { private void validateInherentTypeOfExistingMembers(long index, int offset) { Type targetType; for (long i = index; i < this.size; i++) { - targetType = i + offset >= this.tupleType.getTupleTypes().size() ? + targetType = (i + offset >= this.tupleType.getTupleTypes().size()) ? this.tupleType.getRestType() : this.tupleType.getTupleTypes().get((int) (i + offset)); if (!TypeChecker.checkIsType(this.getRefValue(i), targetType)) { throw ErrorHelper.getRuntimeException( From 3f43158947631a96d023aac5448ee39f879e7242 Mon Sep 17 00:00:00 2001 From: ravinperera00 Date: Wed, 19 Jun 2024 16:33:01 +0530 Subject: [PATCH 4/5] Rafactor the code --- .../io/ballerina/runtime/internal/values/TupleValueImpl.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java index c112a8778384..d6dd5b3485b5 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java @@ -836,8 +836,8 @@ private void validateInherentTypeOfExistingMembers(long index, int offset) { targetType = (i + offset >= this.tupleType.getTupleTypes().size()) ? this.tupleType.getRestType() : this.tupleType.getTupleTypes().get((int) (i + offset)); if (!TypeChecker.checkIsType(this.getRefValue(i), targetType)) { - throw ErrorHelper.getRuntimeException( - getModulePrefixedReason(ARRAY_LANG_LIB, INHERENT_TYPE_VIOLATION_ERROR_IDENTIFIER), + throw ErrorHelper.getRuntimeException(getModulePrefixedReason(ARRAY_LANG_LIB, + INHERENT_TYPE_VIOLATION_ERROR_IDENTIFIER), ErrorCodes.INCOMPATIBLE_TYPE, TypeChecker.getType(this.getRefValue(i)), targetType); } } From 3f004a3016ac8f443b32dc4eb61ab9f314df3e94 Mon Sep 17 00:00:00 2001 From: ravinperera00 Date: Fri, 5 Jul 2024 13:37:52 +0530 Subject: [PATCH 5/5] Change type of index from long to int --- .../ballerina/runtime/internal/values/TupleValueImpl.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java index ae536a64e228..0a38e9430898 100644 --- a/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java +++ b/bvm/ballerina-runtime/src/main/java/io/ballerina/runtime/internal/values/TupleValueImpl.java @@ -738,7 +738,7 @@ protected void checkFixedLength(long length) { @Override protected void unshift(long index, Object[] vals) { handleImmutableArrayValue(); - validateInherentTypeOfExistingMembers(index, vals.length); + validateInherentTypeOfExistingMembers((int) index, vals.length); unshiftArray(index, vals.length, getCurrentArrayLength()); addToRefArray(vals, (int) index); } @@ -831,11 +831,11 @@ private void resetSize(int index) { } } - private void validateInherentTypeOfExistingMembers(long index, int offset) { + private void validateInherentTypeOfExistingMembers(int index, int offset) { Type targetType; - for (long i = index; i < this.size; i++) { + for (int i = index; i < this.size; i++) { targetType = (i + offset >= this.tupleType.getTupleTypes().size()) ? - this.tupleType.getRestType() : this.tupleType.getTupleTypes().get((int) (i + offset)); + this.tupleType.getRestType() : this.tupleType.getTupleTypes().get(i + offset); if (!TypeChecker.checkIsType(this.getRefValue(i), targetType)) { throw ErrorHelper.getRuntimeException(getModulePrefixedReason(ARRAY_LANG_LIB, INHERENT_TYPE_VIOLATION_ERROR_IDENTIFIER),