Can't put C++ Client object in std::map

[Jens Alfke]

Hi! I'm getting started with capnp, using C++ bindings (with C++17 compiled by Clang 12.) I'm having trouble storing remote object references, i.e. xx::Client objects, in STL collections. For example:

std::map<string,Foobar::Client> foobars;

...

Foobar::Client c = promise.wait(waitScope).getFoobar();

foobars.insert({name, c});    // Compile error

The error is "the parameter for this explicitly-defaulted copy constructor is const, but a member or base requires it to be non-const."

I eventually worked out that the error is because Client's copy constructor takes a `Client&` parameter, i.e. requires a mutable reference, which breaks the std::pair class because it needs to be able to copy a const reference. And if you can't put a client into a std::pair, you can't put it into a std::map or std::unordered_map.

I assume there must be a reason for leaving out the standard `const` in the copy constructor's parameter; but I can't work out what it would be. I know Client objects are wrappers for a pointer to a ref-counted value, so copy-constructing one bumps the value's ref-count, but that value is a separate object so it shouldn't matter whether the Client reference is const or not. (FYI, I've implemented my own ref-counted objects in C++ so I'm pretty familiar with the details...)

Is there a good workaround for storing RPC client objects in STL maps? Thanks!

--Jens

[Kenton Varda]

This is arguably a bug in the C++ standard library, but the reason it hits KJ particularly hard is because of KJ's philosophy about constness:

https://github.com/capnproto/capnproto/blob/master/style-guide.md#constness

In particular:

- Constness should be transitive. This implies that copying from a const value can only be allowed when the copy is a deep copy. Otherwise, making a copy would discard the transitive constness on the shared backing objects. Cap'n Proto `Client` objects are copy-by-refcount, so shallow copies, hence cannot be const copies.

- Constness should imply thread safety. Cap.'n Proto `Client` objects are refcounted, and the refcounting is not thread-safe (since thread-safe refcounting is very slow and these objects are tied to a thread-local event loop anyway).

Unfortunately, the C++ standard library mostly takes the view that `const` is shallow. To be fair, this is consistent with how built-in pointer and reference types work, but it is also a much less useful way of using const.

Annoyingly, the C++ standard library containers work just fine with move-only types, but choke on types that are non-const-copyable. These containers really ought to default to using move semantics, or automatically fall back to it when the copy constructor is non-const. I consider it a bug in the standard library implementation that it errors instead.

Luckily, you can work around the problem by wrapping the type in a struct that only has a move constructor, not a copy constructor, which forces the library to use move semantics only:

struct Wrapper<T> {

  Wrapper(Wrapper&) = delete;

  Wrapper(const Wrapper&) = delete;

  Wrapper(Wrapper&&) = default;

  T value;

}

Now you can use `std::map<string,Wrapper<Foobar::Client>>`.

Another alternative is to use `kj::HashMap`, which doesn't suffer these problems, and is faster than `std::unordered_map`.

-Kenton

--
You received this message because you are subscribed to the Google Groups "Cap'n Proto" group.
To unsubscribe from this group and stop receiving emails from it, send an email to capnproto+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/capnproto/63af6853-ac43-4c75-a161-a4c939199e93n%40googlegroups.com.