From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: pgsql-committers@lists.postgresql.org,
        Andrew Dunstan <andrew@dunslane.net>
Subject: Re: pgsql: ecpg: Output dir, source dir,
 stamp file argument for preproc/*.
In-reply-to: <840982.1658182844@sss.pgh.pa.us>
References: <E1oDWqn-000ADD-Oh@gemulon.postgresql.org>
 <20220718211408.o2uyu5kvud56ihlj@awork3.anarazel.de>
 <822625.1658179641@sss.pgh.pa.us>
 <20220718215042.sl3hivoupdb7lkwv@awork3.anarazel.de>
 <838180.1658181982@sss.pgh.pa.us>
 <20220718221535.bf43zwf5rf2wi4wd@awork3.anarazel.de>
 <840982.1658182844@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Mon, 18 Jul 2022 18:20:44 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <846645.1658183762.0@sss.pgh.pa.us>
Date: Mon, 18 Jul 2022 18:36:50 -0400
Message-ID: <846745.1658183810@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/846745.1658183810%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <846645.1658183762.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable

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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-poor-perl-style-2.patch";
	charset="us-ascii"
Content-ID: <846645.1658183762.2@sss.pgh.pa.us>
Content-Description: fix-poor-perl-style-2.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/interfaces/ecpg/preproc/Makefile b/src/interfaces/ecpg/pr=
eproc/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 +=3D -d
 =

-preproc.y: ../../../backend/parser/gram.y parse.pl ecpg.addons ecpg.heade=
r 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.ad=
dons 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/e=
cpg/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  =3D '.';
@@ -142,7 +141,8 @@ while (<$parser_fh>)
 			$in_rule =3D 0 if $arr[$fieldIndexer] eq ';';
 		}
 		elsif (($arr[$fieldIndexer] =3D~ '[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/pr=
eproc/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  =3D '.';
@@ -40,7 +39,8 @@ my $tokenmode             =3D 0;
 =

 my (%buff, $infield, $comment, %tokens, %addons);
 my ($stmt_mode, @fields);
-my ($line,      $non_term_id);
+my $line =3D '';
+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 =3D split(' ');
 =

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

 			# Are we looking at a declaration of a non-terminal ?
 			if (($arr[$fieldIndexer] =3D~ /[A-Za-z0-9]+:/)
-				|| $arr[ $fieldIndexer + 1 ] eq ':')
+				|| (   $fieldIndexer + 1 < scalar(@arr)
+					&& $arr[ $fieldIndexer + 1 ] eq ':'))
 			{
 				$non_term_id =3D $arr[$fieldIndexer];
 				$non_term_id =3D~ 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] =3D~ /'.+'/)
 					|| $stmt_mode =3D=3D 1)
 				{
@@ -472,11 +485,12 @@ sub include_addon
 	my $rec =3D $addons{$block};
 	return 0 unless $rec;
 =

-	if ($rec->{type} eq 'rule')
+	my $rectype =3D (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, '');
 	}

------- =_aaaaaaaaaa0--