Add support for private virtual functions

robotpy_build/hooks.py

@@ -861,6 +861,7 @@ def class_hook(self, cls, data):
                     continue
 
                 is_private = access == "private"
+                is_virtual = fn["override"] or fn["virtual"]
 
                 # this has to be done even on private functions, because
                 # we do overload detection here
@@ -869,7 +870,8 @@ def class_hook(self, cls, data):
                     fn, signature, cls_key, class_data, is_private
                 )
 
-                if not is_private:
+                # Have to process private virtual functions too
+                if not is_private or is_virtual:
                     if method_data.ignore:
                         fn["data"] = method_data
                         continue

robotpy_build/include/robotpy_build.h

@@ -19,6 +19,13 @@ namespace rpybuild_ext {
 using py::raise_from;
 } // namespace rpybuild_ext
 
+#define RPYBUILD_BAD_TRAMPOLINE \
+    "has an abstract trampoline -- and they must never be abstract! One of " \
+    "the generated override methods doesn't match the original class or its " \
+    "bases, or is missing. You will need to provide method and/or param " \
+    "overrides for that method. It is likely the following compiler error " \
+    "messages will tell you which one it is."
+
 // Use this to define your module instead of PYBIND11_MODULE
 #define RPYBUILD_PYBIND11_MODULE(variable) PYBIND11_MODULE(RPYBUILD_MODULE_NAME, variable)
 

robotpy_build/templates/cls.cpp.j2

@@ -29,7 +29,9 @@
   strings are generated properly. First part is to register the class with
   pybind11, second part is to generate all the methods/etc for it.
 
+  TODO: make type_traits optional by detecting trampoline
 #}
+#include <type_traits>
 
 {% for cls in header.classes
    if cls.namespace and not cls.parent and not cls.data.ignore and cls.template is not defined %}

robotpy_build/templates/cls_trampoline.hpp.j2

@@ -143,8 +143,8 @@ struct PyTrampoline_{{ cls.x_qualname_ }} : PyTrampolineBase, virtual py::trampo
 {% endfor %}
 
     {# virtual methods (disabled for buffer overrides for now) #}
-    {% for fn in cls.methods.public + cls.methods.protected
-           if not fn.data.ignore and (fn.virtual or fn.override) and not fn.final and not fn.data.buffers %}
+    {% for fn in cls.methods.public + cls.methods.protected + cls.methods.private
+           if (fn.virtual or fn.override) and not fn.data.ignore and not fn.final and not fn.data.buffers %}
 #ifndef RPYGEN_DISABLE_{{ trampoline_signature(fn) }}
     {{ fn.rtnType }} {{ fn.name }}({{ fn.parameters | join(', ', attribute='x_decl') }}){%
         if fn.const %} const{% endif

robotpy_build/templates/pybind11.cpp.j2

@@ -260,6 +260,7 @@
 {%- macro cls_decl(cls) -%}
   {%- if cls.x_has_trampoline %}
   using {{ cls.x_trampoline_var }} = {{ cls.x_trampoline_name }};
+    static_assert(std::is_abstract<{{ cls.x_trampoline_var }}>::value == false, "{{ cls.x_qualname }} " RPYBUILD_BAD_TRAMPOLINE);
   {% endif -%}
   py::class_<typename {{ cls.x_qualname }}
     {%- if cls.data.nodelete -%}

tests/cpp/rpytest/ft/__init__.py

@@ -53,6 +53,7 @@
     PChild,
     PGChild,
     PatientRef,
+    PrivateAbstract,
     RenamedClass,
     RenamedEnum,
     StaticOnly,
@@ -150,6 +151,7 @@
     "PChild",
     "PGChild",
     "PatientRef",
+    "PrivateAbstract",
     "RenamedClass",
     "RenamedEnum",
     "StaticOnly",

tests/cpp/rpytest/ft/include/abstract.h

@@ -3,4 +3,17 @@ struct Abstract
 {
     virtual ~Abstract() {}
     virtual int mustOverrideMe() = 0;
-};
+};
+
+struct PrivateAbstract
+{
+    PrivateAbstract() {}
+
+    static int getPrivateOverride(PrivateAbstract *p) {
+        return p->mustOverrideMe();
+    }
+
+private:
+    virtual int mustOverrideMe() = 0;
+};
+

tests/test_ft_misc.py

@@ -1,5 +1,6 @@
 from rpytest import ft
 import pytest
+import re
 
 #
 # main module
@@ -46,6 +47,39 @@ def test_good_abstract():
     assert m.mustOverrideMe() == 0x3
 
 
+class MyBadPrivateAbstract(ft.PrivateAbstract):
+    pass
+
+
+def test_private_abstract():
+    # it's private, you can't call it
+    assert not hasattr(ft.PrivateAbstract, "_mustOverrideMe")
+
+
+def test_bad_private_abstract():
+    m = MyBadPrivateAbstract()
+
+    with pytest.raises(
+        RuntimeError,
+        match=r".*"
+        + re.escape(
+            'does not override required function "PrivateAbstract::_mustOverrideMe"'
+        ),
+    ):
+        ft.PrivateAbstract.getPrivateOverride(m)
+
+
+class MyGoodPrivateAbstract(ft.PrivateAbstract):
+    def _mustOverrideMe(self):
+        return 0x3
+
+
+def test_good_private_abstract():
+    m = MyGoodPrivateAbstract()
+    assert m._mustOverrideMe() == 0x3
+    assert ft.PrivateAbstract.getPrivateOverride(m) == 0x3
+
+
 #
 # factory.h
 #