8

Consider this unit test:

std::bitset<8> temp( "11010100" );
reverseBitSet( temp );
CPPUNIT_ASSERT( temp == std::bitset<8>( "00101011" ) );

This implementation works:

template<size_t _Count> static inline void reverseBitSet( std::bitset<_Count>& bitset )
{
    bool val;
    for ( size_t pos = 0; pos < _Count/2; ++pos )
    {
        val = bitset[pos];
        bitset[pos] = bitset[_Count-pos-1];
        bitset[_Count-pos-1] = val;
    }
}

While this one does not:

template<size_t _Count> static inline void reverseBitSet( std::bitset<_Count>& bitset )
{
    for ( size_t pos = 0; pos < _Count/2; ++pos )
    {
        std::swap( bitset[pos], bitset[_Count-pos-1] );
    }
}

Result is "11011011" instead of "00101011"

Why is swap doing it wrong?

Barry
  • 286,269
  • 29
  • 621
  • 977
jpo38
  • 20,821
  • 10
  • 70
  • 151

1 Answers1

7

This:

std::swap( bitset[pos], bitset[_Count-pos-1] );

should actual fail to compile. operator[] for a std::bitset doesn't return a reference, it returns a proxy object. That proxy object isn't an lvalue, so it cannot bind to the T& in std::swap. I'm assuming the fact that it compiles at all means that you're using MSVC which has an extension that allows binding temporaries to non-const references - at which point you're probably just swapping the proxies and not what the proxies are actually referring to.


Side-note: The name _Count is reserved by the standard, as is any other name which begins with an _ followed by a capital letter.

Barry
  • 286,269
  • 29
  • 621
  • 977
  • But why is the doc you linked saying there's a version returning a reference (`(2) reference operator[]( std::size_t pos );`) ? – jpo38 Sep 06 '16 at 13:48
  • 1
    @jpo38 It doesn't say it returns a reference. It says it returns an object of type [`std::bitset::reference`](http://en.cppreference.com/w/cpp/utility/bitset/reference) (which is, admittedly, confusing). – Barry Sep 06 '16 at 13:51
  • 1
    @jpo38 That _reference_ is a `std::bitset::reference`, that is the proxy object mentioned in the answer. – skypjack Sep 06 '16 at 13:54
  • Got it, was confused by the name. You are right, I'm using MSVC for development. – jpo38 Sep 06 '16 at 13:55
  • 2
    Mentioning the flag needed to turn off this bug-inducing feature might be a good idea. (`/Za`) – Yakk - Adam Nevraumont Sep 06 '16 at 15:19
  • 1
    Actually, no, `vector` is even weirder - it has a *static member function* `swap` that works on `reference`. Who came up with this mess? – T.C. Sep 06 '16 at 20:29
  • @T.C. Wait, how does std::swap work then if it's a static function? – Barry Sep 06 '16 at 20:39
  • It doesn't. I misread it as a friend non-member when I wrote the first comment. (libc++ has an extension that handles this correctly, though.) – T.C. Sep 06 '16 at 20:40
  • @T.C. It does on the compilers I've tried somehow. Both compiles *and* swaps. – Barry Sep 06 '16 at 20:42
  • Hmm, libstdc++ is also nice enough to provide a pile of three `swap` overloads. – T.C. Sep 06 '16 at 20:45
  • @T.C. I'll write up a proposal. – Barry Sep 06 '16 at 20:48
  • @Barry I think someone is working on a full bit_reference/_pointer/whatever suite. Might want to check with them. – T.C. Sep 06 '16 at 20:49
  • @T.C. Ahh, [P0237](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0237r0.pdf)? Doesn't seem to have really been discussed in Oulu. – Barry Sep 06 '16 at 20:53