On Fri, 30 Aug 2024 at 11:52, Giuseppe D'Angelo wrote:
>
> Hello,
>
> This patch completes the fix for PR108846, extending it to range-based
> copy/move algorithms, and also fixes a faulty static_assert in them
> (PR116471).
> It's a minor improvement over the patch I've attached to PR116471 (I've
> fixed the constraints of __assign_one).

Nice, thanks for the patch.

Is there any need to constrain __assign_one? It's only going to be
called from internal functions where we know the constraint is already
satisfied. Constraining __assign_one just gives the compiler more work
to do to check satisfaction that all callers have already checked.

The calls to __assign_one should all be qualified to prevent ADL.

For C++20 code we can use [[likely]] instead of __builtin_expect.

And a very minor comment on the ChangeLog part, the new function
should be named in parentheses, just like the existing functions being
changed, i.e.

           * include/bits/ranges_algobase.h (__assign_one): New helper
           function.

But I can make these minor changes locally and push it if you want -
there's no need for a v2 patch.

Reply via email to