Stefan Fuhrmann wrote on Wed, May 28, 2014 at 12:30:36 +0200:
> On Wed, May 28, 2014 at 3:41 AM, Daniel Shahaf <d.s_at_daniel.shahaf.name>wrote:
>
> > Building latest trunk with gcc 4.7:
> >
> > subversion/libsvn_fs_fs/index.c:351:1: warning: always_inline function
> > might not be inlinable [-Wattributes]
> > subversion/libsvn_fs_x/index.c:356:1: warning: always_inline function
> > might not be inlinable [-Wattributes]
> > subversion/libsvn_fs_x/index.c:339:1: warning: always_inline function
> > might not be inlinable [-Wattributes]
> > subversion/libsvn_ra_svn/marshal.c:398:1: warning: always_inline function
> > might not be inlinable [-Wattributes]
> >
> > If the inlining is really required, we should fix the code such that it
> > either inlines the function or fails to compile, rather than this
> > halfway mode.
> >
>
> None of these inlines is required for functional correctness.
> It is just that the compiler made a poor choice of those hot
> code paths.
>
Could you please clarify this in the docstrings, as well?
Right now they say:
* The forced inline is required as e.g. GCC would inline read() into here
* instead of lining the simple buffer access into callers of get().
which could be mistaken as implying "required for functional
correctness".
I'd do this myself but you'd probably be able to describe what they
_are_ required for better.
> > If inline is not required, we should demote the always_inline to a plain
> > APR_INLINE, which should sidestep the warning.
> >
>
> As it turned out, always_inline does not imply inline. So,
> the right fix here is to always add APR_INLINE (except for
> Visual C where __forceinline implies inline).
>
> r1597962 should do the trick.
>
Thanks!
> -- Stefan^2.
Received on 2014-05-28 16:16:24 CEST