FWD » Core Development » Base Language

Here is a test case:

define temp-table ttcust no-undo
   field code as character
   field qty  as integer.

create ttcust.

assign
   ttcust.code = (if "abc" <> ? then "abc" else ttcust.code)
   ttcust.qty  = (if 1 <> ? then 1 else ttcust.qty).

ttcust.code = ("ghi").

display ttcust.code ttcust.qty.

Text and NumberType wrapper types are not generated properly. As a result, the converted code ends up looking like this:

         batch(() -> 
         {
            ttcust.setCode(("abc"));
            ttcust.setQty((1));
         });

This results in the following compilation errors:

just-compile:
    [javac] Compiling 28 source files to /home/aog/gcd/dataset/build/classes
    [javac] Compiling 2 source files to /home/aog/gcd/dataset/build/classes
    [javac] /home/aog/gcd/dataset/src/com/goldencode/dataset/A.java:38: error: incompatible types: String cannot be converted to Text
    [javac]              ttcust.setCode(("abc"));
    [javac]                             ^
    [javac] /home/aog/gcd/dataset/src/com/goldencode/dataset/A.java:39: error: incompatible types: int cannot be converted to NumberType
    [javac]              ttcust.setQty((1));
    [javac]                            ^
    [javac] Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
    [javac] 2 errors

BUILD FAILED

This is the original AST for ttcust.code = (if "abc" <> ? then "abc" else ttcust.code):

        <ast col="16" id="12884901927" line="8" text="=" type="ASSIGN">
          [...]
          <ast col="4" id="12884901930" line="8" text="ttcust.code" type="FIELD_CHAR">
            [...]
          </ast>
          <ast col="0" id="12884901931" line="0" text="expression" type="EXPRESSION">
            [...]
            <ast col="18" id="12884901932" line="8" text="(" type="LPARENS">
              [...]
              <ast col="38" id="12884901943" line="8" text="&quot;abc&quot;" type="STRING">
                [...]
              </ast>
            </ast>
          </ast>
        </ast>

In annotations/cleanup.rules, we reduce expressions to a single output when possible, but the LPARENS node remains. Later, when literal wrapping should be performed in convert/literals, the isAssign = upPath(this, prog.assign, prog.expression) check fails because of the extra layer added by LPARENS.

I created task branch 11977a and committed a fix for this issue in rev16800. Now we drop the parenthesis node whenever a single leaf node is all that remains.

This is how the AST looks after the fix:

        <ast col="16" id="12884901927" line="8" text="=" type="ASSIGN">
          [...]
          <ast col="4" id="12884901930" line="8" text="ttcust.code" type="FIELD_CHAR">
            [...]
          </ast>
          <ast col="0" id="12884901931" line="0" text="expression" type="EXPRESSION">
            [...]
            <ast col="38" id="12884901943" line="8" text="&quot;abc&quot;" type="STRING">
              [...]
            </ast>
          </ast>
        </ast>

And here is the generated output:

         batch(() -> 
         {
            ttcust.setCode(new character("abc"));
            ttcust.setQty(new integer(1));
         });

So, this testcase is resolved in 11977a/16800.

I noticed that Ovidiu worked in this area as well.
Ovidiu, please review these changes.

I understand that the trunk was expecting to reduce the assignment to something like:

ttcust.code = ("abc")

The changes are good. It should work as described.
To note: ref.numChildren == 1 is always true (if lparens doesn't have a exactly one child then there is a problem) and can be dropped.

PS: maybe that strange condition was 'composed' using includes? Or it was simply written to comment out some code - I do sometime add the equivalent of if false in front of a statement to temporarily disable it.

Ovidiu Maxiniuc wrote:

I understand that the trunk was expecting to reduce the assignment to something like:[...]
How will these expressions convert?

Yes, but those expressions written directly in the OE code are handled in fixups/normalize_expressions, where an LPARENS node is stripped if it wraps a single leaf, which is not the case for the original test case. Instead, the current reduction occurs in annotations/cleanup, which runs later during the Code Annotations phase. That's why those expressions convert fine today, whereas the test case does not.

Your changes are limited to reduced ternary cases. I am just thinking if we should move this code in a more generic place.

Indeed. I found another place in the same file (annotations/cleanup) that reduces EQUALS/NOT_EQ against an unknown literal to BOOL_TRUE/BOOL_FALSE. I'm moving this task back to WIP to find a more generic place for these changes. Here is a test case that currently fails with 11977a:

define temp-table ttcust no-undo field flag as logical.
create ttcust.
ttcust.flag = ("abc" <> ?).

Having this error:

    [javac] /home/aog/gcd/dataset/src/com/goldencode/dataset/A.java:30: error: incompatible types: boolean cannot be converted to logical
    [javac]          ttcust.setFlag((true));
    [javac]                          ^
    [javac] Note: Some messages have been simplified; recompile with -Xdiags:verbose to get full output
    [javac] 1 error

PS: maybe that strange condition was 'composed' using includes? Or it was simply written to comment out some code - I do sometime add the equivalent of if false in front of a statement to temporarily disable it.

That's right. The original code that reproduced this issue used includes for that conditional clause. However, the behavior remains the same whether it's written directly in OE or composed via includes.

I've moved the fix to a generic location after all possible reductions in rev16801. Here is a test case containing all the scenarios we discussed so far, which currently passes in 11977a:

define temp-table ttcust no-undo
   field code as character
   field qty  as integer
   field flag as logical.

create ttcust.

assign
   ttcust.code = (if "abc" <> ? then "abc" else ttcust.code)
   ttcust.qty  = (if 1 <> ? then 1 else ttcust.qty).

ttcust.code = ("ghi").
ttcust.flag = ("abc" <> ?).

display ttcust.code ttcust.qty ttcust.flag.

Ovidiu, please review.

Very well! Let's test it.

You can merge this to trunk next.

Branch 11977a was merged into trunk as rev. 16813 and archived.