2

I have the following unordered_maps:

 struct a{
        std::string b;
    };

    int main()
    {
        std::unordered_map<std::string, std::string> source;
        source["1"] = "Test";
        source["2"] = "Test2";

        std::unordered_map<std::string, a> dest = *reinterpret_cast<std::unordered_map<std::string, a>*>(&source);

        std::cout << dest["1"].b << std::endl;
        std::cout << dest["2"].b << std::endl;

    }

Using a reinterpret_cast I convert source into dest. This works since struct a only consists of a std::string.

My question: Is this actually good pratice? GCC yields the following warning:

dereferencing type-punned pointer will break strict-aliasing rules

Can I safely ignore this? Or is there any potential drawback of just casting the raw bytes of STL container?

(cpp.sh/5r2rh)

  • 6
    And have you [read about strict aliasing](http://stackoverflow.com/questions/98650/what-is-the-strict-aliasing-rule)? If the compiler is giving you a warning, it is for a reason and it's telling you that you are doing something which you should not really do. – Some programmer dude May 21 '17 at 21:12
  • ok so reinterpret_cast is not an option in this case. (even though I compiled it with MSVC and GCC on different -O levels and it worked just fine) Is there any other way than actually copying the maps? –  May 21 '17 at 21:17
  • The only standard and good way to copy the map is to loop over each element in the first and add it to the second. Unless the source map is extremely big or you have to do it hundreds of times a second it will not really be noticeable. – Some programmer dude May 21 '17 at 21:19

1 Answers1

3

No that is not good practice. Your code is not safe. In fact it's the opposite: undefined behavior, meaning sometimes it works sometimes it won't, even without telling you.

The real problem is that you have no "legal" way to convert a std::string to an struct a. This isn't C, don't use stuff as plain bytes, use the type system of the language. Then the compiler will help you do avoid bad mistakes.

This is my solution:

#include <unordered_map>
#include <string>
#include <iostream>
#include <algorithm>

struct a {
    std::string b;
    a () = default;
    a (const std::string& b) : b(b){}
};

int main() {
    std::unordered_map<std::string, std::string> source;
    source["1"] = "Test";
    source["2"] = "Test2";

    std::unordered_map<std::string, a> dest;

    std::transform(source.cbegin(),source.cend(),std::inserter(dest,dest.end()),[](const auto& value)
    {
        return std::forward_as_tuple(value.first,value.second);
    });

    std::cout << dest["1"].b << std::endl;
    std::cout << dest["2"].b << std::endl;
}

If you have performance concerns, you can also add a move constructor and more, but trust me, readable clean code, is fast code. Otherwise the bootle neck is not that non casting code, but the use of maps, copying instead of moving and other stuff. But don't optimize prematurely.

Superlokkus
  • 4,731
  • 1
  • 25
  • 57
  • 1
    Wonderful answer! "Know (and use) your stl." should always be your mantra. However please also point out the horrendously bad naming practice `struct a` with a non-type member `b` is just asking for non-readability. – iolo May 22 '17 at 11:57
  • @iolo Thank you very much, but I assume (or hope) that `struct a` is just existing for exemplary purposes. – Superlokkus May 22 '17 at 12:17