Re: pgsql: ecpg: Output dir, source dir, stamp file argument for preproc/*. - Mailing list pgsql-committers

I wrote:
> Andres Freund <andres@anarazel.de> writes:
>> Grepping for 'no warnings' shows that check_rules.pl has the same
>> issue. Perhaps worth fixing at the same time, given how closely related they
>> are?

> Ah, I didn't think to look around.  I'll see what I can do there.

Oh, that one's only hiding *one* sloppy spot.  However, while testing
it I realized that the Makefile rule has two serious deficiencies:

* check_rules.pl not listed as a prereq, so nothing happens if you
change it and say "make".

* check_rules.pl should be run first.  As-is, if it complains,
preproc.y has already been updated and so another run will succeed.

You might wanna check your meson conversion for the same bugs.

            regards, tom lane

diff --git a/src/interfaces/ecpg/preproc/Makefile b/src/interfaces/ecpg/preproc/Makefile
index ec2359810e..3f33f4638f 100644
--- a/src/interfaces/ecpg/preproc/Makefile
+++ b/src/interfaces/ecpg/preproc/Makefile
@@ -64,9 +64,9 @@ preproc.h: preproc.c

 preproc.c: BISONFLAGS += -d

-preproc.y: ../../../backend/parser/gram.y parse.pl ecpg.addons ecpg.header ecpg.tokens ecpg.trailer ecpg.type
-    $(PERL) $(srcdir)/parse.pl --srcdir $(srcdir) --parser $< --output $@
+preproc.y: ../../../backend/parser/gram.y parse.pl check_rules.pl ecpg.addons ecpg.header ecpg.tokens ecpg.trailer
ecpg.type
     $(PERL) $(srcdir)/check_rules.pl --srcdir $(srcdir) --parser $<
+    $(PERL) $(srcdir)/parse.pl --srcdir $(srcdir) --parser $< --output $@

 # generate keyword headers
 c_kwlist_d.h: c_kwlist.h $(GEN_KEYWORDLIST_DEPS)
diff --git a/src/interfaces/ecpg/preproc/check_rules.pl b/src/interfaces/ecpg/preproc/check_rules.pl
index 58a755f454..23a3741993 100644
--- a/src/interfaces/ecpg/preproc/check_rules.pl
+++ b/src/interfaces/ecpg/preproc/check_rules.pl
@@ -18,7 +18,6 @@

 use strict;
 use warnings;
-no warnings 'uninitialized';
 use Getopt::Long;

 my $srcdir  = '.';
@@ -142,7 +141,8 @@ while (<$parser_fh>)
             $in_rule = 0 if $arr[$fieldIndexer] eq ';';
         }
         elsif (($arr[$fieldIndexer] =~ '[A-Za-z0-9]+:')
-            || $arr[ $fieldIndexer + 1 ] eq ':')
+               || (   $fieldIndexer + 1 < $n
+                      && $arr[ $fieldIndexer + 1 ] eq ':'))
         {
             die "unterminated rule at grammar line $.\n"
               if $in_rule;
diff --git a/src/interfaces/ecpg/preproc/parse.pl b/src/interfaces/ecpg/preproc/parse.pl
index a15f563ad4..38548f24e6 100644
--- a/src/interfaces/ecpg/preproc/parse.pl
+++ b/src/interfaces/ecpg/preproc/parse.pl
@@ -14,7 +14,6 @@

 use strict;
 use warnings;
-no warnings 'uninitialized';
 use Getopt::Long;

 my $srcdir  = '.';
@@ -40,7 +39,8 @@ my $tokenmode             = 0;

 my (%buff, $infield, $comment, %tokens, %addons);
 my ($stmt_mode, @fields);
-my ($line,      $non_term_id);
+my $line = '';
+my $non_term_id;


 # some token have to be replaced by other symbols
@@ -195,6 +195,16 @@ sub main
         # Now split the line into individual fields
         my @arr = split(' ');

+        if (!@arr)
+        {
+            # empty line: in tokenmode 1, emit an empty line, else ignore
+            if ($tokenmode == 1)
+            {
+                add_to_buffer('orig_tokens', '');
+            }
+            next line;
+        }
+
         if ($arr[0] eq '%token' && $tokenmode == 0)
         {
             $tokenmode = 1;
@@ -341,7 +351,8 @@ sub main

             # Are we looking at a declaration of a non-terminal ?
             if (($arr[$fieldIndexer] =~ /[A-Za-z0-9]+:/)
-                || $arr[ $fieldIndexer + 1 ] eq ':')
+                || (   $fieldIndexer + 1 < scalar(@arr)
+                    && $arr[ $fieldIndexer + 1 ] eq ':'))
             {
                 $non_term_id = $arr[$fieldIndexer];
                 $non_term_id =~ tr/://d;
@@ -409,11 +420,13 @@ sub main
             if (   $copymode
                 && !$prec
                 && !$comment
+                && $fieldIndexer < scalar(@arr)
                 && length($arr[$fieldIndexer])
                 && $infield)
             {
                 if ($arr[$fieldIndexer] ne 'Op'
-                    && (   $tokens{ $arr[$fieldIndexer] } > 0
+                    && ((   defined $tokens{ $arr[$fieldIndexer] }
+                            && $tokens{ $arr[$fieldIndexer] } > 0)
                         || $arr[$fieldIndexer] =~ /'.+'/)
                     || $stmt_mode == 1)
                 {
@@ -472,11 +485,12 @@ sub include_addon
     my $rec = $addons{$block};
     return 0 unless $rec;

-    if ($rec->{type} eq 'rule')
+    my $rectype = (defined $rec->{type}) ? $rec->{type} : '';
+    if ($rectype eq 'rule')
     {
         dump_fields($stmt_mode, $fields, ' { ');
     }
-    elsif ($rec->{type} eq 'addon')
+    elsif ($rectype eq 'addon')
     {
         add_to_buffer('rules', ' { ');
     }
@@ -487,7 +501,7 @@ sub include_addon

     push(@{ $buff{$buffer} }, @{ $rec->{lines} });

-    if ($rec->{type} eq 'addon')
+    if ($rectype eq 'addon')
     {
         dump_fields($stmt_mode, $fields, '');
     }