6
\$\begingroup\$

What I tried to do is compare arbitrary structures independent of the == operator implementation.

template<typename T>
bool bitWiseCompare(T a, T b)
{
  size_t size = sizeof(T) / sizeof(char);
  char* array_a = (char*)(&a);
  char* array_b = (char*)(&b);
  for(size_t i = 0; i<size; i++)
  {
    if(array_a[i] != array_b[i])
    {
      return false;
    }
  }
  return true;
}
\$\endgroup\$
  • 6
    \$\begingroup\$ return 0==memcmp(&a,&b,sizeof(T)); \$\endgroup\$ – Emily L. Mar 18 '18 at 22:08
  • 1
    \$\begingroup\$ Thats very dangerous. Only really useful for buffers which is why we use operator== normally. \$\endgroup\$ – Martin York Mar 19 '18 at 3:27
  • \$\begingroup\$ @Darius, you do know that sizeof reports in units of char, making sizeof (char) equal to 1 by definition, don't you? \$\endgroup\$ – Toby Speight Mar 19 '18 at 11:44
12
\$\begingroup\$

This comparison can fail if there are any padding bytes within T, as those extra bytes are not initialized to any particular value.

Another source of failure can come from inheritance. If one of the objects is part of a larger class (T is being used as a base class) with virtual functions, the vtable pointers can be different if a and b are part of different class hierarchies. Virtual inheritance of base classes of T can also cause problems.

sizeof(char) is always 1, so there is no need to divide by it.

Since you're only reading from your parameters, you should take them by const &T to avoid making unnecessary copies.

\$\endgroup\$
  • \$\begingroup\$ In other words, this comparison can only handle POD types nicely. \$\endgroup\$ – bipll Mar 19 '18 at 13:49
  • 1
    \$\begingroup\$ @bipll: while it'll work for POD types on most modern machines, it's not really even certain to work for them. Even a plain int is allowed to contain padding bytes. \$\endgroup\$ – Jerry Coffin Mar 19 '18 at 15:00
  • \$\begingroup\$ Ok, there should be 'at most' rather than 'only'. :) \$\endgroup\$ – bipll Mar 19 '18 at 15:08
6
\$\begingroup\$

The C++ way isn't necessarily to stay away of such things as bit comparisons, but rather to do it in the most obvious way possible -by that I mean that your code should look dangerous and explicit at the same time. You have tools for this at your disposal: namespaces, ugly looking casts and algorithms:

#include <algorithm>
namespace unsafe {
    template <typename T>
    constexpr auto begin(const T& item) {
        return reinterpret_cast<const char*>(&item);
    }
    template <typename T>
    constexpr auto end(const T& item) {
        return reinterpret_cast<const char*>(&item)+sizeof(T);
    }
    template <typename T>
    auto bitwise_equal(const T& lhs, const T& rhs) {
        return std::equal(begin(lhs), end(lhs), // will become constexpr with C++20
                          begin(rhs), end(rhs));
    }
}

So it won't look innocuous when you use it:

int main() {
    auto a = 125.5;
    std::cout << std::boolalpha << unsafe::bitwise_equal(a, a);
} 

A few further remarks:

  • bitwise_compare isn't the most accurate name for the function, since compare is traditionally reserved for functions returning order (-1, 0, 1), and not only equality.

  • providing begin() and end() isn't necessary, since begin(item)+sizeof(T) is as correct, and probably as readable a replacement, but aesthetic reasons are sometimes enough

  • std::equal is set to become constexpr with the new standard, meaning this C++'s style implementation might become more efficient than the memcmp implementation given compile-time values.

  • it's generally better to use the four-argument versions of algorithms over two ranges; in this case it is obviously redundant since the length of the two arguments are by construction equal, but then consistency is a virtue.

  • reusing algorithms is a good way to avoid dumb but frequent errors one would make with a raw loop,

for instance:

auto  pa = reinterpret_cast<const char*>(&a);
for (auto pb = reinterpret_cast<const char*>(&b); pb != pb+sizeof(T); ++pb) // oops!
    // ...
\$\endgroup\$
  • \$\begingroup\$ Do you have a preference in end() for repeating the cast, rather than just writing begin() + sizeof item? If so, consider explaining your reasoning. Similarly, is there any benefit to including the fourth argument to std::equal()? \$\endgroup\$ – Toby Speight Mar 19 '18 at 11:48
  • \$\begingroup\$ @TobySpeight: what is the policy, editing the post or adding explanations in a comment? \$\endgroup\$ – papagaga Mar 19 '18 at 12:07
  • \$\begingroup\$ It's better to edit your answer to improve the explanation, as comments can be lost, or crowded out by other comments. The only edits we don't allow here on CR are edits to code in Questions after any answers are received (so that the answers aren't invalidated). \$\endgroup\$ – Toby Speight Mar 19 '18 at 13:10
  • \$\begingroup\$ Perhaps adding a ref-comparison first makes sense? return &lhs == &rhs || std::equal(...); \$\endgroup\$ – bipll Mar 19 '18 at 13:51
  • 1
    \$\begingroup\$ While std::equal is set to become constexpr, reinterpret_cast isn't. So this won't become faster than memcmp anytime soon. I wonder if it would make sense to create a small wrapper, byte_view<T> which would allow the use of the ranges proposal. This would obviate the need to use std::equal with iterators. \$\endgroup\$ – Giel Mar 20 '18 at 13:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.