[Boost.Config] Should BOOST_FORCEINLINE depend on NDEBUG?

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[Boost.Config] Should BOOST_FORCEINLINE depend on NDEBUG?

Boost - Dev mailing list
Right now BOOST_FORCEINLINE does not depend on NDEBUG.

This means that on Debug builds the code asks to be forced-inlined with
optimizations disabled, which does not make much sense to me. This is
reflected by a myriad warnings in msvc under level 4 warnings.
In fact Multiprecision library works around this issue by defining their
own macro BOOST_MP_FORCEINLINE which decays to just inline in Debug builds
(boost/multiprecision/detail/number_base.hpp lines 25-29).

I think this behavior should be adopted by BOOST_FORCEINLINE. I'm happy to
do a PR if there is agreement from maintainers.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost.Config] Should BOOST_FORCEINLINE depend on NDEBUG?

Boost - Dev mailing list
On 2020-01-29 12:11, dariomt--- via Boost wrote:

> Right now BOOST_FORCEINLINE does not depend on NDEBUG.
>
> This means that on Debug builds the code asks to be forced-inlined with
> optimizations disabled, which does not make much sense to me. This is
> reflected by a myriad warnings in msvc under level 4 warnings.
> In fact Multiprecision library works around this issue by defining their
> own macro BOOST_MP_FORCEINLINE which decays to just inline in Debug builds
> (boost/multiprecision/detail/number_base.hpp lines 25-29).
>
> I think this behavior should be adopted by BOOST_FORCEINLINE. I'm happy to
> do a PR if there is agreement from maintainers.

Please, no. There is code (e.g. in Boost.Atomic) which requires inlining
regardless of the release/debug mode.

If a compiler emits warnings about it then the solution is to disable
these warnings.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: [Boost.Config] Should BOOST_FORCEINLINE depend on NDEBUG?

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Wed, 29 Jan 2020 at 10:11, dariomt--- via Boost
<[hidden email]> wrote:
>
> Right now BOOST_FORCEINLINE does not depend on NDEBUG.

Given the current definition of the maro (copied below
https://github.com/boostorg/config/blob/develop/include/boost/config/detail/suffix.hpp#L598-L609

// BOOST_FORCEINLINE ---------------------------------------------//
// Macro to use in place of 'inline' to force a function to be inline
#if !defined(BOOST_FORCEINLINE)
#  if defined(_MSC_VER)
#    define BOOST_FORCEINLINE __forceinline
#  elif defined(__GNUC__) && __GNUC__ > 3
     // Clang also defines __GNUC__ (as 4)
#    define BOOST_FORCEINLINE inline __attribute__ ((__always_inline__))
#  else
#    define BOOST_FORCEINLINE inline
#  endif
#endif

I don't think it should as it somewhat does, implicitly, as MSVC is supposed to
ignore __forceinline in Debug builds.

> This means that on Debug builds the code asks to be forced-inlined with
> optimizations disabled, which does not make much sense to me. This is
> reflected by a myriad warnings in msvc under level 4 warnings.

https://docs.microsoft.com/en-us/cpp/cpp/inline-functions-cpp says:

"""
Even with __forceinline, the compiler cannot inline code in all
circumstances. The compiler cannot inline a function if:
    The function or its caller is compiled with /Ob0 (the default
option for debug builds).
"""

and then

"""
If the compiler cannot inline a function declared with __forceinline,
it generates a level 1 warning, except when:
    The function is compiled by using /Od or /Ob0. No inlining is
expected in these cases.
"""

So, the level 4 warning you are observing, which I guess is C4714,
is just compiler's verbose reminder of the documented circumstances.
The MSVC level 4 warnings are just *informational* warnings.
(I'd risk opinion lots/most of them are not warnings but
notices/infos, in DBMS speak :-))

IMO, Andrey's suggestion is the sensible one indeed.

Best regards,
--
Mateusz Loskot, http://mateusz.loskot.net

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost