Re: [Bloom] Some questions

From: Ivan Matek (libbooze_at_[hidden])
Date: 2025-05-19 19:01:48

On Mon, May 19, 2025 at 8:27 PM Joaquin M López Muñoz via Boost <
boost_at_[hidden]> wrote:

> > It is IDE clang-tidy warning, not sure compiler matters.
> >
> https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-inline-specifier.html
> >
>
>
> Could you please check if adding this comment to the offending lines
>
> /* NOLINT(readability-redundant-inline-specifier) */
>
> makes the warnings go away? If so, I'd happily accept a PR with
> that change. Thank you!
>

I am now a bit confused :)
I believe warning is correct, why not change the code instead of
suppressing the warning?
To be clear this is warning for literal inline keyword not boost force
inline macro, few examples:

inline constexpr std::size_t range()const noexcept{return (std::size_t)rng;}

inline void prepare_hash(boost::uint64_t& hash)const noexcept

// ignore gray for size_t cast, it does not know sometimes we compile on
32bit system.

But to answer your question: suppression with NOLINT works.

On Mon, May 19, 2025 at 8:44 PM Joaquin M López Muñoz via Boost <
boost_at_[hidden]> wrote:

> El 18/05/2025 a las 23:38, Ivan Matek escribió:
> > Had a bit more time to think :) so here are my replies and few more
> > questions.
> >
> >
> >     >     5. Why is BOOST_ASSERT(fpr>=0.0&&fpr<=1.0); not
> >     > BOOST_ASSERT(fpr>0.0&&fpr<=1.0);
> >     >     , i.e. is there benefit of allowing calls with impossible
> >     fpr argument?
> >     fpr==0.0 is a legitimate (if uninteresting) argument value for
> >     capacity_for:
> >
> >     capacity_for(0, 0.0) --> 24
> >     capacity_for(1, 0.0) --> 18446744073709549592
> >
> >     The formal reason why fpr==0.0 is supported is because of symmetry:
> >     some calls to fpr_for actually return 0.0 (for instance,
> >     fpr_for(0, 100)).
> >
> >
> > This is a bit philosophical, but I actually do not feel this is correct.
> > First of all  is (0, 0.0) only usecase where fpr of 0.0 makes sense?
> > i.e any time when n>0 fpr 0.0 is impossible(or I misunderstood
> > something).
>
> Yes, it is impossible: the capacity would have to be infinite --the maximum
> attainable value is returned instead, though this is of little value as
> OOM would ensue (as you point out below).
>
> > So assert could be implies(it is funny because we had discussion about
> > implies on ML few months ago), so something like:
> > BOOST_IMPLICATION(fpr == 0.0, n == 0);
> >
> > Similarly for (1, 0.0) I do not believe result should be size_t max
> > value, as this is not correct value. Now we both know you will OOM
> > before noticing this in reality,
> > but even if we imagine magical computer that can allocate that much
> > memory fpr is not 0.0.
>
>
> I understand your point and can relate to it, but consider this:
>
> capacity_for(1, 1.E-200)
>
> Is this legit? OOM will happen here, too. Where do we put the
> limit?

I have actually considered that also(I actually thought of std::nextafter(0.0,
1.0);) but same idea...
This is getting bit philosophical, but I see those as 2 different, although
similar issues.

   1. impossible to compute result or result over size_t max
   2. practically impossible: e.g. result is 2^47: fits inside size_t but
   will OOM

I am not a big fan of library picking a constant for which result is
unreasonable( to handle 2.), but on the other hand nonsense values should
be detected asap, before program has chance to continue... long story short
I am not sure what is best decision here.
For 1. I am much more convinced that returning nullopt is morally ;)
correct API design.