problems with transparent Compare

[dixlorenz]

This code

using Map = fc::vector_set<std::string, std::less<>>;
Map m;
auto found = m.find("Key");

does not compile:

flat_impl.hpp:434:30: error: no matching function for call to object of type 'fc::impl::flat_set_base<fc::flat_set<std::__1::vector<std::__1::basic_string<char>, std::__1::allocator<std::__1::basic_string<char> > >, std::__1::less<void> >, std::__1::basic_string<char>, std::__1::vector<std::__1::basic_string<char>, std::__1::allocator<std::__1::basic_string<char> > >, std::__1::less<void>, int>::value_compare' (aka 'std::__1::less<void>') if (it == self()->end() || self()->value_comp()(std::ref(key), *it))

ending with

no matching function for call to object of type 'std::__1::less<void>' if (__comp(*__m, __value_))

This is on macOS, Xcode 12.4, using libc++. Using "std::ref" in the find/lower_bound function leads to less::operator(...) being ignored because of a substitution failure. When I take out std::ref it does seem to work. Using a vector_map works.

On the other hand:

std::string_view sv("Key"); auto found = m.find(sv);

works with vector_set, but not vector_map; it balks in first_compare, cannot match "reference_wrapper" (in code transparent_key_t) against basic_string_view.

[pubby]

Thanks for the report!

I put out a quick fix just now, but haven't had too much time to test. Let me know if the issue persists.

[dixlorenz]

My 2 examples compile now, but I can't check my actual code. I had been using a version from July 2020; the new version blows up other stuff in my code:

flat_map_base::mapped_type is declared private.

After fixing that, I get errors when doing something like

auto it = map.find("key");
it->second = "something else";

The compiler complains about it->second being const; I can't figure out why it would think that or how to fix that.

And I get some "call to member function "find" is ambiguous" errors with

template<class P> iterator insert(const_iterator hint, P&& value)
and
template<class InputIt> void insert(InputIt first, InputIt last)

when doing something like

map.insert(othermap.begin(), othermap.end());

[pubby]

All iterators for flat containers are const_iterators by default, so the first behavior is intentional. You can modify the key by using operator[], by using the has member function, or by using.underlying on the iterator. The README says a little more on this.

map["key"] = "something else";
// or
*map.has("key") = "something else";
// or
auto it = map.find("key");
*it->second.underlying = "something else";

And good find! The second issue should be fixed now.

[dixlorenz]

If that behavior is intentional, that would be different to a std::map (and different to before). I understand why, but it is somewhat unfortunate for generic code. For example, one of my own free functions is an "insert_or_assign" that so far worked on all maps; it does not expect a key but does a find using the transparent comparison and either sets it->second or does an emplace_hint.

Not that it does much good here, AFAICT emplace_hint ignores the hint anyway, but at least it did compile ;-)

FYI: in the last example you wrote "find" instead of "has":

if(std::string* value = map.find("qux"))

Suggestion: since has() is just a fancy wrapper around find (and not standardized), it should also use the transparent comparator.

[pubby]

Yeah, it's certainly a downside to the implementation, but does provide a small amount of safety.

And that's a good idea; I went ahead and added a transparent .has version.

[dixlorenz]

Thank you, AFAICT I should be able to get my code to compile again. Final question: do you intend to improve emplace_hint to actually use the hint?

[pubby]

No problem. I'd like to get around to it someday but it's not a high priority for me.