Re: [HACKERS] Re: [COMMITTERS] pgsql: Add macros wrapping all usage of gcc's __attribute__.
On 2015-03-26 11:27:32 -0400, Tom Lane wrote: > Being the one complaining, I'll go do the legwork to clean this up. Looks good, Thanks! Greetings, Andres Freund -- Andres Freund 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
Re: [HACKERS] Re: [COMMITTERS] pgsql: Add macros wrapping all usage of gcc's __attribute__.
Andres Freund writes: > On 2015-03-25 19:11:06 -0400, Tom Lane wrote: >> I think this is a bad idea, because it's going to look like heck after >> pgindent gets through with it. Do we actually need decoration on the >> function definitions? > Hm, I guess it should not look any worse than before? It does, because pgindent treats "pg_attribute_noreturn" differently than it treated "__attribute__((noreturn))". Before you'd end up with something like void __attribute__((noreturn)) plpgsql_yyerror(const char *message) { pgindent forced the __attribute__(()) bit onto its own line, whether you wrote it that way or not, but it doesn't look *too* awful. But now that becomes: void pg_attribute_noreturn plpgsql_yyerror(const char *message) { The best you can get is to manually put the noreturn back onto the "void" line, but you still end up with: voidpg_attribute_noreturn plpgsql_yyerror(const char *message) { So this is just ugly. Maybe we could teach pgindent not to do that, but I'm doubtful. > ... That said, I see little reason > to add the noreturn thingy to the definition and not the declaration for > those. It actually looks to me like there's a declaration for > replication_yyerror, but a plain yyerror is used instead in repl_scanner.l? Right. Also, even in the context of extern declarations, it seems to be a lot easier to get pgindent not to mess with your layout if "pg_attribute_noreturn" is replaced with "pg_attribute_noreturn()". I see no particular reason not to add parens to the macro, do you? Being the one complaining, I'll go do the legwork to clean this up. 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
[HACKERS] Re: [COMMITTERS] pgsql: Add macros wrapping all usage of gcc's __attribute__.
On 2015-03-25 19:11:06 -0400, Tom Lane wrote: > Andres Freund writes: > > Add macros wrapping all usage of gcc's __attribute__. > > I noticed that this commit attached pg_attribute_noreturn not only > to the extern declarations, but to some actual function definitions. Unless either Oskari or I screwed up, it should just have been a 1:1 translation from previous __attribute__((noreturn)) to pg_attribute_noreturn. I looked through the commit just now and didn't see any new locations. > I think this is a bad idea, because it's going to look like heck after > pgindent gets through with it. Do we actually need decoration on the > function definitions? Hm, I guess it should not look any worse than before? None of the locations look like they've been introduced after the last pgindent run. I only see plpgsql_yyerror, yyerror. That said, I see little reason to add the noreturn thingy to the definition and not the declaration for those. It actually looks to me like there's a declaration for replication_yyerror, but a plain yyerror is used instead in repl_scanner.l? Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers