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

On 1/18/2010 11:48 PM, Peter Eisentraut wrote:
> On mån, 2010-01-18 at 16:34 -0800, Kurt Harriman wrote:
>> MSVC does warn about unused static inline functions.  The warning
>> is prevented by using __forceinline instead of __inline.
>
> Hmm, but forceinline is not the same as inline.  Are we confident that
> forcing inlining is not going to lead to disadvantages?  Has this been
> measured?

My patch uses __forceinline only in header files where it is
needed to avoid warnings.

Inline functions in header files are typically trivial: list_head(),
MemoryContextSwitchTo(); so I expect they'll always be inlined
no matter whether we use __inline or __forceinline.   These are
frequently called on heavily trafficked paths, so I think we
want them to be inlined always.

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.

> Is there not a setting to disable this particular warning.  I read that
> MSVC has various ways to set that sort of thing.

Yes, warnings can be turned off by a #pragma specifying the
warning number.
  http://msdn.microsoft.com/en-us/library/2c8f766e%28VS.71%29.aspx
http://msdn.microsoft.com/en-us/library/cw9x3tcf%28VS.100%29.aspx
http://msdn.microsoft.com/en-us/library/thxezb7y%28VS.71%29.aspx

To turn off unused-function warnings for just certain inline
functions, one could use
  #ifdef _MSC_VER  #pragma warning(push)  #pragma warning(disable:4514)  #endif     ... inline function definitions ...
#ifdef _MSC_VER  #pragma warning(pop)  #endif
 

Or compiler switches could be set to disable all such warnings
globally.  Warning 4514 is specific to inline functions; so
maybe it would be alright to keep it turned off globally.

Note, this warning is disabled by default, but that is typically
overridden by the /Wall option which changes all off-by-default
warnings to become on-by-default.  Probably /Wall could be
combined with /Wd4514 to keep unused-inline-function warnings
turned off while enabling the rest.  If that is acceptable,
we wouldn't need #ifdefs, #pragmas, or __forceinline.

Regards,
... kurt