FWD » Core Development » Base Language

6129a/13888 adds runtime support for DYNAMIC-PROPERTY getter and setter. Not all argument validation is implemented.

Code Review Task Branch 6161a Revision 15222

The changes look good.

The only concern I have is about performance. In ObjectOps.validDynamicPropertyArgs() there will be 2 SourceNameMapper calls for scalar props and 5 for extent props. This plus additional context lookups and some reflection usage makes me worry that this will cause a noticable slowdown.

I've moved the bulk getter/setter to be resolved only if no indexed setter/getter is resolved at all. I'm not sure what else to do at this point.

I think we just watch it in case it becomes a hotspot. One solution would be to group all the details of a property into a single data structure that is returned on the first call. Then the details can be accessed as needed without further calls.

Greg Shah wrote:

What testing do you propose?

DYNAMIC-PROPERTY is not being used in other projects than #8433. Standalone tests are OK.

Branch 6161a was merged to trunk rev 15229 and archived.

Constantin Asofiei wrote:

Paul, please review the support for DYNAMIC-PROPERTY and what tests exist, and what needs to be added.

• When setting a simple BDT property the code sometimes converts to the java native type. This results in an error being thrown as there is no support for assigning java native types to a property (with DYNAMIC-PROPERTY statement). The only support there is, is expecting a BDT.
• There is no implementation for setting or getting extent properties (fixed nor dynamic)
• Wrong errors being thrown in some cases

The tests take into account BDT and objects types, scalar and extent (fixed and dynamic), getters and setters, static vs instance, and lastly access modifiers. I am not able to confirm the accuracy of the tests for static dynamic extent properties as there is no way to restore the state of those properties other than the instruction EXTENT(PropertyClass:dynamicExtentProperty) = ? which is not supported in FWD (no overload for new unknown() argument).

One thing to note is that #6490 introduces some support for fixed extent properties. While working on that feature I tested support for chained properties and as far as I know from what I remember they're supported.
With the current implementation the access modifiers, setters and getters for BDT properties (except for the cases described above) seem to work.

Paul Bodale wrote:

• When setting a simple BDT property the code sometimes converts to the java native type. This results in an error being thrown as there is no support for assigning java native types to a property (with DYNAMIC-PROPERTY statement). The only support there is, is expecting a BDT.

There is code in jobject.fromJava, you can refactor that and use it at runtime, to convert any non-BDT to a BDT (including arrays).

• There is no implementation for setting or getting extent properties (fixed nor dynamic)

Is anything fixed via #6490? We need to follow the same approach as for DYNAMIC-INVOKE. Please work on the extent issues after the problems with scalar properties are fixed.

The tests take into account BDT and objects types, scalar and extent (fixed and dynamic), getters and setters, static vs instance, and lastly access modifiers. I am not able to confirm the accuracy of the tests for static dynamic extent properties as there is no way to restore the state of those properties other than the instruction EXTENT(PropertyClass:dynamicExtentProperty) = ? which is not supported in FWD (no overload for new unknown() argument).

How does this convert? Please fix it.

Constantin Asofiei wrote:

There is code in jobject.fromJava, you can refactor that and use it at runtime, to convert any non-BDT to a BDT (including arrays).

Thanks for the tip. Committed rev. 15876 on branch 6161a which introduces support for setting a property via DYNAMIC-INVOKE when the lvalue converts to a Java native type.

Is anything fixed via #6490? We need to follow the same approach as for DYNAMIC-INVOKE. Please work on the extent issues after the problems with scalar properties are fixed.

I'm going to run some tests to be 100% sure but it looks like branch 6490a introduced support at least for setting extent properties. I'm going to patch this additional support from the last version of the branch and work out all the support on branch 6161a so that all support for DYNAMIC-PROPERTY would be on a single branch.

The tests take into account BDT and objects types, scalar and extent (fixed and dynamic), getters and setters, static vs instance, and lastly access modifiers. I am not able to confirm the accuracy of the tests for static dynamic extent properties as there is no way to restore the state of those properties other than the instruction EXTENT(PropertyClass:dynamicExtentProperty) = ? which is not supported in FWD (no overload for new unknown() argument).

How does this convert? Please fix it.

EXTENT(tests.base_language.builtin_functions.support.Props:intDynExtSt) = ?.
EXTENT(tests.base_language.builtin_functions.support.Props:objDynExtSt) = ?.

Props.resizeIntDynExtSt(new unknown());
Props.resizeObjDynExtSt(new unknown());

This is the compiler output:

    [javac] /home/pbb/gcd/testcases/src/com/goldencode/testcases/tests/base_language/builtin_functions/accepted_params/TestSupportDynPrp.java:65: error: incompatible types: unknown cannot be converted to int64
    [javac]          Props.resizeIntDynExtSt(new unknown());
    [javac]                                  ^
    [javac] /home/pbb/gcd/testcases/src/com/goldencode/testcases/tests/base_language/builtin_functions/accepted_params/TestSupportDynPrp.java:66: error: incompatible types: unknown cannot be converted to int64
    [javac]          Props.resizeObjDynExtSt(new unknown());
    [javac]                                  ^
    [javac] Note: /home/pbb/gcd/testcases/src/com/goldencode/testcases/tests/base_language/builtin_functions/support/Props.java uses unchecked or unsafe operations.
    [javac] Note: Recompile with -Xlint:unchecked for details.
    [javac] Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
    [javac] 2 errors

Please see #9488-157 - I think Ioana already fixed the unknown problem for extent resize.

Constantin Asofiei wrote:

Please see #9488-157 - I think Ioana already fixed the unknown problem for extent resize.

I've talked to her and she gave me a patch of convert/literals.rules for this issue but the code still converts the same and fails compilation. I tried debugging and making some changes and prints to the rule but it looks like the changes that are supposed to fix this case get overridden somewhere else. Either way conversion changes might not be enough as there is the runtime behavior that might need to be implemented.

Paul Bodale wrote:

Constantin Asofiei wrote:

Please see #9488-157 - I think Ioana already fixed the unknown problem for extent resize.

I've talked to her and she gave me a patch of convert/literals.rules for this issue but the code still converts the same and fails compilation. I tried debugging and making some changes and prints to the rule but it looks like the changes that are supposed to fix this case get overridden somewhere else. Either way conversion changes might not be enough as there is the runtime behavior that might need to be implemented.

Runtime should work, is the same as i = ?. extent(prop) = i..

Committed rev. 15878 on branch 6161a that introduces support for calling DYNAMIC-PROPERTY when the property retrieved may be an extent.

There are still some cases for dynamic extent variables/properties that I'm working on, as well as the problem with EXTENT(staticOrInstanceProperty) = ?.

One of the problems is the following:

DEFINE VARIABLE a AS INTEGER EXTENT 3 INITIAL [1, 2, 3] NO-UNDO.
props:intDynExt = a.
DEFINE VARIABLE res AS INTEGER EXTENT NO-UNDO.
res = DYNAMIC-PROPERTY(props, "intDynExt"). //works
MESSAGE res[1] res[2] res[3]. //works

Converts to:

props.ref().bulkSetIntDynExt(a);
assignMulti(res, ObjectOps.getDynamicPropPoly(props, new character("intDynExt")));
message(new Object[]
         {
            subscript(res, 1), //here FWD throws error 
            subscript(res, 2),
            subscript(res, 3)
         });

Regarding the problem in the previous note,

I think I know where the problem comes from. In the class where the variable is declared as an array, it is initialized with the size 0. When calling the assignMulti method, it is being reassigned to a copy of the source but the reference in the class given as argument remains the same as before the call. So after the call, the variable returns to the state it had before the call.

One solution is to assign the result of the assignMulti method to the variable. The method already returns the correct result so it'll be pretty straight forward I think.

Please post the entire testcase. The Java reference of an array is re-assigned only if it was resized (so dynamic extent). If is static extent, then the reference doesn't get re-assigned, except in cases when is used as an OUTPUT argument.

Constantin Asofiei wrote:

Please post the entire testcase. The Java reference of an array is re-assigned only if it was resized (so dynamic extent). If is static extent, then the reference doesn't get re-assigned, except in cases when is used as an OUTPUT argument.

DEFINE VARIABLE props AS CLASS Props.
props = NEW Props().

DEFINE VARIABLE a AS INTEGER EXTENT 3 INITIAL [1, 2, 3] NO-UNDO.
props:intDynExt = a.
DEFINE VARIABLE res AS INTEGER EXTENT NO-UNDO.
res = DYNAMIC-PROPERTY(props, "intDynExt").

MESSAGE res[1] res[2] res[3].

Converts to:

public class Play
{
   @LegacySignature(type = Type.VARIABLE, name = "props")
   object<? extends Props> props = UndoableFactory.object(Props.class);

   @LegacySignature(type = Type.VARIABLE, extent = 3, name = "a")
   integer[] a = TypeFactory.integerExtent(3, (long) 1, (long) 2, (long) 3);

   @LegacySignature(type = Type.VARIABLE, extent = -1, name = "res")
   integer[] res = TypeFactory.integerExtent();

   /**
    * External procedure (converted to Java from the 4GL source code
    * in task/play.p).
    */
   @LegacySignature(type = Type.MAIN, name = "task/play.p")
   public void execute()
   {
      externalProcedure(Play.this, new Block((Body) () -> 
      {
         props.assign(ObjectOps.newInstance(Props.class));
         props.ref().bulkSetIntDynExt(a);
         assignMulti(res, ObjectOps.getDynamicPropPoly(props, new character("intDynExt")));
         message(new Object[]
         {
            subscript(res, 1),
            subscript(res, 2),
            subscript(res, 3)
         });
      }));
   }
}

That should have converted as res = assignMulti(res, ...). That is the problem.

Constantin Asofiei wrote:

That should have converted as res = assignMulti(res, ...). That is the problem.

And yes, you were correct in the analysis. FWD should already have done this, please expand dynamic extent LVALUE (res = <expr>) where <expr> is another var, function call, method call, etc.

I've made some debugging and I've reached this place in the convert/assignments.rules:

@@ -343,7 +343,8 @@
                      <!-- if the rvalue is POLY, then emit assignMultiPoly and force assignment -->
                      <action>rvalueType = ecw.expressionType(rvalueExp, false)</action>
                      <rule>rvalueExp.type == prog.db_ref_non_static                          or
                            evalLib("type_pair", rvalueExp, prog.func_poly, prog.kw_dyn_func) or
-                           evalLib("type_pair", rvalueExp, prog.func_poly, prog.kw_dyn_invk)
+                           evalLib("type_pair", rvalueExp, prog.func_poly, prog.kw_dyn_invk) or
+                           evalLib("type_pair", rvalueExp, prog.func_poly, prog.kw_dyn_prop)
                         <action>methodtxt = "assignMultiPoly"</action>
                      </rule>

I've introduced support for the poly API of DYNAMIC-PROPERTY in the last commit. This is one of the cases where it converts to the method returning Object and adding it to the rule like above solves the issue.

FWD should already have done this, please expand dynamic extent LVALUE (res = <expr>) where <expr> is another var, function call, method call, etc.

Should I also touch on the other cases?

Paul Bodale wrote:

Should I also touch on the other cases?

We need to cover the same cases done for DYNAMIC-INVOKE.

I've tried running the existing tests for extent variables but I've a problem.

Tests convert like this:

Assert.legacyEquals(new integer(3), iVar.length);

Compilation fails because there is no overload of Assert.legacyEquals that takes an integer and a int. The problem seems to come from the fact that the chp_wrapper annotation is missing. This patch solves the issue:

=== modified file 'rules/convert/base_structure.xml'
--- old/rules/convert/base_structure.xml    2025-04-09 11:47:08 +0000
+++ new/rules/convert/base_structure.xml    2025-04-30 12:00:06 +0000
@@ -653,12 +653,16 @@
             <action>createPeerAst(java.class_reference, jname, closestPeerId)</action>
          </rule>

+         <rule>this.type == prog.func_int and
+               this.text.equalsIgnoreCase("extent") and
+               isNote("builtin") and getNoteBoolean("builtin")
+            <action>putNote("chp_wrapper", "integer")</action>
+         </rule>
          <!--  all RUN statement cases will be converted using ControlFlowOps
                APIs; explicit Java code is no longer emitted for these cases,
                as 4GL allows an internal proc name to have the same name as an

Please don't use this.text, use evalLib("type_pair", prog.func_int, prog.kw_extent, this) (or something like this).

Constantin Asofiei wrote:

Please don't use this.text, use evalLib("type_pair", prog.func_int, prog.kw_extent, this) (or something like this).

• the problem described above with the EXTENT function (getter) not converting properly because it is expected to always return a INTEGER
• the conversion problem with assigning ? via EXTENT statement (setter)

Constantin, I think that maybe you should take a look at the rule I modified in convert/literals.rules. The comment and the conditions lead me to believe that maybe this problem was encountered before and solved by this rule but then the AST structure changed and it no longer worked. If that's the case we might simplify it.

=== modified file 'rules/convert/literals.rules'
--- old/rules/convert/literals.rules    2025-04-16 07:19:23 +0000
+++ new/rules/convert/literals.rules    2025-05-05 06:55:44 +0000
@@ -821,10 +821,21 @@
                   <!-- EXTENT statement can assign the array's extent to unknown -->
                   <rule>copy.type == prog.unknown_val    and
                         copy.parent.indexPos == 1        and
-                        (upPath(this, prog.statement, prog.kw_extent, prog.expression) or 
-                         upPath(this, prog.statement, prog.kw_assign, prog.kw_extent, prog.expression)) 
+                        (
+                           upPath(this, prog.statement, prog.kw_extent, prog.expression)                   or 
+                           upPath(this, prog.statement, prog.kw_assign, prog.kw_extent, prog.expression)   or
+                           (
+                              parent.matchAnnotation("property-access-type", "resize", null)             and
+                              upPath(this, prog.expression, prog.object_invocation, prog.oo_meth_void)   and
+                              parent.prevSibling.isAnnotation("is-class")                                and
+                              #(boolean) parent.prevSibling.getAnnotation("is-class")
+                          ))

                      <action>classname = "integer"</action>
+                     <action>printfln("================================================== Literals =================================================")</action>
+                     <action>printfln(this.parent.parent.parent.dumpTree(true))</action>
+                     <action>printfln("==============================================================================================================")</action>
+
                   </rule>

Constantin Asofiei wrote:

That should have converted as res = assignMulti(res, ...). That is the problem.

And yes, you were correct in the analysis. FWD should already have done this, please expand dynamic extent LVALUE (res = <expr>) where <expr> is another var, function call, method call, etc.

Regarding changes made for this problem, I tested with both trunk and the branch, assignments to dynamic extent variables as mentioned above and all tests pass.

• I enhanced the API of DYNAMIC-PROPERTY to avoid unnecessary argument wrapping in character of first and second arguments. Execution is now sent upwards in the stack until it reaches the method that takes java.lang.String. This modification also implies changes to the methods that do the validation of the dynamic property arguments.
• I added overloads for DYNAMIC-PROPERTY API where the first argument is a character. This is needed both for cases when the first argument is a character expression and when it's a character variable.
• Added validation for arithmetic operations so that a BDT is expected but DYNAMIC-PROPERTY returns an extent then the correct error would be thrown.
• I also discovered that it is allowed to assign an empty character property to an object and fixed the behavior for this case in FWD.

I'm currently working on the assigning behavior, there are a few cases we need to work on a bit to but the majority of the tests are either completely passing or throwing error when an error should be thrown.

I also found this case and I'm not sure what to do

   //This tests is weird.
   @Test.
   METHOD PUBLIC VOID testFixedExtent3():
      DEFINE VARIABLE res AS INTEGER EXTENT 1 NO-UNDO.
      DEFINE VARIABLE aux AS INTEGER EXTENT 1 INITIAL [1] NO-UNDO.
      props:intExt1 = aux.

      res = DYNAMIC-PROPERTY(props, "intExt1") NO-ERROR.
      AssertExt:Error(14905, "Whole-array assignment target and source must have the same extent unless the target is indeterminate. (14905)").
      Assert:Equals(0, res[1]).
   END METHOD.

This test passes in progress.

• the property is dynamic extent and assigned with a extent of 1.
• extent objects (fixed and dynamic)
• static properties (either bdt or objects, fixed and dynamic extents) are referenced

This is really weird - looks like a bug in 4GL, you can pass DYNAMIC-PROPERTY(props, "intExt1") as an argument to a function or method call with no problem (with the parameter defined either EXTENT or EXTENT 1).

I don't know yet what the solution would be; how does res = DYNAMIC-PROPERTY(props, "intExt1") NO-ERROR. convert with your changes?

Constantin Asofiei wrote:

This is really weird - looks like a bug in 4GL, you can pass DYNAMIC-PROPERTY(props, "intExt1") as an argument to a function or method call with no problem (with the parameter defined either EXTENT or EXTENT 1).

I don't know yet what the solution would be; how does res = DYNAMIC-PROPERTY(props, "intExt1") NO-ERROR. convert with your changes?

Converts to this:

silent(() -> assignMultiPoly(res, ObjectOps.getDynamicPropPoly(props, new character("intExt1"))));

I've found the following conversion problems:

1. clsRef:obj = DYNAMIC-PROPERTY(props, "obj").

2. support.Props:StaticObjectProp = DYNAMIC-PROPERTY(props, "obj"). 

3. support.Props:objExt3St = DYNAMIC-PROPERTY(props, "objExt3")

Converts to:

1. props.ref().setObj(new object(ObjectOps.getDynamicProperty(props, new character("obj"))));

2.Props.setObjSt(new object(ObjectOps.getDynamicProperty(props, new character("obj"))))

3. Props.setAllObjExt3St(ObjectOps.getDynamicProperty(props, new character("objExt3")))

Compilation fails because there is no API for the DYNAMIC-PROPERTY function that returns object (or _BaseObject_).

What is getDynamicProperty returning in your branch?

I would expect if this is a problem of extent vs non-extent, then the setter will not be emitted, and instead a setDynamicProperty would be.

Constantin Asofiei wrote:

What is getDynamicProperty returning in your branch?

• BDT array (extent) -> getDynamicPropExtent
• Object (poly) -> getDynamicPropPoly

I would expect if this is a problem of extent vs non-extent, then the setter will not be emitted, and instead a setDynamicProperty would be.

So I should convert these cases to setDynamicProperty?

Assignment to variables was working fine even for objects but then we changed the assignment rules to convert to the API returning BDT for non-extent lvalue (including objects) and to the one returning poly for extent variables.

This change was made on #6490 and not yet patched on this branch but the same function from commmon-progress.rules is used and the results would be the same after merge.

Committed rev. 15883 to branch 6161 that introduces a fix for the assignment to properties case.

The changes were patched form the 6490a branch and adapted to work on all other cases discovered. This commit passes compilation phase on all tests written so far.

There this thing that needs to be addressed.

I wrote some more tests and like Constantin pointed out, in progress, when referencing an uninstantiated object property, the result of the DYNAMIC-PROPERTY function (getter) is the unknown value and this means that if we try to call a overloaded method with such a DYNAMIC-PROPERTY call as argument this would result in ambiguity. See the case below:

   METHOD PUBLIC VOID overloadedMthd(i AS INTEGER EXTENT 3):
      MESSAGE "Matched on INTEGER EXTENT 3".
   END METHOD.

   METHOD PUBLIC VOID overloadedMthd(o AS ClsA):
      MESSAGE "Matched on Object".
   END METHOD.

mthds:overloadedMthd(DYNAMIC-PROPERTY(props, "obj")). //Error: 13844 Ambiguous runtime call.

Currently FWD returns the property as normal and that makes the resolution process match on the right method even though in progress it fails. Sort of the same problem occurs when giving such a DYNAMIC-PROPERTY call as argument to a method or a function expecting something totally different. In FWD this fails, in progress works. I will update the DYNAMIC-PROPERTY API to check the reference when the result is a object and if it's null (uninstantiated) then return unknown.

• While solving the problem described above I noticed that when using the NEW statement to instantiate a class and the result of the DYNAMIC-PROPERTY call given as argument was an extent of uninstantiated objects it was treated as the unknown value which made the constructor resolution fail because of the overloaded methods.
• When using the DYNAMIC-NEW statement, even if the extent does not contain any instantiated objects, the call would work.

There is a problem with the SetParam builtin function.

I pulled the changes from branch 6490a related to method resolution and parameter validation. The code is following the same route as with the 6490a branch and reaches the place where it should throw the right error but then the tests still fails. I don't know what to do at this point, I tried debugging it but got nowhere. Could this be a problem with the silent worker? Or maybe even a regression?

The tests related to the SetParam function are passing on 6490a.

This is not related to DYNAMIC-PROPERTY but I'm going to log the problem here.
If you make a call to a procedure expecting a object extent and you give instead as argument a integer extent either of the same length or any length if the parameter is declared as dynamic extent, then FWD would manage to "cast" the integer[] reference to a object[] and it would match on that method even if this shouldn't be possible. This has to do with how object[] are treated/processed for method resolution.

RUN procExpectingObjExtent1 (DYNAMIC-PROPERTY(clsRef, "intExt1")). // Should throw error 2570.

I also found this case where it works the other way around:

mthd:expIntExt3(DYNAMIC-PROPERTY(props, "objExt3")).

• the different behavior of simple assignment to a dynamic extent property and assignment via DYNAMIC-PROPERTY statement (setter), the first of which throws an error in progress while the second works without any problem.
• assignment to extent 1 variables

For both of these issues we decided to not pursue a solution as they seem to be a bug in 4gl and it would simply not be worth it.

Found the following cases where a java.lang.NoSuchMethodException is thrown at the command line in the server:

mthds:expIntExt3(DYNAMIC-PROPERTY(props, "obj")) NO-ERROR.

mthds:expObjExt3(DYNAMIC-PROPERTY(props, "obj")) NO-ERROR.

Both tests pass returning the correct error but this is printed in the server:

java.lang.NoSuchMethodException: [Lcom.goldencode.p2j.util.integer;.<init>(com.goldencode.p2j.util.BaseDataType)
    at java.base/java.lang.Class.getConstructor0(Class.java:3585)
    at java.base/java.lang.Class.getConstructor(Class.java:2271)
    at com.goldencode.p2j.util.ControlFlowOps.resolveLegacyEntry(ControlFlowOps.java:6344)
    at com.goldencode.p2j.util.ControlFlowOps.resolveLegacyEntry(ControlFlowOps.java:5553)
    at com.goldencode.p2j.util.ControlFlowOps.resolveLegacyEntry(ControlFlowOps.java:4486)
    at com.goldencode.p2j.util.ObjectOps.invokePolyWorker(ObjectOps.java:1371)
    at com.goldencode.p2j.util.ObjectOps.invokeStandalonePoly(ObjectOps.java:1323)
    at com.goldencode.testcases.tests.base_language.builtin_functions.accepted_params.dynamic_property.MethodsCallArg.lambda$test12$24(MethodsCallArg.java:187)
    at com.goldencode.p2j.util.ErrorManager.silentWorker(ErrorManager.java:4535)
    at com.goldencode.p2j.util.ErrorManager.silent(ErrorManager.java:733)
    at com.goldencode.testcases.tests.base_language.builtin_functions.accepted_params.dynamic_property.MethodsCallArg.lambda$test12$25(MethodsCallArg.java:187)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9613)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:9221)
    at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:902)
    at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:875)
    at com.goldencode.testcases.tests.base_language.builtin_functions.accepted_params.dynamic_property.MethodsCallArg.test12(MethodsCallArg.java:185)
    at com.goldencode.testcases.tests.base_language.builtin_functions.accepted_params.dynamic_property.MethodsCallArgMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9757)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9713)
    at com.goldencode.p2j.util.ControlFlowOps.invokeLegacyMethod(ControlFlowOps.java:5231)
    at com.goldencode.p2j.util.ControlFlowOps.invokeLegacyMethod(ControlFlowOps.java:5152)
    at com.goldencode.p2j.util.ObjectOps.invoke(ObjectOps.java:4603)
    at com.goldencode.p2j.util.ObjectOps.invokeStandalone(ObjectOps.java:2402)
    at com.goldencode.p2j.testengine.TestExecutionSupport.lambda$callClassMethod$0(TestExecutionSupport.java:141)
    at com.goldencode.p2j.testengine.TestExecutionSupport.lambda$doInBlock$4(TestExecutionSupport.java:208)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9613)
    at com.goldencode.p2j.util.BlockManager.doBlockWorker(BlockManager.java:10922)
    at com.goldencode.p2j.util.BlockManager.doBlock(BlockManager.java:1637)
    at com.goldencode.p2j.testengine.TestExecutionSupport.doInBlock(TestExecutionSupport.java:200)
    at com.goldencode.p2j.testengine.TestExecutionSupport.callClassMethod(TestExecutionSupport.java:110)
    at com.goldencode.p2j.testengine.AbstractFWDTestDescriptor.callMethodImpl(AbstractFWDTestDescriptor.java:500)
    at com.goldencode.p2j.testengine.AbstractFWDTestDescriptor.callMethodImpl(AbstractFWDTestDescriptor.java:471)
    at com.goldencode.p2j.testengine.AbstractMethodTestDescriptor.executeTestMethod(AbstractMethodTestDescriptor.java:411)
    at com.goldencode.p2j.testengine.AbstractMethodTestDescriptor.executeImpl(AbstractMethodTestDescriptor.java:278)
    at com.goldencode.p2j.testengine.UnitTestServer.lambda$execute$8(UnitTestServer.java:419)
    at com.goldencode.p2j.testengine.UnitTestServer.convertError(UnitTestServer.java:484)
    at com.goldencode.p2j.testengine.UnitTestServer.execute(UnitTestServer.java:419)
    at com.goldencode.p2j.testengine.UnitTestServerMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:156)
    at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:807)
    at com.goldencode.p2j.net.Conversation.block(Conversation.java:422)
    at com.goldencode.p2j.net.Conversation.run(Conversation.java:235)
    at java.base/java.lang.Thread.run(Thread.java:840)
Listening for transport dt_socket at address: 2080
java.lang.NoSuchMethodException: [Lcom.goldencode.p2j.util.object;.<init>(com.goldencode.p2j.util.BaseDataType)
    at java.base/java.lang.Class.getConstructor0(Class.java:3585)
    at java.base/java.lang.Class.getConstructor(Class.java:2271)
    at com.goldencode.p2j.util.ControlFlowOps.resolveLegacyEntry(ControlFlowOps.java:6344)
    at com.goldencode.p2j.util.ControlFlowOps.resolveLegacyEntry(ControlFlowOps.java:5553)
    at com.goldencode.p2j.util.ControlFlowOps.resolveLegacyEntry(ControlFlowOps.java:4486)
    at com.goldencode.p2j.util.ObjectOps.invokePolyWorker(ObjectOps.java:1371)
    at com.goldencode.p2j.util.ObjectOps.invokeStandalonePoly(ObjectOps.java:1323)
    at com.goldencode.testcases.tests.base_language.builtin_functions.accepted_params.dynamic_property.MethodsCallArg.lambda$test19$38(MethodsCallArg.java:264)
    at com.goldencode.p2j.util.ErrorManager.silentWorker(ErrorManager.java:4535)
    at com.goldencode.p2j.util.ErrorManager.silent(ErrorManager.java:733)
    at com.goldencode.testcases.tests.base_language.builtin_functions.accepted_params.dynamic_property.MethodsCallArg.lambda$test19$39(MethodsCallArg.java:264)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9613)
    at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:9221)
    at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:902)
    at com.goldencode.p2j.util.BlockManager.internalProcedure(BlockManager.java:875)
    at com.goldencode.testcases.tests.base_language.builtin_functions.accepted_params.dynamic_property.MethodsCallArg.test19(MethodsCallArg.java:262)
    at com.goldencode.testcases.tests.base_language.builtin_functions.accepted_params.dynamic_property.MethodsCallArgMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invokeImpl(ControlFlowOps.java:9757)
    at com.goldencode.p2j.util.ControlFlowOps$InternalEntryCaller.invoke(ControlFlowOps.java:9713)
    at com.goldencode.p2j.util.ControlFlowOps.invokeLegacyMethod(ControlFlowOps.java:5231)
    at com.goldencode.p2j.util.ControlFlowOps.invokeLegacyMethod(ControlFlowOps.java:5152)
    at com.goldencode.p2j.util.ObjectOps.invoke(ObjectOps.java:4603)
    at com.goldencode.p2j.util.ObjectOps.invokeStandalone(ObjectOps.java:2402)
    at com.goldencode.p2j.testengine.TestExecutionSupport.lambda$callClassMethod$0(TestExecutionSupport.java:141)
    at com.goldencode.p2j.testengine.TestExecutionSupport.lambda$doInBlock$4(TestExecutionSupport.java:208)
    at com.goldencode.p2j.util.Block.body(Block.java:636)
    at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9613)
    at com.goldencode.p2j.util.BlockManager.doBlockWorker(BlockManager.java:10922)
    at com.goldencode.p2j.util.BlockManager.doBlock(BlockManager.java:1637)
    at com.goldencode.p2j.testengine.TestExecutionSupport.doInBlock(TestExecutionSupport.java:200)
    at com.goldencode.p2j.testengine.TestExecutionSupport.callClassMethod(TestExecutionSupport.java:110)
    at com.goldencode.p2j.testengine.AbstractFWDTestDescriptor.callMethodImpl(AbstractFWDTestDescriptor.java:500)
    at com.goldencode.p2j.testengine.AbstractFWDTestDescriptor.callMethodImpl(AbstractFWDTestDescriptor.java:471)
    at com.goldencode.p2j.testengine.AbstractMethodTestDescriptor.executeTestMethod(AbstractMethodTestDescriptor.java:411)
    at com.goldencode.p2j.testengine.AbstractMethodTestDescriptor.executeImpl(AbstractMethodTestDescriptor.java:278)
    at com.goldencode.p2j.testengine.UnitTestServer.lambda$execute$8(UnitTestServer.java:419)
    at com.goldencode.p2j.testengine.UnitTestServer.convertError(UnitTestServer.java:484)
    at com.goldencode.p2j.testengine.UnitTestServer.execute(UnitTestServer.java:419)
    at com.goldencode.p2j.testengine.UnitTestServerMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:156)
    at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:807)
    at com.goldencode.p2j.net.Conversation.block(Conversation.java:422)
    at com.goldencode.p2j.net.Conversation.run(Conversation.java:235)
    at java.base/java.lang.Thread.run(Thread.java:840)

Is this from this code?

               // last resort - try to convert
               try
               {
                  Constructor<?> ctor = mtypes[i].getConstructor(BaseDataType.class);
                  origArgs[i] = ctor.newInstance(origArgs[i]);
               }
               catch (ReflectiveOperationException exc)
               {
                  exc.printStackTrace();
                  // TODO: what ?
               }

Constantin Asofiei wrote:

Is this from this code?
[...]

Yes

The please improve that code; at the least, if mtypes[i].isArray(), the do not // last resort - try to convert

Committed rev. 15943 on branch 6161a which addresses the issue described above.
This is my proposed solution:

=== modified file 'src/com/goldencode/p2j/util/ControlFlowOps.java'
--- old/src/com/goldencode/p2j/util/ControlFlowOps.java    2025-05-21 13:44:20 +0000
+++ new/src/com/goldencode/p2j/util/ControlFlowOps.java    2025-05-22 07:27:33 +0000
@@ -6336,7 +6336,7 @@
          if (!mtypes[i].isAssignableFrom(origArgs[i].getClass()))
          {
-            if (origArgs[i] instanceof BaseDataType)
+            if (origArgs[i] instanceof BaseDataType && mtypes[i].isAssignableFrom(BaseDataType.class))
             {
                // last resort - try to convert
                try

This solves the issue and makes FWD try conversion only if both the argument and the expected parameters are not arrays.

Shouldn't mtypes[i].isAssignableFrom(BaseDataType.class) be BaseDataType.class.isAssignableFrom(...)? The javadoc states that Determines if the class or interface represented by this Class object is either the same as, or is a superclass or superinterface of, the class or interface represented by the specified Class parameter. .

Constantin Asofiei wrote:

Shouldn't mtypes[i].isAssignableFrom(BaseDataType.class) be BaseDataType.class.isAssignableFrom(...)? The javadoc states that Determines if the class or interface represented by this Class object is either the same as, or is a superclass or superinterface of, the class or interface represented by the specified Class parameter. .

Actually yes, you're right. Sorry, my bad.

Committed rev. 15944 on branch 6161a where I've corrected this condition.

• add missing headers to files where is not set
• shouldn't this honor also direct method calls? same for the #6490 case
  

                 <!-- Check if we need to add a cast to Object for varargs -->
                    <rule>parent.type &gt; prog.begin_functypes and
                          parent.type &lt; prog.end_functypes
  

• common-progress.rules - get_dynamic_case appears twice, remove the later one - I think you modified the first appearance.
• ControlFlowOps - line 10001 - please split it in two
• object.assign - please add paranthesis to the condition, to be easily readable.
• ObjectOps.setDynamicProperty - what if character prop is unknown?
  • convertFromJavaToBTD and some others like getDynamicPropExtent - missing javadoc
• PropertyReference - missing javadoc
• SourceNameMapper.java - no real change, please revert this to trunk's rev.

Committed rev. 15945 on branch 6161 that addresses the feedback received.

Constantin Asofiei wrote:

Review of 6161a rev 15944:

• add missing headers to files where is not set

History entries added.

• shouldn't this honor also direct method calls? same for the #6490 case
  [...]

I tested this multiple times and the results show that the cast is only needed for function calls. This is because we match the DYNAMIC-PROPERTY API to the corresponding parameter of the function (so when the function is expecting a extent we generate the API for extents). If the function generated is returning an array then the elements of the array would be considered individual arguments (and not one extent argument).
For procedure and method calls we do not have this problem because in those cases we always generate the API for the poly case which returns Object.

• common-progress.rules - get_dynamic_case appears twice, remove the later one - I think you modified the first appearance.
• ControlFlowOps - line 10001 - please split it in two
• object.assign - please add paranthesis to the condition, to be easily readable.

Done.

• ObjectOps.setDynamicProperty - what if character prop is unknown?

I've included this case in my tests. This falls into the category of sometimes returning the wrong error but the functionality is 100%.

• convertFromJavaToBTD and some others like getDynamicPropExtent - missing javadoc

• PropertyReference - missing javadoc

Javadoc added.

• SourceNameMapper.java - no real change, please revert this to trunk's rev.

Reverted the changes.

Paul Bodale wrote:

I tested this multiple times and the results show that the cast is only needed for function calls. This is because we match the DYNAMIC-PROPERTY API to the corresponding parameter of the function (so when the function is expecting a extent we generate the API for extents). If the function generated is returning an array then the elements of the array would be considered individual arguments (and not one extent argument).

My understanding is wherever you can pass a direct function call, you can pass a direct method call. Please remind me the example test for the function's case.

• ObjectOps.setDynamicProperty - what if character prop is unknown?

I've included this case in my tests. This falls into the category of sometimes returning the wrong error but the functionality is 100%.

So, if validDynamicPropertyArgs receives the "?" string, what does it do? At least add a comment there.

Constantin Asofiei wrote:

Paul Bodale wrote:

I tested this multiple times and the results show that the cast is only needed for function calls. This is because we match the DYNAMIC-PROPERTY API to the corresponding parameter of the function (so when the function is expecting a extent we generate the API for extents). If the function generated is returning an array then the elements of the array would be considered individual arguments (and not one extent argument).

My understanding is wherever you can pass a direct function call, you can pass a direct method call. Please remind me the example test for the function's case.

We are able to infer what should the DYNAMIC-PROPERTY return if we look at the corresponding parameter so, for this case we either generate the API for scalar or the API for extent but never poly.

For example:

FUNCTION f RETURNS INTEGER (e AS INTEGER EXTENT 3):
    MESSAGE "Function reached".
END FUNCTION.

PROCEDURE proc:
    DEFINE INPUT PARAMETER a AS INTEGER EXTENT 3.

    MESSAGE "Procedure reached".
END PROCEDURE.

//Method call
mthds:auxTest(DYNAMIC-PROPERTY(props, "intExt3")).

//Function call
f(DYNAMIC-PROPERTY(props, "intExt3")).

//Procedure call
RUN proc (DYNAMIC-PROPERTY(props, "intExt3")).

Converts to:

// Method call
ObjectOps.invokeStandalonePoly(mthds, dynamic_property.support.Mthds.class, "auxTest", "I", ObjectOps.getDynamicPropPoly(props, new character("intExt3")));

// Function call
FUNC_CALL_SITE_1.clone().execute((Object) ObjectOps.getDynamicPropExtent(props, new character("intExt3")));

// Procedure call
RUN_CALL_SITE_1.clone().run(ObjectOps.getDynamicPropPoly(props, new character("intExt3")));

I remember that we discussed about converting the DYNAMIC-INVOKE to specifically scalar or extent API for methods but we decided not to, and also decided to convert to poly for the RUN statements.

OK, I think I understand now. Thanks.

Constantin Asofiei wrote:

• ObjectOps.setDynamicProperty - what if character prop is unknown?

I've included this case in my tests. This falls into the category of sometimes returning the wrong error but the functionality is 100%.

So, if validDynamicPropertyArgs receives the "?" string, what does it do? At least add a comment there.

This test for example:

DEFINE VARIABLE p AS CHARACTER NO-UNDO.
p = ?.
DYNAMIC-PROPERTY(props, p) = 1 NO-ERROR.
AssertExt:Error(16581, "Invalid property name passed to DYNAMIC-PROPERTY (16581)").

Returns the error 16548 instead of 16581 like expected but I will can fix this in the next commit.

         if ((value.isUnknown()         && !(value instanceof longchar)) ||
            (value instanceof character && value.toStringMessage().isEmpty()) ||
            (value instanceof integer   && value.equals(new integer(0))))

• instead of toStringMessage() use !isUnknown() && getValue().isEmpty()
• instead of equals(new integer(0)) do the same, !isUnknown() && intValue() == 0

Otherwise, I think is OK. I'll do conversion testing.

Otherwise, please update the %Done - as I assume there are TODO's for edge cases, and also make sure all tests are on xfer (and post here the list of relevant tests).

Paul Bodale wrote:

Returns the error 16548 instead of 16581 like expected but I will can fix this in the next commit.

What if you pass the "?" string instead of unknown?

Constantin Asofiei wrote:

What if you pass the "?" string instead of unknown?

Then the error 16548 would be shown, "Could not find property '?' in class ...". I've included tests for this too and also for the DYNAMIC-PROPERTY function (getter).

I committed rev. 15946 on branch 6161a that addresses the feedback received. In this commit I also added validation for the case described in note #6161-61 and a overload for setDynamicProperty that was omitted.

and also make sure all tests are on xfer (and post here the list of relevant tests).

All tests related to DYNAMIC-PROPERTY are located in tests/base_language/builtin_functions/accepted_params/dynamic_property. To run them, the following zfile_set.txt configuration is necessary:

F ./support/FileUtils.cls
F ./support/test/AssertExt.cls
D ./tests/base_language/builtin_functions/accepted_params/dynamic_property/ (*.cls)

There is also an added new integer wrapper for this case:

DO i = 1 TO EXTENT(extVar):

it will have new integer(extVar.length). Can you limit this new integer only for the required case?

Constantin Asofiei wrote:

There is also an added new integer wrapper for this case:
[...]

it will have new integer(extVar.length). Can you limit this new integer only for the required case?

I've done this because while I was checking the existent support, I found a test class that wasn't converting because Assert:Equals method equivalent doesn't have an overload for java native int type. Except for this specific case, the wrapping can be dropped altogether.

EDIT:
The test class is this one: tests/base_language/extent/TestExtent.cls

Paul Bodale wrote:

Constantin Asofiei wrote:

There is also an added new integer wrapper for this case:
[...]

it will have new integer(extVar.length). Can you limit this new integer only for the required case?

I've done this because while I was checking the existent support, I found a test class that wasn't converting because Assert:Equals method equivalent doesn't have an overload for java native int type.

Assert:Equals is converted code - it should not have Java native type overloads. Why was that wrapper not emitting? Can you use a simple func0(extVar:extent)?

Constantin Asofiei wrote:

Why was that wrapper not emitting?

See note #6161-30 . It looks like it's because the chp_wrapper annotation is missing.

Can you use a simple func0(extVar:extent)?

This example mthd:expIntScalar(EXTENT(props:intExtDyn)).
Converts to mthd.ref().expIntScalar(props.ref().lengthOfIntExtDyn()); which is ok.

As far as I've tested everything related to the EXTENT function works ok.

Sorry, I meant extent(extVar) as argument, not property call.

Constantin Asofiei wrote:

Sorry, I meant extent(extVar) as argument, not property call.

And this is not a class var, but a local method var.

Constantin, relative to our discussion.
This piece of code that I found in ClassDefinition.annotateMethodCall:

         if (isBuiltIn())
         {
            // for builtin method calls, save the typelist too
            for (int i = 0; i < mdat.signature.length; i++)
            {
               String pType = mdat.signature[i].type;
               int idx = pType.indexOf('[');
               if (idx >= 0)
               {
                  pType = pType.substring(0, idx);
               }
               node.putAnnotation("typelist", pType, i);
            }
         }

is related to a revision that introduces changes related to arguments for builtin oo method calls expecting table-handle parameters. Basically, it forces wrapping of arguments in a TableParameter constructor and the typelist annotation was needed to check if the parameter expected is a table-handle.

I made an experiment and placed the piece of code above into the ClassDefinition.addMethod method and then the typelist was also generating for the method definition but the main problem did not go away. I tried to look into how DictionaryWorker.lookupDictionaryObject because this is how the typelist is retrieved in base_structure.xml but this did not lead anywhere.

I tried inspecting the structure of the rules and noticed that there is no "refid" annotation for the Equals method probably because it's builtin. The problem is that the existence of this annotation is one of the conditions to actually check the typelist.

I'm OK with the changes in 6161a, and it passed conversion testing. Please run harness for some apps.

Also, 6161a was rebased and rev 15964 reverts base_structure.xml

Please create a task in Base Language project to improve the skeleton class parsing, to emit typelist, param_modes at the definition AST for the builtin method; this also requires a way to resolve the 'refid' for builtin class methods (via tempidx-file and associated annotations).

I tested harness on the app i have and it passed.

Paul - please create a separate task in Base Language project, for the edge cases in #6161-71.

Constantin Asofiei wrote:

Paul - please create a separate task in Base Language project, for the edge cases in #6161-71.

Created task #10316 for these.