On 12/15/09, Kurt Harriman <harriman@acm.org> wrote:
> Attached is a revised patch, offered for the 2010-01 commitfest.
> It's also available in my git repository in the "submitted" branch:
>
> http://git.postgresql.org/gitweb?p=users/harriman/share.git;a=shortlog;h=refs/heads/submitted
>
> In this version, the "configure" script tests whether a static
> inline function can be defined without incurring a warning when
> not referenced. If successful, the preprocessor symbol PG_INLINE
> is defined in pg_config.h to the appropriate keyword: inline,
> __inline, __inline__, or __forceinline. Otherwise PG_INLINE
> remains undefined.
>
> palloc.h and pg_list.h condition their inline function
> definitions on PG_INLINE instead of the gcc-specific __GNUC__.
> Thus the functions can be inlined on more platforms, not only
> gcc.
>
> Ordinary out-of-line calls are still used if the compiler doesn't
> recognize inline functions, or spews warnings when static inline
> functions are defined but not referenced.
-1. The PG_INLINE is ugly.
In actual C code we should see only "inline" keyword, no XX_INLINE,
__inline, __inline__, etc. It's up to configure to make sure "inline"
is something that C compiler accepts or simply defined to empty string
otherwise.
I assume you are playing with force-inline to avoid warnings on some
compiler? Do you a actual compiler that behaves like that? Unless
it is some popular compiler (as "in actual use") it is premature
complexity. Please remove that.
We may want to have force-inline in the future, when we start converting
some more complex macros to inline functions, but then it should cover
all main compilers, and current patch does not try to do it.
So my suggestions:
1. Make sure "inline" is defined, empty or to something that works. (plain AC_C_INLINE seems to do that)
2. Convert current __inline, __inline__ sites to "inline"
and
3. Remove #ifdefs and duplicate macros, keeping only inline funcs.
There does not seem to be any holding counter-arguments why we should
not do that, so lets go ahead?
--
marko
