vermaseren / form

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

Print a warning when saving expr with too-long names #500

Closed jodavies closed 5 months ago

jodavies commented 5 months ago

This addresses #192 and #334 .

Terminating on this condition will certainly break people's existing FORM scripts.

At least print a warning, since one can certainly produce bugs by using names over MAXENAME (16) characters.

An example bug:

#-
Off statistics;

Global abcdefghijklmnop = 16;
Global abcdefghijklmnopq = 17;
Global abcdefghijklmnopqr = 18;
Global abcdefghijklmnopqrs = 19;
Global abcdefghijklmnopqrst = 20;
Global abcdefghijklmnopqrstu = 21;

Print +s;
.store

*Save test.res, abcdefghijklmnop, abcdefghijklmnopq;
Save test.res;
.end

then

#-
Off Statistics;

Load test.res;

Local Abcdefghijklmnop      = abcdefghijklmnop;
Local Abcdefghijklmnopq     = abcdefghijklmnopq;
Local Abcdefghijklmnopqr    = abcdefghijklmnopqr;
Local Abcdefghijklmnopqrs   = abcdefghijklmnopqrs;
Local Abcdefghijklmnopqrst  = abcdefghijklmnopqrst;
*Local Abcdefghijklmnopqrstu = abcdefghijklmnopqrstu;

Print +s;
.end

which gives:

FORM 4.1 (Apr 26 2024, v4.1-20131025-620-ga113449-dirty)  Run: Fri Apr 26 15:09:02 2024
    #-
 abcdefghijklmnop loaded
 abcdefghijklmnopq loaded
 abcdefghijklmnopqr loaded
 abcdefghijklmnopqrs loaded
 abcdefghijklmnopqrstx loaded
 abcdefghijklmnopqrst loaded

   Abcdefghijklmnop =
       + 16
      ;

   Abcdefghijklmnopq =
       + 17
      ;

   Abcdefghijklmnopqr =
       + 18
      ;

   Abcdefghijklmnopqrs =
       + 19
      ;

   Abcdefghijklmnopqrst =
       + 21
      ;

  0.00 sec out of 0.00 sec

Here, abcdefghijklmnopqrst is loaded with the incorrect name abcdefghijklmnopqrstx, and abcdefghijklmnopqrstu is loaded as abcdefghijklmnopqrst. Then Abcdefghijklmnopqrst has value 21 and not 20.

With the patch, upon saving, you get:

test.frm Line 15 --> Warning: saved expression name over 16 char: abcdefghijklm
nopq
test.frm Line 15 --> Warning: saved expression name over 16 char: abcdefghijklm
nopqr
test.frm Line 15 --> Warning: saved expression name over 16 char: abcdefghijklm
nopqrs
test.frm Line 15 --> Warning: saved expression name over 16 char: abcdefghijklm
nopqrst
test.frm Line 15 --> Warning: saved expression name over 16 char: abcdefghijklm
nopqrstu

What do you think? I used MesPrint and not Warning/HighWarning since those don't take arguments for the format string.

coveralls commented 5 months ago

Coverage Status

coverage: 48.546% (+0.04%) from 48.502% when pulling cf7e96c9f902e25f83e50fc757a5cada42a1e515 on jodavies:issue-192 into a113449cea96d21697a56a8cf0e277ee05cdd697 on vermaseren:master.

jodavies commented 5 months ago

Regarding Vlad's request for a mode which enforces the 16-char limit, I wonder: how about an On strict; mode which does this, but also can be used in the future for parser-related changes which make FORM more robust. In this mode, it would not be guaranteed that you have backward compatibility. If your old FORM scripts were relying on, essentially, buggy/undefined behaviour, they might break if you're using On strict;?

tueda commented 5 months ago

I think this PR is good to merge, but before the merging maybe you would like to squash the commits into one (or I can do that). You may also write some test cases, which could be a separate PR.

jodavies commented 5 months ago

A test case for this will be cleanest with #501 I think.

tueda commented 5 months ago

OK, then I will merge this PR and also #501 (I don't see any problem with this).

jodavies commented 2 months ago

This doesn't actually work quite as intended. If you use a loop to save multiple expressions, as in:

#do i = 1,2
   Save test-`i'.res, abcdefghijklmnop`i';
#enddo

you don't get two messages, but rather:

test-2.frm Line 18 --> Warning: saved expr name over 16 char: abcdefghijklmnop1
++++Errors in Loop

This seems to be due to the presence of the & at the beginning of the format string (which prints the filename and line number).