pgsql: Add macros wrapping all usage of gcc's __attribute__.
Add macros wrapping all usage of gcc's __attribute__.
Until now __attribute__() was defined to be empty for all compilers but
gcc. That's problematic because it prevents using it in other compilers;
which is necessary e.g. for atomics portability. It's also just
generally dubious to do so in a header as widely included as c.h.
Instead add pg_attribute_format_arg, pg_attribute_printf,
pg_attribute_noreturn macros which are implemented in the compilers that
understand them. Also add pg_attribute_noreturn and pg_attribute_packed,
but don't provide fallbacks, since they can affect functionality.
This means that external code that, possibly unwittingly, relied on
__attribute__ defined to be empty on !gcc compilers may now run into
warnings or errors on those compilers. But there shouldn't be many
occurances of that and it's hard to work around...
Discussion: 54B58BA3.8040302@ohmu.fi
Author: Oskari Saarenmaa, with some minor changes by me.
Branch
------
master
Details
-------
http://git.postgresql.org/pg/commitdiff/bbfd7edae5aa5ad5553d3c7e102f2e450d4380d4
Modified Files
--------------
contrib/cube/cubescan.l | 2 +-
contrib/pg_upgrade/pg_upgrade.h | 14 ++++----
contrib/pg_upgrade/util.c | 2 +-
contrib/pg_xlogdump/pg_xlogdump.c | 2 +-
contrib/pgcrypto/px.h | 2 +-
contrib/seg/segscan.l | 2 +-
src/backend/access/transam/xlogreader.c | 2 +-
src/backend/postmaster/autovacuum.c | 4 +--
src/backend/postmaster/pgarch.c | 2 +-
src/backend/postmaster/pgstat.c | 2 +-
src/backend/postmaster/postmaster.c | 4 +--
src/backend/postmaster/syslogger.c | 2 +-
src/backend/replication/repl_scanner.l | 2 +-
src/backend/replication/walsender.c | 2 +-
src/backend/utils/error/elog.c | 2 +-
src/backend/utils/misc/guc.c | 2 +-
src/bin/pg_ctl/pg_ctl.c | 2 +-
src/bin/pg_dump/parallel.c | 2 +-
src/bin/pg_dump/pg_backup.h | 2 +-
src/bin/pg_dump/pg_backup_archiver.h | 6 ++--
src/bin/pg_dump/pg_backup_tar.c | 2 +-
src/bin/pg_dump/pg_backup_utils.h | 8 ++---
src/bin/psql/common.h | 2 +-
src/bin/psql/large_obj.c | 2 +-
src/include/bootstrap/bootstrap.h | 4 +--
src/include/c.h | 49 +++++++++++++++++++++++----
src/include/common/fe_memutils.h | 4 +--
src/include/lib/stringinfo.h | 4 +--
src/include/mb/pg_wchar.h | 4 +--
src/include/parser/parse_relation.h | 4 +--
src/include/parser/scanner.h | 2 +-
src/include/pgstat.h | 2 +-
src/include/port.h | 14 ++++----
src/include/port/atomics/generic-gcc.h | 2 +-
src/include/port/atomics/generic-sunpro.h | 2 +-
src/include/port/atomics/generic-xlc.h | 2 +-
src/include/postgres.h | 2 +-
src/include/postmaster/autovacuum.h | 4 +--
src/include/postmaster/bgworker_internals.h | 2 +-
src/include/postmaster/bgwriter.h | 4 +--
src/include/postmaster/pgarch.h | 2 +-
src/include/postmaster/postmaster.h | 4 +--
src/include/postmaster/startup.h | 2 +-
src/include/postmaster/syslogger.h | 2 +-
src/include/postmaster/walwriter.h | 2 +-
src/include/replication/walreceiver.h | 2 +-
src/include/storage/ipc.h | 2 +-
src/include/storage/itemptr.h | 2 +-
src/include/storage/lock.h | 2 +-
src/include/tcop/tcopprot.h | 6 ++--
src/include/utils/datetime.h | 2 +-
src/include/utils/elog.h | 38 ++++++++++-----------
src/include/utils/help_config.h | 2 +-
src/include/utils/palloc.h | 4 +--
src/interfaces/ecpg/ecpglib/extern.h | 2 +-
src/interfaces/ecpg/include/ecpglib.h | 2 +-
src/interfaces/ecpg/preproc/ecpg.header | 2 +-
src/interfaces/ecpg/preproc/extern.h | 4 +--
src/interfaces/libpq/libpq-int.h | 6 ++--
src/interfaces/libpq/pqexpbuffer.c | 2 +-
src/interfaces/libpq/pqexpbuffer.h | 4 +--
src/interfaces/libpq/win32.c | 2 +-
src/pl/plperl/plperl.h | 4 +--
src/pl/plpgsql/src/pl_scanner.c | 2 +-
src/pl/plpython/plpy_elog.h | 8 ++---
src/test/modules/test_shm_mq/test_shm_mq.h | 2 +-
src/test/modules/worker_spi/worker_spi.c | 2 +-
src/test/regress/pg_regress.c | 6 ++--
68 files changed, 167 insertions(+), 132 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Andres Freund <andres@anarazel.de> 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.
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?
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
On 2015-03-25 19:11:06 -0400, Tom Lane wrote:
Andres Freund <andres@anarazel.de> 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
Andres Freund <andres@anarazel.de> 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:
void pg_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
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