From 7672292e6bcdf3ca4e345019b41995031a37766e Mon Sep 17 00:00:00 2001
From: Jason Rhinelander <jason@imaginary.ca>
Date: Tue, 3 Oct 2017 10:09:49 -0300
Subject: [PATCH] Add informative compilation failure for method_adaptor
 failures

When using `method_adaptor` (usually implicitly via a `cl.def("f",
&D::f)`) a compilation failure results if `f` is actually a method of
an inaccessible base class made public via `using`, such as:

    class B { public: void f() {} };
    class D : private B { public: using B::f; };

pybind deduces `&D::f` as a `B` member function pointer.  Since the base
class is inaccessible, the cast in `method_adaptor` from a base class
member function pointer to derived class member function pointer isn't
valid, and a cast failure results.

This was sort of a regression in 2.2, which introduced `method_adaptor`
to do the expected thing when the base class *is* accessible.  It wasn't
actually something that *worked* in 2.1, though: you wouldn't get a
compile-time failure, but the method was not callable (because the `D *`
couldn't be cast to a `B *` because of the access restriction).  As a
result, you'd simply get a run-time failure if you ever tried to call
the function (this is what #855 fixed).

Thus the change in 2.2 essentially promoted a run-time failure to a
compile-time failure, so isn't really a regression.

This commit simply adds a `static_assert` with an accessible-base-class
check so that, rather than just a cryptic cast failure, you get
something more informative (along with a suggestion for a workaround).

The workaround is to use a lambda, e.g.:

    class Derived : private Base {
    public:
        using Base::f;
    };

    // In binding code:
    //cl.def("f", &Derived::f); // fails: &Derived::f is actually a base
                                // class member function pointer
    cl.def("f", [](Derived &self) { return self.f(); });

This is a bit of a nuissance (especially if there are a bunch of
arguments to forward), but I don't really see another solution.

Fixes #1124
---
 include/pybind11/detail/common.h |  5 +++++
 include/pybind11/pybind11.h      | 12 ++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h
index b39cc0ec9..784bfbac6 100644
--- a/include/pybind11/detail/common.h
+++ b/include/pybind11/detail/common.h
@@ -581,6 +581,11 @@ template <typename T, typename... Us> using deferred_t = typename deferred_type<
 template <typename Base, typename Derived> using is_strict_base_of = bool_constant<
     std::is_base_of<Base, Derived>::value && !std::is_same<Base, Derived>::value>;
 
+/// Like is_base_of, but also requires that the base type is accessible (i.e. that a Derived pointer
+/// can be converted to a Base pointer)
+template <typename Base, typename Derived> using is_accessible_base_of = bool_constant<
+    std::is_base_of<Base, Derived>::value && std::is_convertible<Derived *, Base *>::value>;
+
 template <template<typename...> class Base>
 struct is_template_base_of_impl {
     template <typename... Us> static std::true_type check(Base<Us...> *);
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index 59af02292..90b8ebcc6 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -997,10 +997,18 @@ template <typename /*Derived*/, typename F>
 auto method_adaptor(F &&f) -> decltype(std::forward<F>(f)) { return std::forward<F>(f); }
 
 template <typename Derived, typename Return, typename Class, typename... Args>
-auto method_adaptor(Return (Class::*pmf)(Args...)) -> Return (Derived::*)(Args...) { return pmf; }
+auto method_adaptor(Return (Class::*pmf)(Args...)) -> Return (Derived::*)(Args...) {
+    static_assert(detail::is_accessible_base_of<Class, Derived>::value,
+        "Cannot bind an inaccessible base class method; use a lambda definition instead");
+    return pmf;
+}
 
 template <typename Derived, typename Return, typename Class, typename... Args>
-auto method_adaptor(Return (Class::*pmf)(Args...) const) -> Return (Derived::*)(Args...) const { return pmf; }
+auto method_adaptor(Return (Class::*pmf)(Args...) const) -> Return (Derived::*)(Args...) const {
+    static_assert(detail::is_accessible_base_of<Class, Derived>::value,
+        "Cannot bind an inaccessible base class method; use a lambda definition instead");
+    return pmf;
+}
 
 template <typename type_, typename... options>
 class class_ : public detail::generic_type {