From: Kurt Harriman on
Revised patch is attached (4th edition).
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

With this patch, the "configure" script tests whether a static
inline function can be defined without incurring a warning when
not referenced. If successful, the preprocessor symbol USE_INLINE
is defined to 1 in pg_config.h. Otherwise USE_INLINE remains
undefined.

palloc.h and pg_list.h condition their inline function
definitions on USE_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.

From now on in cross-platform code, plain "inline" should be used
instead of compiler-specific alternate spellings such as __inline__,
because the configure script redefines "inline" to the appropriate
alternate spelling if necessary. (This patch doesn't affect the
redefinition of "inline": that was already in place.)

Changes since the second and third editions of this patch:

- Renamed the new preprocessor symbol to USE_INLINE, following the
advice of Tom Lane, instead of my earlier attempts PG_INLINE or
inline_quietly. It is used like this:

/* include/utils/palloc.h */
#ifdef USE_INLINE
static inline MemoryContext
MemoryContextSwitchTo(MemoryContext context)
{
MemoryContext old = CurrentMemoryContext;
CurrentMemoryContext = context;
return old;
}
#else
extern MemoryContext MemoryContextSwitchTo(MemoryContext context);
#endif /* USE_INLINE */

/* backend/utils/mmgr/mcxt.c */
#ifndef USE_INLINE
MemoryContext
MemoryContextSwitchTo(MemoryContext context)
{
MemoryContext old;
AssertArg(MemoryContextIsValid(context));
old = CurrentMemoryContext;
CurrentMemoryContext = context;
return old;
}
#endif /* ! USE_INLINE */

- Simplified the autoconf logic added to config/c-compiler.m4.

- Removed MSVC-related changes (__forceinline) from the autoconf
stuff. Instead, updated the manually-edited pg_config.h.win32
file to define "inline" as __inline, and USE_INLINE as 1.

- Removed Windows-only misspelling of __inline__ in instr_time.h.
This was the only occurrence of __inline__; therefore, deleted
the no-longer-needed definition of __inline__ from port/win32.h.
Also deleted the definition of inline from port/win32.h, since
it is now defined in pg_config.h.win32, consistent with the
other platforms.

Thanks again to all who have commented.

Regards,
.... kurt

From: Kurt Harriman on
On 2/10/2010 7:12 AM, Tom Lane wrote:
> Kurt, you seem to be more or less impervious to advice :-(.

Thank you for reviewing my patch. It is a rare honor to
have my personal qualities reviewed here as well.

Since this forum is archived for posterity, I suppose I
must point out that I have in fact been responsive to all
the advice that has been offered here. I have answered
each comment fully and politely. I have acted upon most
of the suggestions, and have revised my small patch
accordingly and resubmitted it twice (now thrice,
incorporating this comment of yours).

Admittedly I have used my own judgment in how to adopt these
sometimes conflicting suggestions; and have explained the
rationale for my choices. Everyone may judge whether my
choices and explanations are satisfactory and continue the
dialogue if they are not yet satisfied. By sincerely
engaging in such a process, consensus may be reached.

All contributors to the pg_hackers list are well advised to
be impervious to brusque and sometimes rude dismissals, which
seem to be de rigueur here. However, the evidence of this
thread shows that I have been far from impervious to advice.

By the way, suggestions which must be carried out without
question are "orders", not "advice". When a statement,
meant to be imperative, is phrased softly as advice, it can
easily be mistaken as optional by newcomers who may not have
fully grasped the prevailing reality. Thus, commands are
best stated in clear language.

> Please just make the thing be "inline" and have configure define
> USE_INLINE, as per previous discussion.

Just now I have resubmitted according to your instruction.

> Cluttering the code with nonstandard constructs is not
> good for readability.

Agreed. But any program consists of definitions of new
identifiers, data structures, macros, and conventions or
guidelines for their use. What, or who, differentiates
ordinary programming practice, such as typedefs, from
"nonstandard constructs"?

> More, it is likely to confuse syntax-aware editors and
> pgindent, neither of which will read any of the discussion
> or commit messages.

Good point. This had not been mentioned before. It works
alright with the syntax-aware editors that I use, and I
haven't had occasion to run pgindent, so I didn't think of
this earlier.

Does the same problem exist with the PGDLLIMPORT macro
defined in postgres.h? It, too, is used in the same
syntactic niche where "inline" would be placed.

Regards,
.... kurt

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers