Re: Patch: Remove gcc dependency in definition of inline functions

From: Kurt Harriman <harriman(at)acm(dot)org>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Patch: Remove gcc dependency in definition of inline functions
Date: 2010-02-10 09:26:56
Message-ID: 4B727BE0.8080104@acm.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 1/19/2010 8:01 AM, Peter Eisentraut wrote:
> On tis, 2010-01-19 at 01:29 -0800, Kurt Harriman wrote:
>> On 1/18/2010 11:48 PM, Peter Eisentraut wrote:
>> We have some existing inline functions in .c files. These can be
>> more complicated, so it might be ok if the compiler decides to
>> leave them out-of-line. And they are never unreferenced, so
>> suppression of unused-function warnings is not necessary and
>> perhaps not wanted. To leave these functions undisturbed, my patch
>> doesn't redefine the "inline" keyword; instead it adds a new #define
>> PG_INLINE for use in header files where unused-function warnings
>> need to be suppressed.
>
> One principle that I suppose should have been made more explicit is that
> -- in my mind -- we should avoid littering our code with nonstandard
> constructs in place of standard constructs. Because the next generation
> of developers won't know what PG_INLINE is and why we're not using plain
> inline, even if we document it somewhere.

Unfortunately, to switch to an out-of-line function where inlining is
not supported, a separate preprocessor symbol is needed. The already
existing "inline" define can't be used to test whether the compiler
supports inlining. "inline" is defined as empty if configure doesn't
detect an acceptable variant of "inline". It is left undefined if
the compiler accepts the standard spelling "inline". But the C
preprocessor offers no way to test whether a symbol is defined as
empty. #if can compare integers but not strings.

Everyone seems to hate the name PG_INLINE, so I've changed it to
inline_quietly, which is more descriptive. Anyone who greps for
the definition of inline_quietly will find the comment in pg_config.h
telling what it is and how it should be used, as well as the examples
in pg_list.h and palloc.h. Also it is explained, I hope clearly, in
the proposed CVS commit comment and in this email thread.

> Then just replace in those two locations __GNUC__ by __GNUC__ ||
> __MSVC__ (or whatever the symbol is). Or if you want to make it extra
> nice, create a symbol somewhere like in c.h that reads
>
> #define USE_INLINE __GNUC__ || __MSVC__

That would just add to the compiler-specific preprocessor logic
to be duplicated in every header file in which inline functions
are defined. I'm trying to factor out that compiler dependency
into a central location: pg_config.h.

Regards,
... kurt

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kurt Harriman 2010-02-10 09:43:47 Re: Patch: Remove gcc dependency in definition of inline functions
Previous Message Fujii Masao 2010-02-10 09:19:01 Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL