Tessil / ordered-map

C++ hash map and hash set which preserve the order of insertion
MIT License
512 stars 66 forks source link

tsl::ordered_map<int, const std::string> failed to compile #11

Closed Mark-Joy closed 6 years ago

Mark-Joy commented 6 years ago

The below code is compiled just fine with previous version - when ordered_map, set, and hash are in the same file (ordered_map.h).

With latest library, the below code failed to compile if string is const

const tsl::ordered_map<int, const std::string> testmap{
    { 0, "Test1" },
    { 1, "Test2" },
 };

The below code with no const compile just fine with latest library

const tsl::ordered_map<int, std::string> testmap{
    { 0, "Test1" },
    { 1, "Test2" },
 };
Tessil commented 6 years ago

Thank you for the report.

Unfortunately I'm currently not at home (won't be for the next weeks) and without a laptop with me. I'll check that when I get back as at first glance I don't see where the error can come from.

Alexhuszagh commented 6 years ago

@Tessil Reverting to 5773aa1 fixes the issue on my end. Seems like the checking of the value type should be lenient on CV-correctness.

Tessil commented 6 years ago

Sorry for the delay. I had some time to look at it, and I'm a bit confused by the reason it doesn't compile.

std::deque<std::pair<int, const std::string>> dq;

dq.push_back({0, "Test1"});
//dq.insert(dq.end(), {0, "Test1"}); // Doesn't work on clang and gcc

It seems that std::deque and std::vector require the type to be move-constructible to use the insert method (and emplace with an iterator). It can't use the copy constructor (maybe for the strong exception guarantee? Though I tested with a nothrow copy-only object, and it doesn't work either).

I have to check a bit more, but at worst I can just refactor the code as a simple insert in the map is equivalent to a push_back. The insert_at_position would just require the type to be movable.

@Alexhuszagh Are you sure? The problem should come from the commit 5a430e2dda68577995019376d65b1fd52ac74f61.

Tessil commented 6 years ago

Pushed the fix. Warn me if there is any problem.