Initial Handling of Non-Void Java Method Calls in GnuCOBOL#174
Initial Handling of Non-Void Java Method Calls in GnuCOBOL#174xevor11 wants to merge 8 commits intoOCamlPro:java-interopfrom
Conversation
Changes mostly include:
* New option `--with{out}-java` for `configure` script
* Runtime support for calling `static void (void)` methods
* Lookup of JVM library when JAVA_HOME is set, and pre-load when
possible
* Two preliminary tests
Co-authored-by: Nicolas Berthier <nicolas.berthier@ocamlpro.com>
Co-authored-by: Simon Sobisch <simonsobisch@web.de>
cobc/codegen.c
Outdated
| char *last_dot; | ||
| char *method_name; | ||
| const char *class_name; | ||
| char method_signature[256] = "("; |
There was a problem hiding this comment.
For internal buffer sizes you have COB_*_BUFF definitions in libcob/common.h that you can use here as well. Typical signature length could exceed 256 without much effort I think (with long-enough Java package names). I'd go for COB_NORMAL_BUFF (2048) for now.
cobc/codegen.c
Outdated
| method_name = last_dot + 1; | ||
| class_name = class_and_method_name; | ||
|
|
||
| for(ptr = p->args; ptr != NULL; ptr = ptr->next) { |
There was a problem hiding this comment.
Note: indentation is strange here. I think you need to adjust that in your settings for this project.
cobc/codegen.c
Outdated
| output_prefix(); | ||
| output_line("call_java_%s = ", mangled); | ||
| output("cob_resolve_java(\"%s\", \"%s\", \"()V\");", class_name, method_name); | ||
| output("cob_resolve_java(\"%s\", \"%s\", \"()V\");", class_name, method_name, method_signature); |
There was a problem hiding this comment.
Here you should probable replace ()V with %s so the method signature is included.
cobc/codegen.c
Outdated
| output_line("cob_call_java(call_java_%s);\n", mangled); | ||
| output_newline(); | ||
| output_block_close(); | ||
| cobc_free(mangled); |
There was a problem hiding this comment.
This will likely cause a use-after-free since mangled is referenced in the list created by lookup_java_call.
cbb7903 to
1f90ff3
Compare
GitMensch
left a comment
There was a problem hiding this comment.
next bigger part here is likely adding a testcase that has a return value and a second that has parameters (or both one, for example a simple longJava.Test.addMe (int, int[]))
cobc/codegen.c
Outdated
| char return_type_signature[32]; | ||
| char method_signature[2048] = "("; |
There was a problem hiding this comment.
use COB_MINI_BUFF and COB_NORMAL_BUFF for those and down use strncat with the _MAX defines
cobc/codegen.c
Outdated
| cobc_err_msg(_("Unsupported array element type in Java method call")); | ||
| COBC_ABORT(); | ||
| } | ||
| } | ||
| } | ||
| list_elements = inner_list; | ||
| } | ||
|
|
||
| if (array_dimension > 2) { | ||
| cobc_err_msg(_("Unsupported array dimension: %d"), array_dimension); | ||
| COBC_ABORT(); |
There was a problem hiding this comment.
those two messages should be errors in cb_check_conformance as well as a check for "only field identifiers" (= no literals/functions/...) - which then also allows above to directly cast to a field pointer and operate on this; additional, for parameters passed BY REFERENCE that are a String/BigDecimal mapping item there should be a warning "immutable type implicit passed BY CONTENT".
| (*env)->ExceptionDescribe(env); | ||
| (*env)->ExceptionClear(env); |
There was a problem hiding this comment.
fine during development, needs to be adjusted later - see review on the other PR
cobc/codegen.c
Outdated
| case CB_TAG_INTEGER: | ||
| strcat(method_signature, "[I"); | ||
| break; |
There was a problem hiding this comment.
In case of integer we further have to check the type per usage
| pic/type | java type |
|---|---|
PIC S9(n) COMP-5, where ≥ 1 n ≤ 4 |
short |
PIC S9(n) COMP-5, where ≥ 5 n ≤ 9 |
int |
PIC S9(n) COMP-5, where ≥ 10 n ≤ 18 |
long |
usage COMP-3/PACKED-DECIMAL/DISPLAY numeric |
java.math.BigDecimal |
Note that checking the TAGs and USAGEs and CLASSes in a single switch doesn't work (check with the debugger to get more details).
Also note that PIC X (=single byte) is a special case:
- if it has two or one level88 below it with value x'01' / x'00' -->
boolean - otherwise:
byte
For all of the types you have (after addition of the above) there needs to be a check for the occurs attribute ("more than 1 dimension" will already be checked in the conformance checks) and if existing just add [].
As the valid types are identical for both arguments and returning items, their generation should be moved out to a static helper function called for each of the parameters (the loop here) and the returning item below.
No description provided.