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

Fix checkcast generation for bundled function args #42505

Merged
merged 4 commits into from
Apr 25, 2024
Merged

Fix checkcast generation for bundled function args #42505

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
839b92b
Fix generate cast for bundled function args
gabilang Apr 8, 2024
39b18d6
Generalize function params bundling logic
gabilang Apr 9, 2024
47fa779
Fix graphql test failure
gabilang Apr 9, 2024
7248446
Fix clashes with rest-param mapping
gabilang Apr 18, 2024
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
15 changes: 11 additions & 4 deletions ...rina-lang/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/JvmTerminatorGen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1430,16 +1430,17 @@ private void storeToVar(BIRNode.BIRVariableDcl varDcl) {
private void genResourcePathArgs(List<BIROperand> pathArgs) {
int pathVarArrayIndex = this.indexMap.addIfNotExists("$pathVarArray", symbolTable.anyType);
int bundledArrayIndex = this.indexMap.addIfNotExists("$pathArrayArgs", symbolTable.anyType);
genBundledArgs(pathArgs, pathVarArrayIndex, bundledArrayIndex, TYPE_ANYDATA_ARRAY);
genBundledArgs(pathArgs, pathVarArrayIndex, bundledArrayIndex, TYPE_ANYDATA_ARRAY, true);
}

private void genBundledFunctionArgs(List<BIROperand> args) {
int functionArgArrayIndex = this.indexMap.addIfNotExists("$functionArgArray", symbolTable.anyType);
int bundledArrayIndex = this.indexMap.addIfNotExists("$functionArrayArgs", symbolTable.anyType);
genBundledArgs(args, functionArgArrayIndex, bundledArrayIndex, TYPE_ANY_ARRAY);
genBundledArgs(args, functionArgArrayIndex, bundledArrayIndex, TYPE_ANY_ARRAY, false);
}

private void genBundledArgs(List<BIROperand> args, int argsArrayIndex, int bundledArrayIndex, String fieldName) {
private void genBundledArgs(List<BIROperand> args, int argsArrayIndex, int bundledArrayIndex, String fieldName,
boolean isFromPathArgs) {
mv.visitLdcInsn((long) args.size());
mv.visitInsn(L2I);
mv.visitTypeInsn(ANEWARRAY, OBJECT);
Expand All @@ -1451,7 +1452,13 @@ private void genBundledArgs(List<BIROperand> args, int argsArrayIndex, int bundl
mv.visitLdcInsn((long) i);
mv.visitInsn(L2I);
this.loadVar(arg.variableDcl);
jvmCastGen.generateCheckCastToAnyData(mv, arg.variableDcl.type);
if (isFromPathArgs) {
// Add CheckCast instruction for path args.
jvmCastGen.generateCheckCastToAnyData(mv, arg.variableDcl.type);
} else {
// Add Box instruction if the type is a value type.
jvmCastGen.addBoxInsn(mv, arg.variableDcl.type);
}
mv.visitInsn(AASTORE);
i++;
}
Expand Down
32 changes: 17 additions & 15 deletions ...ng/src/main/java/org/wso2/ballerinalang/compiler/bir/codegen/interop/JMethodResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,15 @@
import org.wso2.ballerinalang.compiler.bir.codegen.JvmCodeGenUtil;
import org.wso2.ballerinalang.compiler.semantics.model.SymbolTable;
import org.wso2.ballerinalang.compiler.semantics.model.symbols.BVarSymbol;
import org.wso2.ballerinalang.compiler.semantics.model.symbols.Symbols;
import org.wso2.ballerinalang.compiler.semantics.model.types.BArrayType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BFiniteType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BTypeReferenceType;
import org.wso2.ballerinalang.compiler.semantics.model.types.BUnionType;
import org.wso2.ballerinalang.compiler.tree.expressions.BLangExpression;
import org.wso2.ballerinalang.compiler.util.TypeTags;
import org.wso2.ballerinalang.util.Flags;

import java.lang.reflect.Constructor;
import java.lang.reflect.Executable;
Expand Down Expand Up @@ -238,19 +240,17 @@ private boolean hasEquivalentPathParamCount(JMethodRequest jMethodRequest, JMeth
}

private boolean hasEquivalentFunctionParamCount(JMethodRequest jMethodRequest, JMethod jMethod) {
// Currently, this is only applicable for resource and remote methods which have empty path parameters.
// If path parameters are present, this will be handled by 'hasEquivalentPathAndFunctionParamCount' method
// by bundling both.
if (jMethodRequest.receiverType == null || jMethodRequest.pathParamCount != 0
|| jMethodRequest.bParamTypes.length == 0) {
return false;
}
// This is only applicable for resource and remote methods which have at least one function
// parameter other than path parameters and the bundling of path parameters is not required.
Class<?>[] paramTypes = jMethod.getParamTypes();
boolean isFirstParamServiceObject = jMethodRequest.bParamTypes[0].tag == TypeTags.SERVICE;
// Get the count of parameters of the resource/remote methods by excluding the receiver type.
int count = isFirstParamServiceObject ? paramTypes.length - 1 : paramTypes.length;
int reducedParamCount = isFirstParamServiceObject ? 2 : 1;
if (count < reducedParamCount || count > reducedParamCount + 2) {
int count = paramTypes.length;
int reducedParamCount = jMethodRequest.pathParamCount + 1;
int functionParamCount = jMethodRequest.bFuncParamCount - jMethodRequest.pathParamCount;
// TODO: Remove 'Symbols.isFlagOn(jMethodRequest.bParamTypes[0].flags, Flags.SERVICE)' check after fixing
// https://github.com/ballerina-platform/ballerina-lang/issues/42456.
if (jMethodRequest.receiverType == null || functionParamCount < 1
|| count < reducedParamCount || count > reducedParamCount + 2
|| Symbols.isFlagOn(jMethodRequest.bParamTypes[0].flags, Flags.SERVICE)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to remove this check once addressed the issue #42456. The logic used to bundle the parameters clashes with following use cases without the introduced check. https://github.com/ballerina-platform/module-ballerina-graphql/blob/8bd564d7d2eb2513a377bc58cc5d94a7d762803d/ballerina/engine.bal#L427 Anyway, Specifying the parameter annotation will resolve the conflicts.

return false;
}
if (!isParamAssignableToBArray(paramTypes[count - 1])
Expand All @@ -263,10 +263,12 @@ private boolean hasEquivalentFunctionParamCount(JMethodRequest jMethodRequest, J
// This is for object interop functions when self is passed as a parameter
jMethod.setReceiverType(jMethodRequest.receiverType);
return true;
} else if (count == (reducedParamCount + 2)) {
// This is for object interop functions when both BalEnv and self is passed as parameters.
jMethod.setReceiverType(jMethodRequest.receiverType);
return jMethod.isBalEnvAcceptingMethod();
}
// This is for object interop functions when both BalEnv and self is passed as parameters.
jMethod.setReceiverType(jMethodRequest.receiverType);
return jMethod.isBalEnvAcceptingMethod();
return false;
}

private boolean isParamAssignableToBArray(Class<?> paramType) {
Expand Down
20 changes: 20 additions & 0 deletions ...ballerina-unit-test/src/test/resources/test-src/javainterop/basic/static_method_tests.bal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,22 @@ public isolated client class ClientObj {
isolated function getResourceMethod(service object {} serviceObject, string[] path) returns anydata = @java:Method {
'class: "org/ballerinalang/nativeimpl/jvm/tests/StaticMethods"
} external;

resource isolated function get albums/[int id](Person person, string s, typedesc<any> targetType = <>) returns
targetType|error = @java:Method {
'class: "org/ballerinalang/nativeimpl/jvm/tests/StaticMethods",
name: "getResource",
paramTypes: ["io.ballerina.runtime.api.Environment", "io.ballerina.runtime.api.values.BObject",
"io.ballerina.runtime.api.values.BArray", "io.ballerina.runtime.api.values.BArray"]
} external;

resource isolated function get albums(Person person, string s, typedesc<any> targetType = <>) returns
targetType|error = @java:Method {
'class: "org/ballerinalang/nativeimpl/jvm/tests/StaticMethods",
name: "getResource",
paramTypes: ["io.ballerina.runtime.api.Environment", "io.ballerina.runtime.api.values.BObject",
"io.ballerina.runtime.api.values.BArray"]
} external;
}

public function testBundleFuncArgsToBArray() returns error? {
Expand All @@ -527,4 +543,8 @@ public function testBundleFuncArgsToBArray() returns error? {
test:assertEquals(res, 5);
res = cl.getResourceMethod(isolated service object {}, ["orderitem", "1234", "abcd"]);
test:assertEquals(res, 1000);
res = check cl->/albums/[1](new Person(29), "123");
test:assertEquals(res, 5);
res = check cl->/albums(new Person(29), "123");
test:assertEquals(res, 10);
}
Loading