Remove if-else to make branch predictor happy

[curdeius]

libcxx/include/bitset

777–778

When here, please update this part too.

803

I assume that there's no subtle difference between _Traits::eq(__c, __one) and !_Traits::eq(__c, __zero), right?

[prehistoric-penguin]

Updating D100828: Remove if-else to make branch predictor happy

[prehistoric-penguin]

Updating D100828: Remove if-else to make branch predictor happy

[prehistoric-penguin]

libcxx/include/bitset

777–778

I have fixed this part. The CI fail is caused by the clang-format checking, it has nothing to do with my change.

803

Yes, there are only two possible values __one and __zero

[ldionne]

This LGTM but can you please rebase onto main and update the review? That will run the CI again, which failed in the format checks (but those should not be a hard error).

Ship it when CI is green.

[• Quuxplusone]

Also LGTM. To clarify @ldionne's comment: Normally buildkite runs a clang-format check, which is allowed to "soft-fail" (it'll tell you that clang-format disagrees with the patch's style, but it won't break the build, and you'll still end up with a green light from buildkite). In this PR, though, you caught main at a bad time: the clang-format check was "hard-failing" because of a misconfiguration. The misconfiguration was fixed in ff0ada4e1607385be30b2b38885cea2127568e04. So, if you rebase, then you should see buildkite (either soft-fail or pass) the clang-format check as intended, and then it'll run the rest of the tests. If the rest of the tests are green, then ship it!

[prehistoric-penguin]

Thanks for all the detailed reviewed messages. I am a newcomer here. This rebased patch is updated by manually create. If anything wrong please notify me.

[Mordante]

Nice patch, LGTM! Note it seems the build bot acted strange, can you rebase to start a new CI job?

[prehistoric-penguin]

The CI fails is caused by my mistake, for the previous function, we can't call the _Traits::eq since the _Traits doesn't exist in the function signature. Now the CI should pass, but the code change (*this)[__i] = (__c == __one); needs to be reviewed again, I don't know if it violates the code style.

[• Quuxplusone]

libcxx/include/bitset

777–778

Here you could use char_traits<_CharT>::eq(__c, __one) if you like. I don't think it matters which expression you use, in that they all do the same thing in practice and the Standard doesn't seem to tell us which to pick in theory. I have the weakest of preferences for the idea of using == directly and saving a template instantiation, which is exactly what you're doing, so, LGTM.

[prehistoric-penguin]

libcxx/include/bitset

777–778

Thank you for clarify!

[prehistoric-penguin]

The CI is green, I don't have the commit access, could you reviewers please commit it?

[mstorsjo]

In D100828#2710744, @prehistoric-penguin wrote:

The CI is green, I don't have the commit access, could you reviewers please commit it?

Can you provide the git author string you'd want the commit attributed to, i.e. Real Name <email@address>?

[prehistoric-penguin]

In D100828#2711309, @mstorsjo wrote:

In D100828#2710744, @prehistoric-penguin wrote:

The CI is green, I don't have the commit access, could you reviewers please commit it?

Can you provide the git author string you'd want the commit attributed to, i.e. Real Name <email@address>?

The author string: Shu Tian <[e-mail]>

Thank you very much!

libcxx/include/bitset

bitset<_Size>::bitset(const _CharT* __str,
     for (size_t __i = 0; __i < __rlen; ++__i)
         if (__str[__i] != __zero && __str[__i] != __one)
             __throw_invalid_argument("bitset string ctor has invalid argument");
 
     size_t _Mp = _VSTD::min(__rlen, _Size);
     size_t __i = 0;
     for (; __i < _Mp; ++__i)
     {
         _CharT __c = __str[_Mp - 1 - __i];
-        if (__c == __zero)
-            (*this)[__i] = false;
-        else
-            (*this)[__i] = true;
+        (*this)[__i] = (__c == __one);
     }
     _VSTD::fill(base::__make_iter(__i), base::__make_iter(_Size), false);
 }
 
 template <size_t _Size>
 template<class _CharT, class _Traits, class _Allocator>
 bitset<_Size>::bitset(const basic_string<_CharT,_Traits,_Allocator>& __str,
        typename basic_string<_CharT,_Traits,_Allocator>::size_type __pos,
        typename basic_string<_CharT,_Traits,_Allocator>::size_type __n,
        _CharT __zero, _CharT __one)
 {
     if (__pos > __str.size())
         __throw_out_of_range("bitset string pos out of range");
 
     size_t __rlen = _VSTD::min(__n, __str.size() - __pos);
     for (size_t __i = __pos; __i < __pos + __rlen; ++__i)
         if (!_Traits::eq(__str[__i], __zero) && !_Traits::eq(__str[__i], __one))
             __throw_invalid_argument("bitset string ctor has invalid argument");
 
     size_t _Mp = _VSTD::min(__rlen, _Size);
     size_t __i = 0;
     for (; __i < _Mp; ++__i)
     {
         _CharT __c = __str[__pos + _Mp - 1 - __i];
-        if (_Traits::eq(__c, __zero))
-            (*this)[__i] = false;
-        else
-            (*this)[__i] = true;
+        (*this)[__i] = _Traits::eq(__c, __one);
     }
     _VSTD::fill(base::__make_iter(__i), base::__make_iter(_Size), false);
 }
 
 template <size_t _Size>
 inline
 bitset<_Size>&
 bitset<_Size>::operator&=(const bitset& __rhs) _NOEXCEPT