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/*.
Date: Mon, 18 Jul 2022 18:36:50 -0400
Message-ID: <846745.1658183810@sss.pgh.pa.us> (raw)
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>
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
Attachments:fix-poor-perl-style-2.patch (text/x-diff, 3.7K) downloaddiff --git a/src/interfaces/ecpg/preproc/Makefile b/src/interfaces/ecpg/preproc/Makefileindex 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.plindex 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.plindex 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, '');
}
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: tgl@sss.pgh.pa.us, andres@anarazel.de, andrew@dunslane.net
Subject: Re: pgsql: ecpg: Output dir, source dir, stamp file argument for preproc/*.
In-Reply-To: <846745.1658183810@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox