FWD » Core Development » Base Language

Please look also into what force_no_wrap means. And find where when evalLib("oo_call_type", parent, this) was added.

Constantin Asofiei wrote:

find when evalLib("oo_call_type", parent, this) was added.

It was added in trunk revision 14925 referencing the issue #8156. What I understood is that that problem was about function/method calls given as arguments to other function/method calls and annotating the call itself with the "wrap" annotation was considered that "its arguments might need wrapping" instead of "the call itself needs wrapping".
Part of the solution was to avoid wrapping the call to a function or a method in the literals.rules file, hence the condition (not(evalLib("call_to_user_defined_function", parent) or evalLib("oo_call_type", parent, this))).

After a closer look, I saw that the case for which the current issue is about is handled in the next line: copy.parent.isAnnotation("wrap") but it gets ignored because the condition fails.

I would enhance copy.parent.isAnnotation("wrap") to check that the parent is SUPER or THIS-OBJECT (use the type_pair function).

Or otherwise find why the literal at SUPER/THIS-OBJECT doesn't have 'wrap' annotation.

Constantin Asofiei wrote:

Or otherwise find why the literal at SUPER/THIS-OBJECT doesn't have 'wrap' annotation.

I don't fully understand why but it might be because of the expression node that is inserted as the father of the literal. It alone has the wrap annotation put in place by this rule found in base_structure.xml line 820:

<rule>(upPath(this, prog.kw_this_obj, prog.lparens, prog.parameter) or 
                   upPath(this, prog.kw_super, prog.lparens, prog.parameter)) and
                  parent.parent.parent.isAnnotation("javaname")
               <action>putNote("wrap", true)</action>
            </rule>

Looks like the cases of SUPER/THIS-OBJECT calls were specifically considered to be treated here instead of in the same place as a normal method call which gets it's arguments wrapped in oo_references.rules at line 584 (or at least where my simple example does).
The example I'm using is this a:voidMthd(123, "abc").

Please post the AST for the SUPER and the voidMthd call.

SUPER (cResult, 123). :

     [java] SUPER [KW_SUPER]:408021893238 @13:9
     [java]       (javaname=__task_baseClass_constructor__, refid=403726925905, access-mode=2130, support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435, signature=baseclass(KW_OUTPUT character, KW_INPUT integer), javaname-byrule=annotations/method_defs.rules:971, refid-byrule=annotations/early_annotations.xml:193, static=false, found-in-cls=task.BaseClass, found-in-source-file=./task/BaseClass.cls, param_modes=OI)
     [java]    ( [LPARENS]:408021893240 @13:15
     [java]          (support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435)
     [java]       parameter [PARAMETER]:408021893241 @0:0
     [java]             (assign_back=true, jtype=character, parmtype=774)
     [java]          expression [EXPRESSION]:408021893242 @0:0
     [java]                (support_level=16400, param_index=0, assign_back=true, support_level-byrule=gaps/gap_analysis_marking.xml:435, parmtype=774)
     [java]             cResult [VAR_CHAR]:408021893243 @13:16
     [java]                   (support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435, oldtype=2982, refid=408021893215, refid-byrule=fixups/post_parse_fixups.xml:406)
     [java]       parameter [PARAMETER]:408021893246 @0:0
     [java]             (jtype=integer, parmtype=706)
     [java]          expression [EXPRESSION]:408021893247 @0:0
     [java]                (param_index=1, wrap-byrule=convert/base_structure.xml:823, wrap=true, parmtype=706, support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435)
     [java]             123 [NUM_LITERAL]:408021893248 @13:25
     [java]                   (is-literal=true, support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435, use64bit=false, use64bit-byrule=annotations/cleanup.rules:469)

a:voidMthd(123, "abc"). :

     [java] assignment [ASSIGNMENT]:416611827770 @0:0
     [java]    expression [EXPRESSION]:416611827771 @0:0
     [java]          (support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435)
     [java]       : [OBJECT_INVOCATION]:416611827772 @8:2
     [java]          a [VAR_CLASS]:416611827773 @8:1
     [java]                (is-java=false, source-file=./task/AuxCls.cls, is-class=true, refid=416611827729, support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435, qualified-reference-byrule=annotations/oo_references.rules:339, containing-package-byrule=annotations/oo_references.rules:337, simple-java-class-byrule=annotations/oo_references.rules:336, oldtype=2982, is-interface=false, full-java-class-byrule=annotations/oo_references.rules:335, qualified-reference=true, is-enum=false, qualified=task.auxcls, refid-byrule=fixups/post_parse_fixups.xml:406, containing-package=com.goldencode.testcases.task, dotnet-cls=false, builtin-cls=false, simple-java-class=AuxCls, indirect-dotnet=false, full-java-class=com.goldencode.testcases.task.AuxCls)
     [java]          voidMthd [OO_METH_VOID]:416611827774 @8:3
     [java]                (javaname=voidMthd, refid=399431958559, found-in-full-java-class=com.goldencode.testcases.task.AuxCls, access-mode=2130, found-in-full-java-class-byrule=annotations/oo_references.rules:582, signature=voidmthd(KW_INPUT integer, KW_INPUT character), javaname-byrule=annotations/oo_references.rules:573, refid-byrule=annotations/early_annotations.xml:193, static=false, found-in-cls=task.AuxCls, found-in-source-file=./task/AuxCls.cls, param_modes=II)
     [java]             123 [NUM_LITERAL]:416611827776 @8:12
     [java]                   (param_index=0, wrap=true, use64bit=false, use64bit-byrule=annotations/cleanup.rules:469, is-literal=true, support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435, chp_wrapper-byrule=convert/base_structure.xml:732, wrap-byrule=include/common-progress.rules:6475, jtype=integer, parmtype=706, chp_wrapper=integer)
     [java]             "abc" [STRING]:416611827779 @8:17
     [java]                   (param_index=1, wrap=true, is-literal=true, support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435, classname-byrule=convert/base_structure.xml:708, classname=character, chp_wrapper-byrule=convert/base_structure.xml:732, wrap-byrule=include/common-progress.rules:6475, jtype=character, parmtype=706, chp_wrapper=character)

Is having the rule like this work?

         <rule>
            !isNote("force_no_wrap") and
            (isNote("wrap")  or
             (parent.type == prog.expression and copy.parent.isAnnotation("wrap")
              and (parent.parent.type == prog.kw_this_obj or parent.parent.type == prog.kw_super)) or
             ((not(evalLib("call_to_user_defined_function", parent) or 
                   evalLib("oo_call_type", parent, this))) and
             ....

I don't understand the need to remove copy.parent.isAnnotation("wrap") or from the lower part.

Constantin Asofiei wrote:

Is having the rule like this work?
[...]

I don't understand the need to remove copy.parent.isAnnotation("wrap") or from the lower part.

You're right, there's no need to remove it, I just moved it one level above so it won't get ignored and I thought that having it in both places is redundant.
I checked the rule and it passes conversion and compilation phases. I think this is the solution you described here:

I would enhance copy.parent.isAnnotation("wrap") to check that the parent is SUPER or THIS-OBJECT (use the type_pair function).

Constantin Asofiei wrote:

Is having the rule like this work?

I checked again with trunk revision 15840 ( + rules modification) and this method fails compilation with the same problem as before.
I don't know what's the problem here but with the condition modified as initially proposed the code converts and compiles.

• SUPER(b:retChar())
• SUPER(b:retDate())
• SUPER(b:retDateTime())
• SUPER(b:retInt())
• SUPER(b:retInt64())
• SUPER(123)
• SUPER(-123)
• THIS-OBJECT(a:retDateTime())
• THIS-OBJECT(a:retDateTimeTz())
• THIS-OBJECT(a:retDecimal())
• THIS-OBJECT(a:retInt64())
• THIS-OBJECT(a:retLongChar())

The constructors for the integer literals are generated correctly but they are matched with the constructor expecting decimal.

Paul Bodale wrote:

• SUPER(123)
• SUPER(-123)
  The constructors for the integer literals are generated correctly but they are matched with the constructor expecting decimal.

You mean the super-constructor has parameters defined as decimal, and the call for super wraps the literal in new integer? If so, we need to fix this. I think we need to add a different 'classname' annotation for 'wrap', not depending on the literal, but on the call. How does the method call in this case work? Parameter defined as decimal, integer literal as argument.

Constantin Asofiei wrote:

Paul Bodale wrote:

• SUPER(123)
• SUPER(-123)
  The constructors for the integer literals are generated correctly but they are matched with the constructor expecting decimal.

How does the method call in this case work? Parameter defined as decimal, integer literal as argument.

In the base class I have constructors for every data type that can be given/received as parameters (they are 15 in total). The wrapping works fine it's just the method resolution that's lacking possibly because of so many constructors.

It might also be worth noting, while converting the class I used for testing THIS-OBJECT method, this warning was printed:

     [java] WARNING: more than one method def found using fuzzy lookup: bclass from class tests.oo.support.Bclass
     [java] 
     [java]    CALLER THIS-OBJECT [KW_THIS_OBJ]:<id_unavailable> @274:7
     [java]       (original-token=["THIS-OBJECT",<897>,line=274,col=7])
     [java]    ( [LPARENS]:<id_unavailable> @274:19
     [java]          (original-token=["(",<2971>,line=274,col=19])
     [java]       parameter [PARAMETER]:<id_unavailable> @0:0
     [java]          expression [EXPRESSION]:<id_unavailable> @0:0
     [java]                (param_index=0)
     [java]             cResult [VAR_CHAR]:<id_unavailable> @274:20
     [java]                   (found-in-cls=tests.oo.support.Bclass, oldtype=2982, found-in-source-file=./tests/oo/support/Bclass.cls, tempidx=412, original-token=["cResult",<384>,line=274,col=20], tempidx-file=./tests/oo/support/Bclass.cls)
     [java]       parameter [PARAMETER]:<id_unavailable> @0:0
     [java]          expression [EXPRESSION]:<id_unavailable> @0:0
     [java]                (param_index=1)
     [java]             THIS-OBJECT [VAR_CLASS]:<id_unavailable> @274:29
     [java]                   (is-class=true, dotnet-cls=false, oldtype=897, qualified=tests.oo.support.bclass, original-token=["THIS-OBJECT",<385>,line=274,col=29], indirect-dotnet=false, is-java=false, is-enum=false, is-interface=false, source-file=./tests/oo/support/Bclass.cls, builtin-cls=false)
     [java] 
     [java]    PARAMETERS: 2
     [java]        character
     [java]        object<? extends tests.oo.support.bclass>
     [java]    MATCH 0: METRICS 2 exact; 0 cvt; 0 extExact; 0 extCvt; 0 wildcard; [I@1dbfbd94 ooLvl; [Ljava.lang.String;@ec1b776 overrides; 
     [java]         KW_PUBLIC instance member Bclass (OO_METH_VOID), extent 0, javaname bclass, override false, abstract false in class tests.oo.support.Bclass, tempIdx 66
     [java]    PARAMETERS: 2
     [java]       KW_OUTPUT character
     [java]       KW_INPUT object<? extends tests.oo.support.bclass>
     [java]  data
     [java] 
     [java]    MATCH 1: METRICS 2 exact; 0 cvt; 0 extExact; 0 extCvt; 0 wildcard; [I@2e3b4394 ooLvl; [Ljava.lang.String;@70ccb3bf overrides; 
     [java]         KW_PUBLIC instance member Bclass (OO_METH_VOID), extent 0, javaname bclass_2, override false, abstract false in class tests.oo.support.Bclass, tempIdx 111
     [java]    PARAMETERS: 2
     [java]       KW_OUTPUT character
     [java]       KW_OUTPUT object<? extends tests.oo.support.bclass>
     [java]  data

I also had to play a bit with the signature of the constructors so I could make it possible to have all of them in the same class.

• the super-class defining a 'decimal' constructor
• the sub-class calling super(1)

I'm interested if this converts properly. After that, add to super-class another constructor with a single 'integer' parameter - check how this converts, too, with the sub-class still calling super(1).

The issues with constructor overload may be an existing one, and may be not related only to literals, but to variables, too (like you call super(intVar).

Constantin Asofiei wrote:

Please make a simple test where you have:

• the super-class defining a 'decimal' constructor
• the sub-class calling super(1)

I'm interested if this converts properly.

Yes, the code with the structure above converts. And after adding the constructor to the super-class, the result is still the same. FWD matches on method receiving DECIMAL even if it has one with INTEGER.

1. In the following order, I declared the constructor taking DECIMAL in the super-class, then I defined the constructor in the child class that calls SUPER(1).
2. In a procedure I created an object and progress was able to match on the constructor in the super-class taking DECIMAL.
3. Then I added another constructor in the super-class taking an INTEGER and saved.
4. Then I ran the procedure again and again progress matched on the constructor in the super-class taking DECIMAL.
5. I then did a small change (inserted a space) to the file in which the child class is declared and saved and now, progress matched the right constructor in the super-class.

• in the ControlFlowOps.resolveLegacyEntry method, after the refinement phase there are only 2 constructors that remain, which are the ones taking INTEGER and DECIMAL with DECIMAL being the first one in the list
• the execution continues to this if if (overloads.size() > 1) { // check if there is an exact match where the integer argument is converted to it's decimal form by this piece of code:
  

  if (candidateType != sigType)
  {
     arg = normalizeType(arg, sigType, candidateType); // <- here 
     sigType = arg.getClass();
  }
  

Because of this the wrong InternalEntry instance is returned.

Paul Bodale wrote:

1. I then did a small change (inserted a space) to the file in which the child class is declared and saved and now, progress matched the right constructor in the super-class.

That's because the sub-class was not re-compiled with the new super-class definition until you changed it.

In FWD, I traced the calls to a case where the wrong constructor is matched and here is what I found:

• in the ControlFlowOps.resolveLegacyEntry method, after the refinement phase there are only 2 constructors that remain, which are the ones taking INTEGER and DECIMAL with DECIMAL being the first one in the list
• the execution continues to this if if (overloads.size() > 1) { // check if there is an exact match where the integer argument is converted to it's decimal form by this piece of code:
  [...]

Because of this the wrong InternalEntry instance is returned.

Or you talking about NEW or SUPER/THIS-OBJECT? Because ControlFlowOps.resolveLegacyEntry is used for NEW.

I debugged a failing test and started with NEW

It just occurred to me, this means that the problem is not with SUPER or THIS-OBJECT but with how the constructor in the child class is resolved.

After our discussion I was looking into the problem with the double constructor wrapping of the literal, and I found that there are 2 annotations at play. One is "wrap_parameter" and the other one is simply "wrap".

Based on the code I saw, the first annotation is used to mark arguments that need to be wrapped according to what the function/method is expected to receive. The second one is used to wrap a node based on the datatype of the expression.

After I rebased the branch with the trunk rev. 15840 and reverted all the changes to .rules files (including the function "oo_call_type"), the problem wasn't the simple Java literals given as arguments anymore but the fact that 2 constructors were inserted, the outer one (inserted because of "wrap_parameter" annotation) matching the type expected for the wrong constructor and the inner one being according to the progress datatype of the literal. And modifying the oo_call_type function gets rid of the inner constructor.

I am currently investigating why this happens.

Regarding the problem with the double constructors that we discussed about I discovered that it came from the fact that the right constructor was declared with the wrong name (instead of "BaseClass" it was "BaseCls").
FWD doesn't check that the name declared for a certain constructor is the same as the name for the class it's declared in. Progress checks this at compile time and throws error immediately if the name is not right. This feature could save some time in debugging in the future.

• literals.rules
  

  -            (isNote("wrap")  or
  -             ((not(evalLib("call_to_user_defined_function", parent) or 
  -                   evalLib("oo_call_type", parent, this))) and
  -               (copy.parent.isAnnotation("wrap")                         or
  +            (isNote("wrap") or
  +            (
  +               copy.parent.isAnnotation("wrap") and 
  +               (upPath(this, prog.kw_super, prog.lparens, prog.parameter, prog.expression) or 
  +               upPath(this, prog.kw_this_obj, prog.lparens, prog.parameter, prog.expression))
  +            ) or
  +            (
  +               (not(evalLib("call_to_user_defined_function", parent) or evalLib("oo_call_type", parent, this))) and
  +               (copy.parent.isAnnotation("wrap")                        or
  

• common-progress.rules
  

            (upPath(chref, prog.func_class, prog.lparens, prog.parameter) and 
  -          evalLib("type_pair", ooref.parent.parent, prog.func_class, prog.kw_new))
  +          evalLib("type_pair", ooref.parent.parent, prog.func_class, prog.kw_new))  or
  +         upPath(ooref, prog.statement, prog.kw_super, prog.lparens, prog.parameter) or
  +         upPath(ooref, prog.statement, prog.kw_this_obj, prog.lparens, prog.parameter)
  
  

That being said, the test for calling SUPER with integer literals (123 and -123) still fails. I don't know how but one time I managed to make it pass somehow but then I ran the server again without the -d option for debug and the tests failed. I might've had some problems with sockets not being closed properly but I asked a colleague to also run them locally and still failed. I'll further look into this as it's most probably the one I described here #9841-20 (last part).

it's most probably the one I described here #9841-20 (last part)

Confirmed.

This is not a problem regarding constructor resolution and might not be in scope for this current task.

I'm trying to match to the method with the following signature CONSTRUCTOR Aclass (INPUT i AS INTEGER, OUTPUT cResult AS CHARACTER, INPUT isLiteral AS LOGICAL) by making the following call aInst = NEW Aclass (123, OUTPUT res, TRUE).

The call converts to aInst.assign(ObjectOps.newInstance(Aclass.class, "IOI", 123, res, true)).

Because no constructor is inserted, the arguments remain Java literals which, when trying to find an exact match, can be converted to any compatible progress type by ControlFlowOps.normalizeType() method. If the method with the right type is not declared above any other method with a compatible type then the wrong method would be matched by FWD.

OK, so this is about NEW function, and looks like we must wrap the literals with the parameter types for the expected constructor call. If this is not an easy fix, delay it.

What I need for now is the SUPER and THIS-OBJECT statements for constructor calls to work properly (and super(this-object) also).

Constantin Asofiei wrote:

What I need for now is the SUPER and THIS-OBJECT statements for constructor calls to work properly (and super(this-object) also).

Commited rev. 15843 on branch 9841a. This revision introduces full support for those cases with the limitations as described below.

• The current implementation could fail for classes which declare constructors with compatible datatypes. Fail comes from the process of method resolution and not super/this-object methods.
• Still regarding the process of method resolution, in some cases the class of the argument that dictates the method signature is found to be com.goldencode.p2j.util.ObjectVar while the candidate type is com.goldencode.p2j.util.object which makes the resolution process fail.
• Both of these cases are more likely to be encountered in classes which declare a large number of similar constructors.

Constantin Asofiei wrote:

OK, so this is about NEW function, and looks like we must wrap the literals with the parameter types for the expected constructor call.

Committed rev. 15844 on branch 9841a. This revision introduces a fix for the behavior of NEW function by wrapping literals given as arguments.
Note that this problem persists.

in some cases the class of the argument that dictates the method signature is found to be com.goldencode.p2j.util.ObjectVar while the candidate type is com.goldencode.p2j.util.object which makes the resolution process fail.

Paul Bodale wrote:

in some cases the class of the argument that dictates the method signature is found to be com.goldencode.p2j.util.ObjectVar while the candidate type is com.goldencode.p2j.util.object which makes the resolution process fail.

I assume this is runtime? These two should be compatible.

• oo_references.rules - you are calling wrap_argument while on the argument, and wrap_argument is meant to be called on the function AST.
• literas.rules - if upPath(this, prog.func_class, prog.lparens, prog.parameter)) is meant for NEW function, add a check for it with type_pair
• the change in oo_call_type I proposed is wrong - we can't match on the PARAMETER, we need to match only on the STATEMENT/{KW_SUPER|KW_THIS_OBJ} I think. Otherwise, it will break cases like this in base_structure.xml,
  

           <rule>evalLib("function_calls") or evalLib("oo_call_type", this, null)
              <action>deleteScope("function")</action>
           </rule>
  

  
  as this will match on each and every argument passed to SUPER or THIS-OBJECT.
• all files need history entries

Commited rev. 15845 to branch 9841a with the following changes.

Constantin Asofiei wrote:

• oo_references.rules - you are calling wrap_argument while on the argument, and wrap_argument is meant to be called on the function AST.

Structure of a NEW statement and structure of a normal method call:

[java] expression [EXPRESSION]:403726925861 @0:0
[java]    NEW [FUNC_CLASS]:403726925862 @4:9
[java]       task.ClsA [CLASS_NAME]:403726925864 @4:13
[java]       ( [LPARENS]:403726925865 @4:22
[java]          parameter [PARAMETER]:403726925866 @0:0
[java]             1 [NUM_LITERAL]:403726925867 @4:23
[java] ==============================================================================================================
[java] assignment [ASSIGNMENT]:403726925873 @0:0
[java]    expression [EXPRESSION]:403726925874 @0:0
[java]       : [OBJECT_INVOCATION]:403726925875 @7:6
[java]          aInst [VAR_CLASS]:403726925876 @7:1
[java]          voidMthd [OO_METH_VOID]:403726925877 @7:7
[java]             "a" [STRING]:403726925879 @7:16

Calling wrap_argument on the NEW node either doesn't work or produces these errors:

[java] EXPRESSION EXECUTION ERROR:
[java] ---------------------------
[java] evalLib("literals", ref.type)
[java]                         ^  { Cannot invoke "antlr.collections.AST.getType()" because the return value of "com.goldencode.expr.CompiledExpression.getVar(int)" is null }
[java] ---------------------------
[java] EXPRESSION EXECUTION ERROR:
[java] ---------------------------
[java] execLib("wrap_argument", copy)
[java] ^  { Expression execution error @1:25 }
[java] ---------------------------
[java] ERROR:
[java] com.goldencode.p2j.pattern.TreeWalkException: ERROR!  Active Rule:
[java] -----------------------
[java]       RULE REPORT      
[java] -----------------------
[java] Rule Type :   WALK
[java] Source AST:  [ NEW ] BLOCK/CLASS_DEF/BLOCK/METHOD_DEF/BLOCK/ASSIGNMENT/ASSIGN/EXPRESSION/FUNC_CLASS/ @29:15 {146028888241}
[java] Copy AST  :  [ NEW ] BLOCK/CLASS_DEF/BLOCK/METHOD_DEF/BLOCK/ASSIGNMENT/ASSIGN/EXPRESSION/FUNC_CLASS/ @29:15 {146028888241}
[java] Condition :  evalLib("literals", ref.type)
[java] Loop      :  false
[java] --- END RULE REPORT ---

I also tried matching on the child of NEW function and it didn't work. Seems that the function wrap_argument was not meant for the NEW statement and because it doesn't have the same structure as a normal oo_call_type call it needs sepparate processing.

• literas.rules - if upPath(this, prog.func_class, prog.lparens, prog.parameter)) is meant for NEW function, add a check for it with type_pair

Extra check added.

• the change in oo_call_type I proposed is wrong [...]

Made the following modification to the function:

-         upPath(ooref, prog.statement, prog.kw_super, prog.lparens, prog.parameter) or
-         upPath(ooref, prog.statement, prog.kw_this_obj, prog.lparens, prog.parameter)
+         upPath(ooref, prog.statement, prog.kw_super)                               or
+         upPath(ooref, prog.statement, prog.kw_this_obj)

• all files need history entries

Should I insert 2 entries one for modifications related to this task and one for modifications related to wrapping NEW function's arguments?

Paul Bodale wrote:

Should I insert 2 entries one for modifications related to this task and one for modifications related to wrapping NEW function's arguments?

One entry is enough, i.e. H00n, with date/etc, but the comment needs to describe all changes.

Commited rev. 15846 to branch 9841a.

As we've discussed, this revision reverts the changes made to the oo_call_type function and resolves the problem with THIS-OBJECT reference given as argument not converting properly.

Constantin, I've encountered some problems after the last commit but it might've been because of the problems I had with the testcases project.
After I've set up the testcases project again, I've committed rev. 15847 on branch 9841a (hopefully with the right changes this time). If you'll have time, I suggest to make a test locally on your computer to make sure there's nothing wrong with current implementation, otherwise we might ask a colleague.

I'll send you an email with the details.

Commited rev. 15848 on branch 9841a. This revision solves the following issue:

in some cases the class of the argument that dictates the method signature is found to be com.goldencode.p2j.util.ObjectVar while the candidate type is com.goldencode.p2j.util.object which makes the resolution process fail.

Paul, please try this with your branch:

def private var s1 as com.goldencode.util.CaseInsensitiveHashMap.
s1 = new com.goldencode.util.CaseInsensitiveHashMap(true).

I think we can't wrap the arguments for a NEW with a native Java type.

Do we still need the wrap_parameter change for NEW, after the change from oo_call_type was removed? Because also now it wraps every literal in a c'tor like new integer(1)

Constantin Asofiei wrote:

Paul, please try this with your branch:
[...]

Code converts to this:

@LegacySignature(type = Type.VARIABLE, name = "s1")
private jobject<? extends com.goldencode.util.CaseInsensitiveHashMap> s1 = UndoableFactory.jobject(com.goldencode.util.CaseInsensitiveHashMap.class);

@LegacySignature(type = Type.MAIN, name = "task/play.p")
public void execute()
{
   externalProcedure(Play.this, new Block((Body) () -> 
   {
      s1.assign(new com.goldencode.util.CaseInsensitiveHashMap(new boolean(true)));
   }));
}

Indeed there is a problem.

Do we still need the wrap_parameter change for NEW, after the change from oo_call_type was removed? Because also now it wraps every literal in a c'tor like new integer(1)

The reason behind this constructor wrapping was for the cases where a java literal is given as argument.

Take for example native java int. This could be considered as a DECIMAL or INTEGER. If there are 2 methods one expecting integer and the other decimal, FWD would match the first one that comes across which might not be the one expected. That is the reason for which two of the tests I ran were failing.

Paul Bodale wrote:

Take for example native java int. This could be considered as a DECIMAL or INTEGER. If there are 2 methods one expecting integer and the other decimal, FWD would match the first one that comes across which might not be the one expected. That is the reason for which one of the tests I ran was failing.

• if this is a NEW <native-java-type> then do not wrap the arguments (this is mandatory to fix the problem in previous note)
• for the NEW statement, I assume we have the target? If so, lets see if we can find a way to wrap the argument only and only if the argument and parameter do not convert to the same BDT (i.e. argument is int literal and parameter is DECIMAL, as you say).

Constantin, following our discussion I made a simple case for testing:

CLASS task.ClsA:
    CONSTRUCTOR PUBLIC ClsA (INPUT a AS DECIMAL):
        MESSAGE "Matched DECIMAL constructor.".
    END CONSTRUCTOR.
    CONSTRUCTOR PUBLIC ClsA (INPUT a AS INTEGER):
        MESSAGE "Matched INTEGER constructor.".
    END CONSTRUCTOR.
END CLASS.

DEFINE VARIABLE a AS CLASS task.ClsA.
a = NEW task.ClsA (1).

The ast of the NEW function is:

[java] NEW [FUNC_CLASS]:150323855396 @13:5
[java]       (builtin-cls=false, access-mode=2130, tempidx-file=./task/ClsA.cls, tempidx=4, support_level=16400, found-in-source-file=./task/ClsA.cls, signature=clsa(KW_INPUT integer), found-in-cls=task.ClsA, returnsunknown=false, is-enum=false, support_level-byrule=gaps/gap_analysis_marking.xml:435, oldtype=742, is-java=false, builtin=true, source-file=./task/ClsA.cls, static=false, param_modes=I, indirect-dotnet=false, dotnet-cls=false, qualified=task.clsa, is-class=true, is-interface=false)
[java]    task.ClsA [CLASS_NAME]:150323855398 @13:9
[java]          (source-file=./task/ClsA.cls, builtin-cls=false, is-enum=false, indirect-dotnet=false, dotnet-cls=false, oldtype=2982, is-java=false, qualified=task.clsa, is-interface=false, is-class=true)
[java]    ( [LPARENS]:150323855400 @13:19
[java]          (support_level=16400, support_level-byrule=gaps/gap_analysis_marking.xml:435)
[java]       parameter [PARAMETER]:150323855401 @0:0
[java]             (jtype=integer, parmtype=706, param_index=0)
[java]          1 [NUM_LITERAL]:150323855402 @13:20
[java]                (parmtype=706, is-literal=true, support_level=16400, param_index=0, support_level-byrule=gaps/gap_analysis_marking.xml:435)

The signature annotation and also the parameter's jtype annotation suggests that the integer version should be used but FWD still matches on decimal constructor (sometimes). I don't think that changing the rules would help avoid this problem (or at least I don't see how it would help). As I see it the problem comes from the process of method resolution specifically ControlFLowOps.resolveLegacyEntry method.
Since this problem can occur only for java literals that can be "normalized" into more than one progress data type then maybe we could modify the method to handle java int and String literals differently?

Made the following changes to resolveLegacyEntry the method:

=== modified file 'src/com/goldencode/p2j/util/ControlFlowOps.java'
--- old/src/com/goldencode/p2j/util/ControlFlowOps.java    2025-04-10 08:50:18 +0000
+++ new/src/com/goldencode/p2j/util/ControlFlowOps.java    2025-04-14 09:40:12 +0000
@@ -6147,8 +6147,9 @@

       if (overloads.size() > 1)
       {
-         // check if there is an exact match
-         for (InternalEntry ie : overloads)
+         List<InternalEntry> overloadsCopy = new LinkedList<>(overloads);
+
+         for (InternalEntry ie : overloadsCopy)
          {
             Method m = ie.getMethod();
             Class<?>[] mtypes = m == null ? null : m.getParameterTypes();
@@ -6177,15 +6178,21 @@
                }
             }

-            if (ok)
+            if (!ok)
             {
-               return ie;
+               overloadsCopy.remove(ie);
             }
          }
+
+         // check if there is an exact match
+         if (overloadsCopy.size() == 1)
+         {
+            return overloadsCopy.get(0);
+         }

          // TODO: before fuzzy searching, we need exact matches in super-classes; if none found, perform 
          // fuzzy match 
-         
+         // here use the overloads list before the last operation
          FuzzyMethodRt fuzzy = new FuzzyMethodRt((Class<? extends _BaseObject_>) type, overloads);

          InternalEntry ie = fuzzy.lookup(thisClass, name, isStatic, modes, origArgs);

I tested this method and now the method matched is always the wrong one.

Does parse time resolve the c'tor via exactMethodLookup or fuzzyMethodLookup?

Constantin Asofiei wrote:

Does parse time resolve the c'tor via exactMethodLookup or fuzzyMethodLookup?

How can I check this? I tried to look around but can't find the right place

Debug ClassDefinition.lookupMethodWorker.

Constantin Asofiei wrote:

Does parse time resolve the c'tor via exactMethodLookup or fuzzyMethodLookup?

Short answer would be that it's via exactMethodLookup. First all the constructors are put in a list then it is filtered with the java Stream API with the help of a simple lambda expression that checks the equality of the candidates expressed through a custom class which is ParameterKey.

We probably should leave behind a matchType annotation at the call site that records "fuzzy" or "exact".

Paul, I understand about the decimal and integer c'tors and int literal. But I don't think this is part of the original request of this task, right? The SUPER/THIS-OBJECT statements.

Lets get the NEW issue into another task and we'll work there, we need #9841 in trunk ASAP.

Constantin Asofiei wrote:

Paul, I understand about the decimal and integer c'tors and int literal. But I don't think this is part of the original request of this task, right? The SUPER/THIS-OBJECT statements.

I agree. I just committed rev. 15849 on branch 9841a which reverts changes made to wrap literals given as arguments to a NEW function. I'll also create a new issue on redmine where I'll put all the details of this secondary problem.

So rules/convert/literals.rules has no changes compared to rev 15840? If so, do a bzr revert -r 15840 rules/convert/literals.rules

Constantin Asofiei wrote:

So rules/convert/literals.rules has no changes compared to rev 15840? If so, do a bzr revert -r 15840 rules/convert/literals.rules

Commited rev. 15950 on branch 9841a that reverts unwanted changes to convert/literals.rules .