Re: [Bloom] review starts on 13th of May

From: Alexander Grund (alexander.grund_at_[hidden])
Date: 2025-05-23 08:18:51

Am 23.05.25 um 03:13 schrieb David Bien via Boost:
> I noticed the specialized emplace() right away - it looked weird so I thought about it for a bit:
>
>    template<typename... Args>
>    BOOST_FORCEINLINE void emplace(Args&&... args)
>    {
>      insert(detail::allocator_constructed<allocator_type,value_type>{
>        get_allocator(),std::forward<Args>(args)...}.value());
>    }
>    template<
>      typename U,
>      typename std::enable_if<
>        std::is_same<T,detail::remove_cvref_t<U>>::value>::type* =nullptr
>    >
>    BOOST_FORCEINLINE void emplace(U&& x)
>    {
>      insert(x); /* avoid value_type construction */
>    }
>
> Given
> T t;
> So, the emplace( std::move(t)) will call emplace(U&&), which doesn’t move anything out of the value x, cuz insert(const&).
Which is fine, isn't it? No functions is required to "destroy" the 
rvalue-arg.
> emplace(t) will call the emplace(Args&&… args) because emplace(U&&) can’t accept a const T& - and will make a copy of t before calling insert().
I don't think so. Isn't the 2nd overload a universal reference? I.e. it 
trivially does accept a const T& and in fact I think this is the whole 
point of it as indicated by the concept-like.
So it accepts any form of a T so no copy should be made.
Although I guess a std::forward would make the pattern more clear.

> Why even have the element type? `insert` can in theory
> accept any object which is hashable. And emplace doesn't seem as valuable
> for the bloom filter container as it is for regular containers, since there
> are no nodes. That is, rather than `emplace(args...)` a user could just as
> easily write `insert(T(args...))`
Having the element type is useful to distinguish a filter for ints vs 
strings or some custom type. I wouldn't want to loose that.
As for emplace I see the value in using the containers allocator. Maybe 
this should be highlighted in the documentation as the main reason over 
`insert(T(...))`.
Also the interface of implicitly constructing the T avoids the noise in 
the call.