Обсуждение: Clean up optional rules in grammar
There are a number of rules like this in the grammar:
opt_foo: FOO
| /*EMPTY*/
;
And there are some like this:
opt_foo: FOO {}
| /*EMPTY*/ {}
;
and some even like this:
%type <node> opt_foo
opt_foo: FOO { $$ = NULL; }
| /*EMPTY*/ { $$ = NULL; }
;
(I mean here specifically those rules where FOO is a noise word and the
actions are the same in each branch.)
It's obviously confusing to have multiple different styles to do the
same thing. And these extra rules (including the empty ones) also end
up in the output, so they create more work down the line.
The attached patch cleans this up to make them all look like the first
style.
Вложения
On 11/11/2020 11:12, Peter Eisentraut wrote: > The attached patch cleans this up to make them all look like the first > style. +1 - Heikki
On 11/11/20 10:12 AM, Peter Eisentraut wrote:
> There are a number of rules like this in the grammar:
>
> opt_foo: FOO
> | /*EMPTY*/
> ;
>
> And there are some like this:
>
> opt_foo: FOO {}
> | /*EMPTY*/ {}
> ;
>
> and some even like this:
>
> %type <node> opt_foo
>
> opt_foo: FOO { $$ = NULL; }
> | /*EMPTY*/ { $$ = NULL; }
> ;
>
> (I mean here specifically those rules where FOO is a noise word and the
> actions are the same in each branch.)
>
> It's obviously confusing to have multiple different styles to do the
> same thing. And these extra rules (including the empty ones) also end
> up in the output, so they create more work down the line.
>
> The attached patch cleans this up to make them all look like the first
> style.
No objections, but could we also take this opportunity to standardize
the comment itself? Even in your patch there is a mix of spacing and
casing.
My preference is /* EMPTY */. That is, uppercase with spaces, but
whatever gets chosen is fine with me.
--
Vik Fearing
On 2020-11-11 12:43, Vik Fearing wrote: > No objections, but could we also take this opportunity to standardize > the comment itself? Even in your patch there is a mix of spacing and > casing. > > My preference is /* EMPTY */. That is, uppercase with spaces, but > whatever gets chosen is fine with me. Looks like /*EMPTY*/ is the most common right now. I agree this should be straightened out.
On Wed, Nov 11, 2020 at 5:13 AM Peter Eisentraut <peter.eisentraut@enterprisedb.com> wrote:
It's obviously confusing to have multiple different styles to do the
same thing. And these extra rules (including the empty ones) also end
up in the output, so they create more work down the line.
The attached patch cleans this up to make them all look like the first
style.
On 2020-11-11 11:54, Heikki Linnakangas wrote: > On 11/11/2020 11:12, Peter Eisentraut wrote: >> The attached patch cleans this up to make them all look like the first >> style. > > +1 done