Fix wrong invalid conditional expression

[lefessan]

IF V ZERO AND W EQUAL 1 is interpreted as invalid when it should be legal

The test added in the testsuite fails with previous versions.

[codecov-commenter]

Codecov Report

Merging #87 (a2102b0) into gcos4gnucobol-3.x (743651f) will decrease coverage by 0.03%. The diff coverage is 75.75%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@                  Coverage Diff                  @@
##           gcos4gnucobol-3.x      #87      +/-   ##
=====================================================
- Coverage              65.19%   65.16%   -0.03%     
=====================================================
  Files                     31       31              
  Lines                  58363    58370       +7     
  Branches               15377    15375       -2     
=====================================================
- Hits                   38050    38039      -11     
- Misses                 14382    14401      +19     
+ Partials                5931     5930       -1     

Impacted Files	Coverage Δ
cobc/typeck.c	64.66% <75.00%> (-0.27%)	:arrow_down:
cobc/tree.c	74.07% <100.00%> (+0.10%)	:arrow_up:

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[GitMensch]

Why should IF VAR ZERO be valid?

[lefessan]

Why should IF VAR ZERO be valid?

It is already the case, GnuCOBOL allows IF VAR IS ZERO as a valid expression, but not IF VAR IS ZERO AND ...

Isn't it 8.8.4.7.2 of ISO 1989:2023 ?

[GitMensch]

You confuse me, your previous version shows that after your change IF VAR ZERO would compile, but the operator is mandatory, so this is wrong and should raise an error.

IF VAR IS ZERO AND VAR2 IS NOT ZERO should of course work, and I believe it does already.

[lefessan]

Actually, I wonder if the operator is mandatory: I made this patch because we have some code without the operator (that used to compile on GCOS) and ISO 1989:2013 does not underline the IS, so it should be optional, which is already the case in GnuCOBOL (I didn't modify the parser, and the POSITIVE case works without my patch).

IF VAR IS ZERO AND VAR2 IS NOT ZERO should of course work, and I believe it does already.

No, it complains about invalid conditional expression

[lefessan]

I created a bug report: https://sourceforge.net/p/gnucobol/bugs/875/

[lefessan]

I think there was a related bug on the sourceforge bugtracker, would it be possible to merge this PR too ?

[GitMensch]

:-) this shows that there is no Changelog entry in this PR as this would have a reference to the bug issue :-)

Yes, there is - it is https://sourceforge.net/p/gnucobol/bugs/880/ and it actually should be fixed before 3.2 final, but it may has to wait until after the RC3.

As far as I remember there was an issue with this change, I'd have to recheck.

Apart from the missing Changelog - is 880 the same as youz 875?

[lefessan]

This new version should fix 875 and 880. 880 is a different bug, it is because NOT EQUAL was not translated to ~ in the parser, so it has to be done in cb_build_expr. When fixing that, I found another bug in the testsuite: PIC 99 >= 99 would print a warning that it may always be false, when it should only be printed for PIC 99 > 99. It is also fixed in the PR.

[lefessan]

@GitMensch Do you think this PR is ready for merge ? (after fixing the ChangeLog conflict)

[GitMensch]

I'll try to have a look at this soon (bit that may still take some time as I'm not at the PC).

[GitMensch]

When fixing that, I found another bug in the testsuite: PIC 99 >= 99 would print a warning that it may always be false, when it should only be printed for PIC 99 > 99. It is also fixed in the PR.

Actually this was added (and therefore was on purpose) - we may could present a different message here, but the point is that the ">" part can never be true, and this is an important information because it hints at possibly "unexpected" PIC or mistyped condition.

[lefessan]

What you mean is that the warning is triggered because >= should be replaced by EQUAL as the > part is not possible... I see

[lefessan]

I think that, except the two test cases that I am unable to generate without deeper thinking, I addressed all your requests.

[GitMensch]

Please try to push that soon, as it should be part of RC3.

[GitMensch]

Out of interest: does this bad program still compile?

       IDENTIFICATION DIVISION.
       PROGRAM-ID.    CHECKCOND.
       DATA DIVISION.
       WORKING-STORAGE SECTION.
       01  VAR1                    PIC X.
                88 VAR1-K VALUE 'K'
       01  VAR2                    PIC X.
                88 VAR2-K VALUE 'K'

       PROCEDURE DIVISION.
       MAIN-PROGRAM SECTION.
       BUG.
           IF VAR1-K AND NOT = VAR2-K
              DISPLAY "INVALID" UPON STDERR.
           GOBACK.

Please add this to the testsuite, either with not compiling or as expected failure.

[lefessan]

While trying to add another test case on conditions with numeric operators, I found a segfault in the compiler in decimal_expand(), hence another fix in this PR.

[GitMensch]

Last thing to re-review is the new setting of cb_error_node in one case - I trust you make the right decision on how to tackle that - feel free to directly commit the result to svn.

[GitMensch]

When everything passes - feel free to check the result in.

[lefessan]

Merged in SVN, thanks for the reviews !