pgsql: Remove extra comma at end of enum list

Started by Magnus Haganderover 9 years ago16 messageshackers
Jump to latest
#1Magnus Hagander
magnus@hagander.net

Remove extra comma at end of enum list

C99-specific feature, and wasn't intentional in the first place.

Per buildfarm member mylodon

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/d97a59a4c5597af5f53869a5a1c753893752c66b

Modified Files
--------------
src/bin/pg_basebackup/walmethods.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Michael Paquier
michael@paquier.xyz
In reply to: Magnus Hagander (#1)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

(Moved to -hackers)

On Sun, Oct 23, 2016 at 10:57 PM, Magnus Hagander <magnus@hagander.net> wrote:

Remove extra comma at end of enum list

C99-specific feature, and wasn't intentional in the first place.

Per buildfarm member mylodon

This is the second time I see that in the last couple of weeks. Would
it be better to use some custom CFLAGS to detect that earlier in the
review process? I have never much used gcc's std or clang extensions
in this area. Has somebody any recommendations?
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Michael Paquier (#2)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

C99-specific feature, and wasn't intentional in the first place.

Per buildfarm member mylodon

This is the second time I see that in the last couple of weeks. Would
it be better to use some custom CFLAGS to detect that earlier in the
review process? I have never much used gcc's std or clang extensions
in this area. Has somebody any recommendations?

My 0.02€, I've used the following for years without problem, but it would
need testing with a several versions of gcc & clang before committing:

sh> gcc -std=c89/... ...
sh> clang -std=c89/... ...

And a free advice, hoping not to start a troll:

Even as conservative and old fashioned as I am, I was 18 in 1989 and had a
driving license. In 2017 C99 will also turn 18 and might be considered old
and stable enough for pg:-)

I find it annoying that "//" comments are not supported, or having to
declare variables at the beginning of a block instead of when first
used...

--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fabien COELHO (#3)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

Fabien COELHO wrote:

I find it annoying that "//" comments are not supported, or having to
declare variables at the beginning of a block instead of when first used...

I think some c99 features would be nice (variadic macros for one), but
those particular two get a big "meh" from me.

.oO( I wonder if it'd be possible to make ereport() an inline function ... )

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#4)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Fabien COELHO wrote:

I find it annoying that "//" comments are not supported, or having to
declare variables at the beginning of a block instead of when first used...

I think some c99 features would be nice (variadic macros for one), but
those particular two get a big "meh" from me.

Yeah. Personally, I'd want to continue the rule against // comments just
as a matter of maintaining stylistic consistency. Code that's got a
random mishmash of // and /* comments looks messy, if you ask me.

An alternative that would be worth considering is to adopt a uniform
rule of // for line-ending comments and /* for all other uses. We'd
have to teach pgindent about that, and I dunno how hard that is.

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

#6Stephen Frost
sfrost@snowman.net
In reply to: Tom Lane (#5)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Fabien COELHO wrote:

I find it annoying that "//" comments are not supported, or having to
declare variables at the beginning of a block instead of when first used...

I think some c99 features would be nice (variadic macros for one), but
those particular two get a big "meh" from me.

Yeah. Personally, I'd want to continue the rule against // comments just
as a matter of maintaining stylistic consistency. Code that's got a
random mishmash of // and /* comments looks messy, if you ask me.

+1

An alternative that would be worth considering is to adopt a uniform
rule of // for line-ending comments and /* for all other uses. We'd
have to teach pgindent about that, and I dunno how hard that is.

I'm not sure it's worth the resulting mess.

Thanks!

Stephen

#7Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#5)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

On Mon, Oct 24, 2016 at 03:03:19PM -0400, Tom Lane wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Fabien COELHO wrote:

I find it annoying that "//" comments are not supported, or having to
declare variables at the beginning of a block instead of when first used...

I think some c99 features would be nice (variadic macros for one), but
those particular two get a big "meh" from me.

Yeah. Personally, I'd want to continue the rule against // comments just
as a matter of maintaining stylistic consistency. Code that's got a
random mishmash of // and /* comments looks messy, if you ask me.

An alternative that would be worth considering is to adopt a uniform
rule of // for line-ending comments and /* for all other uses. We'd
have to teach pgindent about that, and I dunno how hard that is.

I think it would be easy to teach pg_indent.

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

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#8Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Tom Lane (#5)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

I find it annoying that "//" comments are not supported, or having to
declare variables at the beginning of a block instead of when first used...

I think some c99 features would be nice (variadic macros for one), but
those particular two get a big "meh" from me.

Yeah. Personally, I'd want to continue the rule against // comments just
as a matter of maintaining stylistic consistency. Code that's got a
random mishmash of // and /* comments looks messy, if you ask me.

Coding styles are by definition a subjective area... Homogeneity
*whatever the precise but reasonable rules* in a code base is a good
thing.

An alternative that would be worth considering is to adopt a uniform
rule of // for line-ending comments and /* for all other uses.

Why not. As far as comments are concerned, editors usually highlight them
in some color, and my eyes get used to the comment color, so the simpler &
shorter the better, really.

We'd have to teach pgindent about that, and I dunno how hard that is.

Maybe it is enough to just to turn "/* no-nl */" to "// no-nl" in
pgindent's post_indent, but probably I'm too naᅵve:-)

--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#9Petr Jelinek
petr@2ndquadrant.com
In reply to: Stephen Frost (#6)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

On 24/10/16 21:11, Stephen Frost wrote:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Fabien COELHO wrote:

I find it annoying that "//" comments are not supported, or having to
declare variables at the beginning of a block instead of when first used...

I think some c99 features would be nice (variadic macros for one), but
those particular two get a big "meh" from me.

Yeah. Personally, I'd want to continue the rule against // comments just
as a matter of maintaining stylistic consistency. Code that's got a
random mishmash of // and /* comments looks messy, if you ask me.

+1

+1

An alternative that would be worth considering is to adopt a uniform
rule of // for line-ending comments and /* for all other uses. We'd
have to teach pgindent about that, and I dunno how hard that is.

I'm not sure it's worth the resulting mess.

I think the current commenting style stylistically nicer.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#10Fabien COELHO
coelho@cri.ensmp.fr
In reply to: Alvaro Herrera (#4)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

I find it annoying that "//" comments are not supported, or having to
declare variables at the beginning of a block instead of when first used...

I think some c99 features would be nice (variadic macros for one), but
those particular two get a big "meh" from me.

This is probably a matter of taste.

The question really to know when some C99 features (inline, mixing code &
declarations, flexible array members, slash-slash comments, compound
literals, variadic macros, restrict qualifier, ...), will be considered
okay for Pg sources, or if they should be kept C89 compliant for the next
27 years.

Well, it seems that Microsoft took time to implement C99 features, which
would mean that a recent "Visual C++" would be required to compile pg on
windows if pg would use C99 features.

.oO( I wonder if it'd be possible to make ereport() an inline function ... )

Probaby not: ISTM that ereport relies on the fact that it is a macro to
avoid evaluating some arguments until necessary, but the compiler would
not do that on a function, whether inlined or not, because it would change
the semantics if the evaluation was to fail.

--
Fabien.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fabien COELHO (#8)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

Fabien COELHO <coelho@cri.ensmp.fr> writes:

An alternative that would be worth considering is to adopt a uniform
rule of // for line-ending comments and /* for all other uses.

Why not. As far as comments are concerned, editors usually highlight them
in some color, and my eyes get used to the comment color, so the simpler &
shorter the better, really.

We'd have to teach pgindent about that, and I dunno how hard that is.

Maybe it is enough to just to turn "/* no-nl */" to "// no-nl" in
pgindent's post_indent, but probably I'm too naïve:-)

Well, IMO the point of making that change would be to buy an additional
three characters of space for the comment before it wraps. So I'd suspect
that post-processing is too late. But I've not looked into pgindent to
see where the decisions are made exactly.

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

#12Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#11)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

On Tue, Oct 25, 2016 at 6:59 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Fabien COELHO <coelho@cri.ensmp.fr> writes:

An alternative that would be worth considering is to adopt a uniform
rule of // for line-ending comments and /* for all other uses.

Why not. As far as comments are concerned, editors usually highlight them
in some color, and my eyes get used to the comment color, so the simpler &
shorter the better, really.

We'd have to teach pgindent about that, and I dunno how hard that is.

Maybe it is enough to just to turn "/* no-nl */" to "// no-nl" in
pgindent's post_indent, but probably I'm too naïve:-)

Well, IMO the point of making that change would be to buy an additional
three characters of space for the comment before it wraps. So I'd suspect
that post-processing is too late. But I've not looked into pgindent to
see where the decisions are made exactly.

Well... Coming back to the subject, are there any recommendations from
committers? -std=c89 in CFLAGS does not seem to help much to detect
extra commas in enums, even if this has been added in C99.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#13Peter Eisentraut
peter_e@gmx.net
In reply to: Michael Paquier (#12)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

On 10/24/16 8:37 PM, Michael Paquier wrote:

Well... Coming back to the subject, are there any recommendations from
committers? -std=c89 in CFLAGS does not seem to help much to detect
extra commas in enums, even if this has been added in C99.

The only option that gives you a warning for this is -pedantic, and
that's not going to work because it disabled a bunch of other stuff.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#14Thomas Munro
thomas.munro@gmail.com
In reply to: Peter Eisentraut (#13)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

On Tue, Oct 25, 2016 at 2:34 PM, Peter Eisentraut
<peter.eisentraut@2ndquadrant.com> wrote:

On 10/24/16 8:37 PM, Michael Paquier wrote:

Well... Coming back to the subject, are there any recommendations from
committers? -std=c89 in CFLAGS does not seem to help much to detect
extra commas in enums, even if this has been added in C99.

The only option that gives you a warning for this is -pedantic, and
that's not going to work because it disabled a bunch of other stuff.

Considering your work to make PostgreSQL a valid C++ program, I just
wanted to note that C++03 doesn't like trailing commas in enums (since
it incorporates the earlier C standard). That means that the baseline
for C++ would need to be at least C++11 for that to compile. There
are also C99 features that are not in any C++ standard including
variable length arrays (which the C++ people consider to be insane
AFAIK) and restrict.

--
Thomas Munro
http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#15Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paquier (#12)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

Michael Paquier <michael.paquier@gmail.com> writes:

Well... Coming back to the subject, are there any recommendations from
committers? -std=c89 in CFLAGS does not seem to help much to detect
extra commas in enums, even if this has been added in C99.

My opinion is don't worry about it. The buildfarm will find such problems
just fine, and there's no functional reason to stress about finding
extra commas before that.

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

#16Michael Paquier
michael@paquier.xyz
In reply to: Tom Lane (#15)
Re: [COMMITTERS] pgsql: Remove extra comma at end of enum list

On Tue, Oct 25, 2016 at 1:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Michael Paquier <michael.paquier@gmail.com> writes:

Well... Coming back to the subject, are there any recommendations from
committers? -std=c89 in CFLAGS does not seem to help much to detect
extra commas in enums, even if this has been added in C99.

My opinion is don't worry about it. The buildfarm will find such problems
just fine, and there's no functional reason to stress about finding
extra commas before that.

Thanks for the feedback. Alea jacta est.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers