FWD » Core Development » Base Language

The 4GL testcase:

start.p:

DEFINE VARIABLE h AS HANDLE.
FUNCTION f RETURNS LOGICAL (INPUT x AS CHARACTER) IN h.
RUN start2.p PERSISTENT SET h.
MESSAGE f(?).

start2.p:

FUNCTION f RETURNS LOGICAL (INPUT x AS CHARACTER) :
  RETURN yes.
END FUNCTION.

The key line here is MESSAGE f(?) that gets converted to message(FUNC_CALL_SITE_1.clone().execute(logical.class));. When executing this testcase, the program throws a Mismatched number of parameters passed to routine f start2.p. (3234). The execute function should be execute(logical.class, new unknown()), or in other words, the current behavior is missing a second parameter, the unknown value that was passed as an argument in the 4GL code.

May I get a hint as to where I should look (i.e. in which file) first? Thank you!

Make sure you -Drules.tracing=true to see where/how this gets emitted. Also, add a normal function and check how the ? literal is being converted - that will allow you to backtrack and see why that rule doesn't include this function call.

Florin Eugen Rotaru wrote:

I will re-launch the conversion with to test this change.

The original note is #10109-91, if you want to check the before and after code.

My approach is not good, Instead of relying on the null classname, we might want to make sure that the classname is of the specific datatype expected by the function (in our case character).

Thus I am investigating why the classname isn't set properly at line
<action>classname = getNoteString("classname")</action> in literals.rules and if we can do that. This would mean that

message(FUNC_CALL_SITE_1.clone().execute(logical.class, new unknown()));

could become

message(FUNC_CALL_SITE_1.clone().execute(logical.class, new character())); or other types, depending of the function signature. This is because when calling f(?), FWD interprets initialized primitive datatypes as unknown values.

Radu, would this be OK for your testcase?

Florin Eugen Rotaru wrote:

Radu, would this be OK for your testcase?

Yes, I just checked and it's alright to have new character().

The issue has to be in this condition from base_structure.xml:

         <!-- TODO: some "virtual" OO method calls (e.g. Progress.Lang.Enum:GetEnum) have no original method
                    definition to reference, which means there is no refid; wrapping for these will not work
                    right now; to avoid bad behavior we must not try to process the dictionary lookup since
                    it will NOT have non-null values for this current call and may return an enclosing method
                    or function call's typelist/modes instead; this is why we exclude the missing refid case
                    -->
         <rule>
            parent != null                                                                        and
            parent.isAnnotation("refid")                                                          and
            ((evalLib("function_call_type", parent.type)                                  and 
              (!parent.isAnnotation("param_modes") or copy.indexPos > 0))                      or 
             (evalLib("oo_call_type", parent, copy) and !evalLib("is_builtin_cls", parent) and
              !evalLib("is_java_cls", parent) and isNote("param_index")))

...

In our case, this rule is not entered and some annotations are not added. Especially important is the one which should have been added at line

                  <rule>type == prog.string or type == prog.unknown_val
                     <action>putNote("classname", wrapper)</action>
                  </rule>

Florin Eugen Rotaru wrote:

The issue has to be in this condition from base_structure.xml:

[...]

In our case, this rule is not entered and some annotations are not added. Especially important is the one which should have been added at line
[...]

Greg, do you remember what the use case for the part !parent.isAnnotation("param_modes") or copy.indexPos > 0) was? Why do we want to avoid here the first arguments of calls for functions that have param_modes annotations?

Removing it solves the bug, I am now testing with a conversion.

I have adopted a different strategy after noticing that replacing INPUT x AS CHARACTER with INPUT-OUTPUT x AS CHARACTER in Radu's testcase, the issue seems to be gone.

=== modified file 'rules/annotations/annotations.xml'
--- old/rules/annotations/annotations.xml       2024-09-06 13:47:47 +0000
+++ new/rules/annotations/annotations.xml       2025-08-11 12:43:32 +0000
@@ -794,7 +794,7 @@
             <action>deftype = fref.getAnnotation("typelist", parIdx)</action>
             <action>defmodes = fref.getAnnotation("param_modes")</action>
             <action>defmode = charAt(defmodes, parIdx)</action>
-            <rule>defmode != "I" and (deftype == "integer" or deftype == "int64" or deftype == "character")
+            <rule>deftype == "logical" or deftype == "integer" or deftype == "int64" or deftype == "character" 
                <!-- required only for direct calls for which the argument type can't be narrowed via the BDT
                     hierarchy - like 'longchar' vs 'character' at OUTPUT mode 
                     See ClassDefinition.annotateCallSignature for the equivalent done for OO calls. -->

Waiting for the conversion to finish.

Florin, will this fix work if the function from the first note returns a character instead of a logical? I've encountered such a case in the application I am testing, so I was wondering if I should redo the conversion (I can't remember if I did the conversion when your changes got into 6506a).

Radu Apetrii wrote:

Florin, will this fix work if the function from the first note returns a character instead of a logical? I've encountered such a case in the application I am testing, so I was wondering if I should redo the conversion (I can't remember if I did the conversion when your changes got into 6506a).

Radu, yes, it will fix those instances too. I have seen cases such cases myself when comparing the sources. Probably you need to re-convert.

Code Review Task Branch 6506a Revision 15775

No objections. Please edit the comment at location so that it doesn't focus on output parameters.

Greg Shah wrote:

Code Review Task Branch 6506a Revision 15775

No objections. Please edit the comment at location so that it doesn't focus on output parameters.

Done. Committed rev 6506a/15776.

Any objections, Constantin?

For rev 15775/15776 - the change is for unknown literal, right? If so, can you right a test for decimal, also?

Because I wonder if this is not needed only if the argument is unknown literal, and not always. Also, we need to check for OO calls which emit in a similar way, if it works OK.

Constantin Asofiei wrote:

For rev 15775/15776 - the change is for unknown literal, right?

More specific, the change adds the <classname> tag to the .ast for function parameters of the type "INPUT", as it was missing only for this type. In practice, this was only affecting the unknown literals, because unlike the other literals/values, it could not infer a classname, and because it was null, it ended up being skipped altogether.

I tested for decimal, int, logical literals and it works fine.

I've also tested with OO calls and it also looks OK:

def var obj1 as D.

obj1 = new D().
obj1:m1(?).
obj1:m2(0.2).

Converts to:

         obj1.assign(ObjectOps.newInstance(com.hotel.D.class));
         obj1.ref().m1(new character());
         obj1.ref().m2(decimal.fromLiteral("0.2"));

Please use a DYNAMIC-INVOKE call.

Constantin Asofiei wrote:

Please use a DYNAMIC-INVOKE call.

Sure, I tested this code:

def var obj1 as D.

obj1 = new D().

DYNAMIC-INVOKE(obj1, "m1", ?).
DYNAMIC-INVOKE(obj1, "m1", 0.2).

Which converts to

      externalProcedure(Start1.this, new Block((Body) () ->
      {
         obj1.assign(ObjectOps.newInstance(com.apptechnologies.apppro.D.class));
         ObjectOps.invokeStandalone(obj1, "m1", "I", new unknown());
         ObjectOps.invokeStandalone(obj1, "m1", "I", decimal.fromLiteral("0.2"));
      }));

Constantin: Do you have any remaining concerns?

The changes should be in 6506b.

We will rely upon the testing of 6506b to test these changes.

The changes are already in 6506b, and from the testing that I did until now, things seemed really good.