Thread: 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
>> 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.
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
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
* 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
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 +
>>> 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.
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
>> 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.
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
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
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
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
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
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