From: Kurt Harriman on 10 Feb 2010 17:53 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 10 Feb 2010 18:04
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 |