Обсуждение: Erroring out on parser conflicts
While hacking on parser/gram.y just now I noticed in passing that the automatically generated ecpg parser had 402 shift/reduce conflicts. (Don't panic, the parser in CVS is fine.) If you don't pay very close attention, it is easy to miss this. Considering also that we frequently have to educate contributors that parser conflicts are not acceptable, should we try to error out if we see conflicts? Something like this could work: $(srcdir)/preproc.c: $(srcdir)/preproc.y $(BISON) -d $(BISONFLAGS) -o $@ $< 2>preproc.stderr cat preproc.stderr [ ! -s preproc.stderr ]
Peter Eisentraut <peter_e@gmx.net> writes: > While hacking on parser/gram.y just now I noticed in passing that the > automatically generated ecpg parser had 402 shift/reduce conflicts. > (Don't panic, the parser in CVS is fine.) If you don't pay very close > attention, it is easy to miss this. Considering also that we frequently > have to educate contributors that parser conflicts are not acceptable, > should we try to error out if we see conflicts? Would "%expect 0" produce the same result in a less klugy way? regards, tom lane
On Tuesday 25 November 2008 15:09:37 Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > While hacking on parser/gram.y just now I noticed in passing that the > > automatically generated ecpg parser had 402 shift/reduce conflicts. > > (Don't panic, the parser in CVS is fine.) If you don't pay very close > > attention, it is easy to miss this. Considering also that we frequently > > have to educate contributors that parser conflicts are not acceptable, > > should we try to error out if we see conflicts? > > Would "%expect 0" produce the same result in a less klugy way? Great, that works. I'll see about adding this to our parser files.
Peter Eisentraut wrote: > On Tuesday 25 November 2008 15:09:37 Tom Lane wrote: > > Peter Eisentraut <peter_e@gmx.net> writes: > > > While hacking on parser/gram.y just now I noticed in passing that the > > > automatically generated ecpg parser had 402 shift/reduce conflicts. > > > (Don't panic, the parser in CVS is fine.) If you don't pay very close > > > attention, it is easy to miss this. Considering also that we frequently > > > have to educate contributors that parser conflicts are not acceptable, > > > should we try to error out if we see conflicts? > > > > Would "%expect 0" produce the same result in a less klugy way? > > Great, that works. I'll see about adding this to our parser files. FYI, this is going to make it hard for developers to test CVS changes until they get their grammar cleaned up; perhaps add a comment on how to disable the check? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian <bruce@momjian.us> writes: > FYI, this is going to make it hard for developers to test CVS changes > until they get their grammar cleaned up; perhaps add a comment on how > to disable the check? Well, the point is that their grammar changes are broken if that check fails, so I'm not sure what the value of "testing" a known-incorrect grammar might be. It wouldn't necessarily act the same after being fixed. regards, tom lane
On 3 Dec 2008, at 03:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Bruce Momjian <bruce@momjian.us> writes: >> FYI, this is going to make it hard for developers to test CVS changes >> until they get their grammar cleaned up; perhaps add a comment on >> how >> to disable the check? > > Well, the point is that their grammar changes are broken if that check > fails, so I'm not sure what the value of "testing" a known-incorrect > grammar might be. It wouldn't necessarily act the same after being > fixed. > Well surely the c code the parser invokes will behave the same. A lot of c hackers are not bison grammar hackers. Even many of us former bison grammar hackers are way rusty. There have been a number of times when someone has posted an otherwise working patch with a grammar conflict you fixed Bruce surely nobody would object if you posted a path to add a comment. People would of course quibble with the wording but that's just par for the course. Perhaps something like "postgres jas a policy of maintaining zero parser conflicts. If you disable this for testing make sure you re- enable it and eliminate any conflicts. Or post to -hackers asking for advice" I'm not sure where to put a comment pointing them to the %expected line though. What does the error look like if they violate it? > regards, tom lane > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers