Keyword list sanity check

I had previously considered adding an assertion in the backend to
check they're sorted properly. That would be less formatting dependent
and would be only a couple lines of C.

I don't think we can do that with the gram.y check though.

--
Greg

On 28 Apr 2009, at 09:33, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com

Greg Stark wrote:

I had previously considered adding an assertion in the backend to check
they're sorted properly. That would be less formatting dependent and
would be only a couple lines of C.

I don't think we can do that with the gram.y check though.

Well, the ordering in gram.y is just pro forma anyway. Checking that all
keywords are present and correctly labeled in kwlist.h is more
important. What's still missing is to check that all keywords defined
with "%token <keyword>" in gram.y are present in one of the keyword lists.

Hmm, it just occurred to me that this script is only a few steps away
from actually generating kwlist.h from gram.y, instead of merely
cross-checking them.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Tue, Apr 28, 2009 at 11:57:19AM +0300, Heikki Linnakangas wrote:

Greg Stark wrote:

I had previously considered adding an assertion in the backend to
check they're sorted properly. That would be less formatting
dependent and would be only a couple lines of C.

I don't think we can do that with the gram.y check though.

Well, the ordering in gram.y is just pro forma anyway. Checking that
all keywords are present and correctly labeled in kwlist.h is more
important. What's still missing is to check that all keywords
defined with "%token <keyword>" in gram.y are present in one of the
keyword lists.

Hmm, it just occurred to me that this script is only a few steps
away from actually generating kwlist.h from gram.y, instead of
merely cross-checking them.

Single Point of Truth is good :)

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Tue, Apr 28, 2009 at 11:33:28AM +0300, Heikki Linnakangas wrote:

I wrote a little perl script to perform a basic sanity check to
keywords in gram.y and kwlist.h. It checks that all lists are in
alphabetical order, all keywords present in gram.y are listed in
kwlist.h in the right category, and conversely that all keywords
listed in kwlist.h are listed in gram.y.

It found one minor issue already:

$ perl src/tools/check_keywords.pl 'SCHEMA' after 'SERVER' in
unreserved_keyword list is misplaced

SERVER is not in the right place in gram.y, it should go between
SERIALIZABLE and SERVER. I'll fix that.

I'll put this into src/tools. It's heavily dependent on the format
of the lists in gram.y and kwlist.h but if it bitrots due to
changes in those files, we can either fix it or just remove it if
it's not deemed useful anymore.

Please clean up this code at least to the point where it's
strict-clean, which means putting "use strict;" right after the
shebang line and not checking it in until it runs that way.

I tried, but couldn't make heads or tails of the thing, given all the
unused- and similarly-named variables, failure to indent, etc. I
don't know how to put this gently, but if you checked in C code with
quality like this, you'd have it bounced with derision.

I'd also like to propose that "strict clean" be a minimum code quality
metric for any Perl code in our code base. A lot of what's in there
is just about impossible to maintain.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Tue, Apr 28, 2009 at 10:33 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

I wrote a little perl script to perform a basic sanity check to keywords in
gram.y and kwlist.h. It checks that all lists are in alphabetical order, all
keywords present in gram.y are listed in kwlist.h in the right category, and
conversely that all keywords listed in kwlist.h are listed in gram.y.

Friendly greetings !
Here is a new version of check_keywords.pl :
- perl -w and "use strict" enabled (and all the fixes that come with it)
- minor cleaning

--
Laurent Laborde
Sysadmin at jfg://networks
http://www.over-blog.com/

Laurent Laborde wrote:

On Tue, Apr 28, 2009 at 10:33 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

I wrote a little perl script to perform a basic sanity check to keywords in
gram.y and kwlist.h. It checks that all lists are in alphabetical order, all
keywords present in gram.y are listed in kwlist.h in the right category, and
conversely that all keywords listed in kwlist.h are listed in gram.y.

Friendly greetings !
Here is a new version of check_keywords.pl :
- perl -w and "use strict" enabled (and all the fixes that come with it)
- minor cleaning

Thanks, applied!

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

David Fetter wrote:

Please clean up this code at least to the point where it's
strict-clean, which means putting "use strict;" right after the
shebang line and not checking it in until it runs that way.

I tried, but couldn't make heads or tails of the thing, given all the
unused- and similarly-named variables, failure to indent, etc. I
don't know how to put this gently, but if you checked in C code with
quality like this, you'd have it bounced with derision.

Yep, I'm completely novice at perl. I've checked in Laurent's patch to
clean it up, which at least makes it strict-clean.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Thu, Apr 30, 2009 at 3:27 AM, David Fetter <david@fetter.org> wrote:

On Tue, Apr 28, 2009 at 11:33:28AM +0300, Heikki Linnakangas wrote:

I wrote a little perl script to perform a basic sanity check to
keywords  in gram.y and kwlist.h. It checks that all lists are in
alphabetical  order, all keywords present in gram.y are listed in
kwlist.h in the  right category, and conversely that all keywords
listed in kwlist.h are  listed in gram.y.

It found one minor issue already:

$ perl src/tools/check_keywords.pl 'SCHEMA' after 'SERVER' in
unreserved_keyword list is misplaced

SERVER is not in the right place in gram.y, it should go between
SERIALIZABLE and SERVER. I'll fix that.

I'll put this into src/tools. It's heavily dependent on the format
of  the lists in gram.y and kwlist.h but if it bitrots due to
changes in  those files, we can either fix it or just remove it if
it's not deemed  useful anymore.

Please clean up this code at least to the point where it's
strict-clean, which means putting "use strict;" right after the
shebang line and not checking it in until it runs that way.

And "use warnings;", too.

...Robert

On Thursday 30 April 2009 10:27:45 David Fetter wrote:

I'd also like to propose that "strict clean" be a minimum code quality
metric for any Perl code in our code base. A lot of what's in there
is just about impossible to maintain.

use strict and use warnings, I think, although with use warnings I have
occasionally run into the trouble of some old versions not supporting it (only
via perl -w).

On Apr 30, 2009, at 2:27 AM, David Fetter wrote:

Please clean up this code at least to the point where it's
strict-clean, which means putting "use strict;" right after the
shebang line and not checking it in until it runs that way.

I can take care of this, David. Shouldn't be too tough.

--
Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance

David Fetter wrote:

On Tue, Apr 28, 2009 at 11:33:28AM +0300, Heikki Linnakangas wrote:

I wrote a little perl script to perform a basic sanity check to
keywords in gram.y and kwlist.h. It checks that all lists are in
alphabetical order, all keywords present in gram.y are listed in
kwlist.h in the right category, and conversely that all keywords
listed in kwlist.h are listed in gram.y.

It found one minor issue already:

$ perl src/tools/check_keywords.pl 'SCHEMA' after 'SERVER' in
unreserved_keyword list is misplaced

SERVER is not in the right place in gram.y, it should go between
SERIALIZABLE and SERVER. I'll fix that.

I'll put this into src/tools. It's heavily dependent on the format
of the lists in gram.y and kwlist.h but if it bitrots due to
changes in those files, we can either fix it or just remove it if
it's not deemed useful anymore.

Please clean up this code at least to the point where it's
strict-clean, which means putting "use strict;" right after the
shebang line and not checking it in until it runs that way.

I tried, but couldn't make heads or tails of the thing, given all the
unused- and similarly-named variables, failure to indent, etc. I
don't know how to put this gently, but if you checked in C code with
quality like this, you'd have it bounced with derision.

I'd also like to propose that "strict clean" be a minimum code quality
metric for any Perl code in our code base. A lot of what's in there
is just about impossible to maintain.

Good suggestion; I see this has been done:

revision 1.2
date: 2009/04/30 10:26:35; author: heikki; state: Exp; lines: +24 -11
Clean up check_keywords.pl script, making it 'strict' and removing a few
leftover unused variables.

Laurent Laborde

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Apr 30, 2009, at 6:41 AM, Robert Haas wrote:

Please clean up this code at least to the point where it's
strict-clean, which means putting "use strict;" right after the
shebang line and not checking it in until it runs that way.

And "use warnings;", too.

I'll prob'ly come up with a policy file for Perl::Critic and a make
target for perlcritic.

xoa

--
Andy Lester => andy@petdance.com => www.theworkinggeek.com => AIM:petdance

Robert Haas wrote:

On Thu, Apr 30, 2009 at 3:27 AM, David Fetter <david@fetter.org> wrote:

On Tue, Apr 28, 2009 at 11:33:28AM +0300, Heikki Linnakangas wrote:

I wrote a little perl script to perform a basic sanity check to
keywords ?in gram.y and kwlist.h. It checks that all lists are in
alphabetical ?order, all keywords present in gram.y are listed in
kwlist.h in the ?right category, and conversely that all keywords
listed in kwlist.h are ?listed in gram.y.

It found one minor issue already:

$ perl src/tools/check_keywords.pl 'SCHEMA' after 'SERVER' in
unreserved_keyword list is misplaced

SERVER is not in the right place in gram.y, it should go between
SERIALIZABLE and SERVER. I'll fix that.

I'll put this into src/tools. It's heavily dependent on the format
of ?the lists in gram.y and kwlist.h but if it bitrots due to
changes in ?those files, we can either fix it or just remove it if
it's not deemed ?useful anymore.

Please clean up this code at least to the point where it's
strict-clean, which means putting "use strict;" right after the
shebang line and not checking it in until it runs that way.

And "use warnings;", too.

I now see at the top of the CVS file:

#!/usr/bin/perl -w

use strict;

so it seems it has both now.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Thu, Apr 30, 2009 at 11:39:33AM -0500, Andy Lester wrote:

On Apr 30, 2009, at 6:41 AM, Robert Haas wrote:

Please clean up this code at least to the point where it's
strict-clean, which means putting "use strict;" right after the
shebang line and not checking it in until it runs that way.

And "use warnings;", too.

I'll prob'ly come up with a policy file for Perl::Critic and a make
target for perlcritic.

The current code has a bunch of 5s in it, so it's a target-rich
environment :)

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Thu, Apr 30, 2009 at 09:40:50AM -0700, David Fetter wrote:

On Thu, Apr 30, 2009 at 11:39:33AM -0500, Andy Lester wrote:

On Apr 30, 2009, at 6:41 AM, Robert Haas wrote:

Please clean up this code at least to the point where it's
strict-clean, which means putting "use strict;" right after the
shebang line and not checking it in until it runs that way.

And "use warnings;", too.

I'll prob'ly come up with a policy file for Perl::Critic and a make
target for perlcritic.

The current code has a bunch of 5s in it, so it's a target-rich
environment :)

Here's a patch that gets it to pass perlcritic -4 and still (as far as
I can tell) work.

Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Apr 30, 2009, at 2:11 PM, David Fetter wrote:

Here's a patch that gets it to pass perlcritic -4 and still (as far as
I can tell) work.

Tell ya what. Let me at it and I'll give a larger, more inclusive
patch.

xoxo,
Andy

--
Andy Lester => andy@petdance.com => www.theworkinggeek.com => AIM:petdance

Peter Eisentraut wrote:

On Thursday 30 April 2009 10:27:45 David Fetter wrote:

I'd also like to propose that "strict clean" be a minimum code quality
metric for any Perl code in our code base. A lot of what's in there
is just about impossible to maintain.

use strict and use warnings, I think, although with use warnings I have
occasionally run into the trouble of some old versions not supporting it (only
via perl -w).

Right. I think strict mode is probably sufficient for utility code like
this.

Also note that we maintain the usual postgres indentation standards in
the perl MSVC stuff, by using perltidy thus:

perltidy -b -bl -nsfs -naws -l=100 -ole=unix *.pl *.pm

cheers

andrew