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.
Attachments:
0001-Clean-up-optional-rules-in-grammar.patchtext/plain; charset=UTF-8; name=0001-Clean-up-optional-rules-in-grammar.patch; x-mac-creator=0; x-mac-type=0Download+40-41
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.
+1 for standardizing in this area. It's worth noting that Bison 3.0
introduced %empty for this situation, which is self-documenting. Until we
get there, this is a good step.
--
John Naylor
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company