On Thu, Nov 19, 2020 at 07:16:52PM +0000, Jonathan Wakely wrote:
> On 19/11/20 12:57 -0500, Lewis Hyatt via Libstdc++ wrote:
> > Hello-
> > 
> > PR61369 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61369) points out
> > that std::discrete_distribution can return an event even if it has 0
> > probability, and proposes a simple fix. It seems that this fix was never
> > applied, because there was an expectation of redoing this code anyway to
> > use a more efficient algorithm (PR57925). Given that this new algorithm
> > has not been implemented so far, would it make sense to apply the simple
> > fix to address this issue? The attached patch does this.
> > 
> > One question about the patch, a slight annoyance is that only
> > std::lower_bound() is currently available in random.tcc, as this file
> > includes only bits/stl_algobase.h and not bits/stl_algo.h (via including
> > <vector>). Is there a preference between simply including stl_algo.h, or
> > moving upper_bound to stl_algobase.h, where lower_bound is? I noticed
> > that in C++20 mode, <vector> includes stl_algo.h already, so I figured
> > it would be fine to just include it in random.tcc unconditionally.
> 
> But the increase in header sizes in C++20 is a regression:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92546
> 
> Anyway, I'll review this patch tomorrow, thanks for sending it.
> 
> > bootstrap + testing were done on x86-64 GNU/Linux, all tests the same
> > before + after plus 2 new passes from the new test. Thanks for taking a
> > look!
> > 
> > -Lewis
>

Sorry for the noise on this... I realized the patch I sent before missed
one line. I guess there is an internal __generate() function that
presumably needs the same change as operator()(). This version modifies
that one as well and adds a test case. Would you mind please reviewing this
one instead? Thanks!

-Lewis