On 9/23/22 11:25, Alexander Grund via Boost wrote:
Am 23.09.22 um 09:18 schrieb Andrey Semashev via Boost:
Maybe we could use __pragma in BOOST_FORCEINLINE on a recent enough MSVC?
https://godbolt.org/z/oh5szGd4r This is an awesome idea and I'm surprised it actually works.
I wanted to propose a PR to Boost.Config which adds your code to the test suite making sure it compiles warning-free.
However it fails on GCC:
error: inlining failed in call to ‘always_inline’ ‘unsigned int f_not_inlined(unsigned int)’: function not considered for inlining 19 | BOOST_FORCEINLINE unsigned f_not_inlined( unsigned x ) | ^~~~~~~~~~~~~ note: called from here 21 | return x == 0 ? 0: x * f_not_inlined( x-1 );
So I'm actually not sure how we want to define the behavior of BOOST_FORCEINLINE especially as it is a macro officially available for users too.
Given the name a warning is appropriate especially given that other compilers error in such cases. For the GCC behavior I found https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63220 which cites
always_inline is _not_ an optimization hint! always_inline was meant to mark functions that won't work correctly if not inlined.
So I'd say we need a `BOOST_INLINE` macro expanding to a (strong) inlining hint and does not generate errors/warnings. And Boost libraries should most likely use that, not the FORCEINLINE macro.
There's no middle ground behavior between inline keyword and BOOST_FORCEINLINE, so I don't see the reason to have yet another BOOST_INLINE macro. You want opportunistic inlining - use plain inline. If __pragma/_Pragma approach doesn't work, I'm inclined to shift the burden to library maintainers (those who are willing to deal with this) and users. To users: you've brought this on yourselves by using -Werror and -Wextra (or whatever equivalents your compiler has), so you pay the price by having to deal with this. Using max warnings, especially if promoted to errors, is useless and a bad idea. That said, I'm still in favor of an MSVC-specific fix by adding __pragmas to BOOST_FORCEINLINE. That pesky warning has come up a few times in the libraries I maintain and I never found it useful.