Read-only bindings for C++ const classes

[lyskov]

Dear Developers,

I wonder if it will be possible update Pybind11 so it honor C++ const correctness? In my package i have lots of functions that return MyClass const & and our users mistakenly tries to alter such objects witch lead to undefined behavior. If such const objects could be bound in some kind 'read-only' mode it would be really helpful!

Thank you,

[jagerman]

It's theoretically possible, but it would really depend on how much complexity it added to pybind11 and what the limitations would be. I don't think either would be small, but I don't think anyone has thought deeply about it since early pybind11 days (well before my time).

[wjakob]

A key change which happened a while back is that the registered_instances map is now a multimap:

std::unordered_multimap<const void *, void*> registered_instances; // void * -> PyObject*

This means that several Python objects (possibly with different types) can map to the same C++ instance, which makes supporting const feasible.

[wjakob]

I've pushed a sketch of how pybind11 'const' support could work here: wjakob@6491cf2.

The change introduces a per-instance const_ flag as well as checks which detect violations during casts. Example usage:

#include <pybind11/pybind11.h>

namespace py = pybind11;

struct Pet {
    Pet(const std::string &name) : name(name) { }
    void set_name(const std::string &name_) { name = name_; }
    const std::string &get_name() const { return name; }

    std::string name;
};

struct PetShop {
    Pet &polly() { return polly_; }
    const Pet &polly_const() const { return polly_; }

    Pet polly_{"polly"};
};

PYBIND11_PLUGIN(example) {
    py::module m("example", "pybind11 example plugin");

    py::class_<Pet>(m, "Pet")
        .def(py::init<const std::string &>())
        .def("set_name", &Pet::set_name)
        .def("name", &Pet::get_name);

    py::class_<PetShop>(m, "PetShop")
        .def(py::init<>())
        .def("polly", &PetShop::polly, py::return_value_policy::reference_internal)
        .def("polly_const", &PetShop::polly_const, py::return_value_policy::reference_internal);

    return m.ptr();
}

Example:

Python 3.5.2 |Anaconda 4.2.0 (x86_64)| (default, Jul  2 2016, 17:52:12)
[GCC 4.2.1 Compatible Apple LLVM 4.2 (clang-425.0.28)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import example
>>> p = example.PetShop()
>>> a = p.get_polly()
>>> b = p.get_polly_const()
>>> a.set_name("Polly 2")
>>> print(b.name())
Polly 2
>>> b.set_name("Polly 3")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: set_name(): incompatible function arguments. The following argument types are supported:
    1. (self: example.Pet, arg0: str) -> None

Invoked with: <example.Pet object at 0x10072f8a0>, 'Polly 3'

There are still a bunch of problems though:

1. Passing const to non-const arguments is now caught, but the error message is not very helpful.

2. There are no const annotations in the help, and it's not clear how those should look for compatibility with Sphinx and other libraries. PEP 484 has nothing to say either.

3. The change caused a regression, and the test suite now fails

4. The shared pointer casters don't do proper const handling yet.

The extra checks add +1.5% to the binary size of our test suite.

I just thought I'd throw this out to see what others think, and whether this is something we generally want.

[jagerman]

Interesting approach!

Some thoughts:

If I'm understanding correctly, there will be two separate Python instances for the same reference. I think it would be preferable to just have one with const_ = false indicating that we've seen a non-const reference somewhere along the line. Thus if one method returns a const T & and another returns a T &, we change the wrapper to non-const, but use the same wrapper.The main advantage is that Python then sees this as two references to the same instance, rather than seeing them as two separate instances. It would also need some minor changing to the logic, to require something along the lines of want_const || !have_const instead of just want_const == have_const, but that seems a minor detail.

1.5% seems pretty reasonable for this feature.

[wjakob]

If I follow your suggestion correctly, I wonder if that will lead to desirable semantics? In that case, in the above example, the second call to b.set_name() would succeed because a non-const reference to the object in question existed and would have precedence over the const one. If we map const to Python, then I think it should behave fairly similarly to the C++ side.

[jagerman]

Yeah, it would. On the other hand, a is b would be true, but I don't think it will be in the example above; it's really a tradeoff between those two ideas, and it doesn't seem possible to support both.

[jagerman]

I suppose I'd think of it along the lines of, in C++: If I have a mutable lvalue reference a and const lvalue reference b to the same object, I can always mutate b by mutating a. I could even (though this is a bit unusual) safely const_cast away the constness of b, since I also hold a non-const reference to the same object. The Python-side promotion to non-const would be essentially the same.

[wjakob]

I think there is a problematic aspect of this, which might lead to hard-to-find bugs. Imagine the following contrived piece of code which should probably fail:

b = a.get_const_reference()
blackbox()
b.mutate()

However, ifblackbox() acquires a non-const reference, the b.mutate() call will succeed. Due to the global connection of const and non-const references, a later change in blackbox() may trigger this fault. At that point, it may be very hard to understand why something that previously worked now doesn't.

[jagerman]

Hmm, yeah, that's true; the promotion would ideally need to be coupled with a demotion when the non-const reference goes out of scope, which doesn't seem feasible. So I suppose the dual instances tradeoff is indeed the better approach.

[EricCousineau-TRI]

One concern with the dual instance method: How would this be handled with inheritance?
It doesn't seem like you can store the dual instances in Pybind and have Python classes derived from a pybind C++ class honor this mutability.

This point may be moot if your concern is the C++ const-ness of existing objects, and assume that any derived Python objects are Pythonic in nature (and thus care must be taken). However, this does mean that derived Python objects would bypass any const-checks that'd be done for pure C++ instances, since you can't really "duplicate" the object with another Python instance. That being said, you could do it if you make it a proxy from the get-go.

As a potential related solution, I tinkered with teaching Python some basics of C++ const-ness using const-proxies.
I can't say this is something that should make it into to core pybind (as it's (a) kinda complicated and (b) simplest to implement in pure Python), but could be considered for an extension module type thing?
I have the pure Python prototype and some notes on pybind integration here:
RobotLocomotion/drake#7793 (comment)
(This mentions why this is important to us: we have cache invalidation based on access to mutators, rather than hashing or what not.)

I have tinkered with some other basic "extension modules" here, and the paradigm seems simple enough:
https://github.com/RobotLocomotion/drake/tree/c5dad75/bindings/pydrake/util

UPDATE:
Here's an example integrating with pybind:
cpp_const_pybind.h
cpp_const_pybind_test.py

[sizmailov]

Please correct me if I'm wrong here.

From type system point of view T and const T are two different types. Type T is treated as derived from const T. Same rule applies to references and pointers.

Python has no concept of const, but it could be easily emulated via explicit inheritance scheme:

class constA(object):
    pass
class A(constA):
    pass

This hack is a way to propagate const-correctness from C++ to Python.

Advantages:

• Could be done on top of any version of pybind11
• Template machinery for that could be in a separate header and will not affect one who don't want that feature (don't pay for what you don't use)
• IDE / static analysis tools could find const-incorrect code before you run it

Disadvantages:

• More scaffolds are required on C++ side
• Number of types is doubled
• Compilation time and binary size will grow (would it dramatically affect performance?)

At first glance this approach is free of inheritance and mutated blackbox issues described above.

What do you think about it?

Here is fast&dirty illustration of the idea. Since you can't pass const T as base class of T to pybind, you need a helper classes.

class A{
public:
  std::string f() const{ return std::string(__PRETTY_FUNCTION__);}
  std::string g(){ return std::string(__PRETTY_FUNCTION__); }
};

namespace py = pybind11;

template<typename T>
struct ConstValueWrapper{
  static_assert(!std::is_const<T>::value,"");
  static_assert(!std::is_reference<T>::value,"");
  static_assert(!std::is_pointer<T>::value,"");
  const T& cref() const{return value;}

protected:
  T& ref(){return this->value;}
  T value;
};

template<typename T>
struct ValueWrapper : public ConstValueWrapper<T> {
using ConstValueWrapper<T>::ref;
};


PYBIND11_MODULE(example, m) {

  py::class_<ConstValueWrapper<A>>(m,"constA")
      .def(py::init<>())
      .def("f",[](const ConstValueWrapper<A>& ref){ return ref.cref().f(); })
      ;
  py::class_<ValueWrapper<A>,ConstValueWrapper<A>>(m,"A")
      .def(py::init<>())
      .def("g",[](ValueWrapper<A>* ref){ return ref->ref().g(); })
      ;

}

Usage from python

from example import *

def accept_const_A(a: constA):
    assert isinstance(a,constA)


def accept_A(a: A):
    assert isinstance(a, A)

a = A()
consta = constA()


accept_const_A(a) # ok, prints "std::__cxx11::string A::f() const"
accept_const_A(consta) #ok, prints "std::__cxx11::string A::f() const"

accept_A(a) # ok, prints "std::__cxx11::string A::f() const \n std::__cxx11::string A::g()"
accept_A(consta) # assertion triggered (PyCharm is able to find this simple error)

class C(A):
    pass

c = C()  # there is no way to obtain pure-python const C

accept_A(c) # ok
accept_const_A(c) # ok

[EricCousineau-TRI]

Cool stuff! That does look much simpler for inheritance!

[...] free of inheritance [...] issues [...]

I guess I hadn't highlighted this in the solution I mentioned, but since Python is rich enough (for better and worse) to offer __instancecheck__, something like isinstance(x_const, list) could be equivalent to isinstance(x, list) if this were implemented.
issubclass could also be overridden with __subclasscheck__; however, getting the type itself would require a proxy function, so your approach is definitely better in that regards.

[...] and mutated blackbox issues described above [...]

Sorry for my ignorance, but could you specify what issues these are?

One concern here is that this approach does not handle inheritance, as you mentioned. If you have a method that intends to pass a const T& where T is virtual, and you have Python-derived class which extends T, the auto-downcasting from pybind11 will automatically strip the const-ness from that object, meaning you'll be able to mutate the properties inherited from T in this instance.

This is handled using the const-proxy approach (this is in pure-Python, but can be easily decorated in C++):
https://github.com/RobotLocomotion/drake/blob/fdcc7a81624d2fb283139a8a150434fa14547c2d/bindings/pydrake/util/test/cpp_const_test.py#L132

More scaffolds are required on C++ side

Perhaps you could write a wrapper around py::class_ that could automatically dispatch between the mutable wrapper and the const wrapper, and automatically create the two classes; it could also possible handle inheritance as well. May require some fine crafting, but it does seem viable!

If you're interested in using it, I have some basic C++14 code for wrapping functions (methods, pointers, functors, etc.) if you have a pattern that you can write with some specializations + SFINAE:
https://github.com/RobotLocomotion/drake/blob/fdcc7a81624d2fb283139a8a150434fa14547c2d/bindings/pydrake/util/test/wrap_function_test.cc#L217
Example usage geared towards a workaround for callbacks + mutation:
https://github.com/EricCousineau-TRI/drake/blob/feature/py_system_scalar_types_user-wip/bindings/pydrake/util/wrap_pybind.h#L70

[sizmailov]

By blackbox issue I mean a problem described here #717 (comment)

One concern here is that this approach does not handle inheritance, as you mentioned. If you have a method that intends to pass a const T& where T is virtual, and you have Python-derived class which extends T, the auto-downcasting from pybind11 will automatically strip the const-ness from that object, meaning you'll be able to mutate the properties inherited from T in this instance.

I disagree on that. Here I do not expose to python T nor const T, only wrappers around them. While wrapper<T> could be casted to T& or const T& on C++ side, the conversion from wrapper<T> to const_wrapper<T> is not provided at all.