Re: [Bloom] Some questions

From: Joaquin M López Muñoz (joaquinlopezmunoz_at_[hidden])
Date: 2025-05-21 18:41:55

El 21/05/2025 a las 16:42, Alexander Grund via Boost escribió:
> Am 21.05.25 um 14:28 schrieb Joaquin M López Muñoz via Boost:
>> El 21/05/2025 a las 13:54, Ivan Matek via Boost escribió:
>>> Warning is not false positive.
>>>
>>>
>>>> The warning does not indicate a problem in
>>>> the code, and whether it is more or less readable is very 
>>>> subjective. It
>>>> seems like a pretty pointless warning to me. Even if you care about 
>>>> such
>>>> things, you really shouldn't be applying it to any code other than 
>>>> your
>>>> own.
>>>>
>>> Well as I said it is not a problem since inline is just redundant.
>>
>> It is redundant from the point of view of the language standard, but
>> compilers take inline as a hint to favor inlining in optimization modes.
>> Consider for instance:
>>
>> https://godbolt.org/z/dnfhYvnMK
>>
>> You can see that X::foo is inlined while X::bar is not. This is the 
>> original
>> reason why those member functions in candidate Boost.Bloom are added
>> the "inline" bit, namely, to invite compilers to inline --though, 
>> admittedly,
>> for very short functions this is hardly going to make a difference.
>
> I think the warning is mostly correct. From the Docs:
> > |inline| is redundant since constexpr functions are implicitly inlined
> > |inline| is redundant since member functions defined entirely inside 
> a class/struct/union definition are implicitly inlined.
>
> In your godbolt example you have *static*Â  member functions.
> This seems to be a difference. See https://godbolt.org/z/h76z4GfzP for 
> your example with static removed

 From the point of view of the standard, there's no difference between
static and non-static member functions when it comes to how inline
applies to them --that is, it is redundant if the function definition
is in-class.

 From a optimization hint point ot view, static/non-static it may or may 
not make
a difference, but at any rate compilers seem to be still paying 
attention to the
inline keyword as of this writing.

> I'm wondering if this is a compiler/optimizer issue that should be fixed.
>
> So I'd say at least "inline constexpr" can be simplified, which is 
> used in the source.
>

It's used once, if I'm not wrong, here:

https://github.com/joaquintides/bloom/blob/develop/include/boost/bloom/detail/constexpr_bit_width.hpp

Yes, removing inline here should be ok as the function is always used at
compile time, so inlining is immaterial.

>
> However you are right in that clang-tidy also warns for "static 
> inline" where it does matter: https://godbolt.org/z/WqK3oK93Y
> It seems to indeed take it as a hint as the function will be inlined 
> when you make it smaller.
>

Joaquin M Lopez Munoz