vermaseren / form

The FORM project for symbolic manipulation of very big expressions
GNU General Public License v3.0
982 stars 118 forks source link

Introduction of preprocessor instruction #continuedo #443

Closed rldelgado closed 1 year ago

rldelgado commented 1 year ago
tueda commented 1 year ago

Thank you for starting this pull request! I was just wondering if you are still working on it or if you need any help moving forward (though I am not sure how it can be implemented).

This pull request does not work for, for example,

#do i=1,5
  #if `i' == 3
    #continuedo
  #endif
  #message `i'
#enddo
.end

which should correspond to the following C code:

#include <stdio.h>

int main() {
  for (int i = 1; i <= 5; i++) {
    if (i == 3) {
      continue;
    }
    printf("%d\n", i);  // 1, 2, 4, 5
  }
}
rldelgado commented 1 year ago

I see my error, @tueda . Many thanks for your help. Looking at function:

int TheDefine(UBYTE *s, int mode)

it looks like I should check execute first something like:

if ( AP.PreSwitchModes[AP.PreSwitchLevel] != EXECUTINGPRESWITCH ) return(0);
if ( AP.PreIfStack[AP.PreIfLevel] != EXECUTINGIF ) return(0);

in order do not execute the #continuedo in these conditions. Hence, I have introduced new commit a86dd9afe36607420c44c8e5c991b2597adc6a6b

Yes, help would be very welcomed. :)

tueda commented 1 year ago

The program

#continuedo
.end

leads to segfault, but FORM should give some error message in this case.

Would you also like to elaborate on the test cases and the manual? (Maybe these can be done by others. The same for possible extension of #continuedo <num>.) For tests, for example, you can insert the following to check/features.frm:

*--#[ Issue243_1 :
* #continuedo?
NF f;
L F = 1;
#do i=1,5
  #if `i' == 3
    #continuedo
  #endif
  multiply right, f(`i');
#enddo
chainin f;
P;
.end
assert succeeded?
assert result("F") =~ expr("f(1,2,4,5)")
*--#] Issue243_1 : 
*--#[ Issue243_2 :
#continuedo
.end
assert compile_error?
assert stdout =~ exact_pattern("#continuedo without #do")
*--#] Issue243_2 : 

(I assumed that the error message is #continuedo without #do).

You can test the specific test case like

check/check.rb --verbose sources/vorm ContinueDo_1

The documentation can be added into doc/manual/prepro.tex.

If you want, you can rebase and then force-push to edit/combine the commits. If you want, I can also use "merge squash" (to combine all commits into one) when merging.

rldelgado commented 1 year ago

I have identified the bug, @tueda , and modified the code accordingly. I have uploaded a new rebased commit that should work now.

Those failing checks, apparently I also have errors when I run make check on 242b17f89fd91463c78f87d9ef958d3a7d4252fe commit on master branch. Please, let me know if you see any bad behavior. What checks should worry me?

I will take a look at the manual and whether I find a way to implement #continuedo <num> shortly... The new checks appear to work:

rdelgado@ubuntu:~/git/form$ check/check.rb --verbose sources/form Issue243_1
FORM 5.0 (May 11 2023)                           Run: Thu May 11 23:52:34 2023
      #-
warning: failed to get the wordsize of 'sources/form'
warning: assuming wordsize = 4
Check /home/rdelgado/git/form/sources/form
FORM 5.0 (May 11 2023)                           Run: Thu May 11 23:52:34 2023
Loaded suite check/check
Started
Command: /home/rdelgado/git/form/sources/form 1.frm

===============================================================================
Issue243_1 (features.frm:1034) SUCCEEDED
===============================================================================
FORM 5.0 (May 11 2023)                           Run: Thu May 11 23:52:34 2023
    * #continuedo?
    NF f;
    L F = 1;
    #do i=1,5
      #if `i' == 3
        #continuedo
      #endif
      multiply right, f(`i');
    #enddo
    chainin f;
    P;
    .end

Time =       0.00 sec    Generated terms =          1
               F         Terms in output =          1
                         Bytes used      =         64

   F =
      f(1,2,4,5);

  0.00 sec out of 0.00 sec
===============================================================================

.
Finished in 0.004189615 seconds.
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1 tests, 2 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
238.69 tests/s, 477.37 assertions/s
rdelgado@ubuntu:~/git/form$ check/check.rb --verbose sources/form Issue243_2
FORM 5.0 (May 11 2023)                           Run: Thu May 11 23:52:40 2023
      #-
warning: failed to get the wordsize of 'sources/form'
warning: assuming wordsize = 4
Check /home/rdelgado/git/form/sources/form
FORM 5.0 (May 11 2023)                           Run: Thu May 11 23:52:40 2023
Loaded suite check/check
Started
Command: /home/rdelgado/git/form/sources/form 1.frm

===============================================================================
Issue243_2 (features.frm:1050) SUCCEEDED
===============================================================================
FORM 5.0 (May 11 2023)                           Run: Thu May 11 23:52:40 2023
    #continuedo
1.frm Line 2 ==> #continuedo without #do
    .end
Program terminating at 1.frm Line 1 --> 
  0.00 sec out of 0.00 sec
===============================================================================

.
Finished in 0.002409797 seconds.
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1 tests, 2 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
414.97 tests/s, 829.95 assertions/s
tueda commented 1 year ago

I think it is OK if the following check passes the 2 tests:

check/check.rb sources/vorm 'Issue243_*'

Other failing checks and warnings are due to FORM 5 source code.

Could you slightly arrange the commit message to fit it into a more standard convention? I mean

the 1st line is the commit message title, 72 characters or less
(the 2nd line must be blank)
detailed description from the 3rd line

I think this is a minimal set of rules (so to say, the 72/* rule) with which GitHub or git log --oneline works well (people usually try to fit into a stricter rule, the 50/72 rule).

Then, I will "rebase and merge" your pull request.

rldelgado commented 1 year ago

I have implemented the suggested changes. Now, the commit should be Ok. Please, check it. Many thanks for your help.

Best regards, Rafael Delgado López.

El 14/5/23 a las 1:50, Takahiro Ueda escribió:

I think it is OK if the following check passes the 2 tests:

|check/check.rb sources/vorm 'Issue243_*' |

Other failing checks and warnings are due to FORM 5 source code.

Could you slightly arrange the commit message to fit it into a more standard convention? I mean

|the 1st line is the commit message title, 72 characters or less (the 2nd line must be blank) detailed description from the 3rd line |

I think this is a minimal set of rules (so to say, the 72/* rule) with which GitHub or |git log --oneline| works well (people usually try to fit into a stricter rule, the 50/72 rule).

Then, I will "rebase and merge" your pull request.

— Reply to this email directly, view it on GitHub https://github.com/vermaseren/form/pull/443#issuecomment-1546770434, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMZNPMWVD6CYFML4QCXXLHTXGAM4FANCNFSM6AAAAAAW32DV7Q. You are receiving this because you authored the thread.Message ID: @.***>

tueda commented 1 year ago

Merged. Thank you for your effort!