From e88656ab45ae75df7dcb1fcdd2c89805b52e4665 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Tue, 27 Feb 2018 22:33:41 -0400 Subject: [PATCH] Improve macro type handling for types with commas - PYBIND11_MAKE_OPAQUE now takes ... rather than a single argument and expands it with __VA_ARGS__; this lets templated, comma-containing types get through correctly. - Adds a new macro PYBIND11_TYPE() that lets you pass the type into a macro as a single argument, such as: PYBIND11_OVERLOAD(PYBIND11_TYPE(R<1,2>), PYBIND11_TYPE(C<3,4>), func) Unfortunately this only works for one macro call: to forward the argument on to the next macro call (without the processor breaking it up again) requires also adding the PYBIND11_TYPE(...) to type macro arguments in the PYBIND11_OVERLOAD_... macro chain. - updated the documentation with these two changes, and use them at a couple places in the test suite to test that they work. --- docs/advanced/cast/stl.rst | 3 --- docs/advanced/misc.rst | 33 +++++++++++++++++++++++++------- include/pybind11/cast.h | 8 ++++++-- include/pybind11/pybind11.h | 8 ++++---- tests/test_opaque_types.cpp | 10 +++++++--- tests/test_virtual_functions.cpp | 4 +++- 6 files changed, 46 insertions(+), 20 deletions(-) diff --git a/docs/advanced/cast/stl.rst b/docs/advanced/cast/stl.rst index 3f30c0290..468bd2c66 100644 --- a/docs/advanced/cast/stl.rst +++ b/docs/advanced/cast/stl.rst @@ -175,9 +175,6 @@ in Python, and to define a set of available operations, e.g.: }, py::keep_alive<0, 1>()) /* Keep vector alive while iterator is used */ // .... -Please take a look at the :ref:`macro_notes` before using the -``PYBIND11_MAKE_OPAQUE`` macro. - .. seealso:: The file :file:`tests/test_opaque_types.cpp` contains a complete diff --git a/docs/advanced/misc.rst b/docs/advanced/misc.rst index 5faf11f0d..be4699d36 100644 --- a/docs/advanced/misc.rst +++ b/docs/advanced/misc.rst @@ -7,13 +7,32 @@ General notes regarding convenience macros ========================================== pybind11 provides a few convenience macros such as -:func:`PYBIND11_MAKE_OPAQUE` and :func:`PYBIND11_DECLARE_HOLDER_TYPE`, and -``PYBIND11_OVERLOAD_*``. Since these are "just" macros that are evaluated -in the preprocessor (which has no concept of types), they *will* get confused -by commas in a template argument such as ``PYBIND11_OVERLOAD(MyReturnValue, myFunc)``. In this case, the preprocessor assumes that the comma indicates -the beginning of the next parameter. Use a ``typedef`` to bind the template to -another name and use it in the macro to avoid this problem. +:func:`PYBIND11_DECLARE_HOLDER_TYPE` and ``PYBIND11_OVERLOAD_*``. Since these +are "just" macros that are evaluated in the preprocessor (which has no concept +of types), they *will* get confused by commas in a template argument; for +example, consider: + +.. code-block:: cpp + + PYBIND11_OVERLOAD(MyReturnType, Class, func) + +The limitation of the C preprocessor interprets this as five arguments (with new +arguments beginning after each comma) rather than three. To get around this, +there are two alternatives: you can use a type alias, or you can wrap the type +using the ``PYBIND11_TYPE`` macro: + +.. code-block:: cpp + + // Version 1: using a type alias + using ReturnType = MyReturnType; + using ClassType = Class; + PYBIND11_OVERLOAD(ReturnType, ClassType, func); + + // Version 2: using the PYBIND11_TYPE macro: + PYBIND11_OVERLOAD(PYBIND11_TYPE(MyReturnType), + PYBIND11_TYPE(Class), func) + +The ``PYBIND11_MAKE_OPAQUE`` macro does *not* require the above workarounds. .. _gil: diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 3ec6f7288..8a680ed2d 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -2054,9 +2054,13 @@ object object_api::call(Args &&...args) const { NAMESPACE_END(detail) -#define PYBIND11_MAKE_OPAQUE(Type) \ +#define PYBIND11_MAKE_OPAQUE(...) \ namespace pybind11 { namespace detail { \ - template<> class type_caster : public type_caster_base { }; \ + template<> class type_caster<__VA_ARGS__> : public type_caster_base<__VA_ARGS__> { }; \ }} +/// Lets you pass a type containing a `,` through a macro parameter without needing a separate +/// typedef, e.g.: `PYBIND11_OVERLOAD(PYBIND11_TYPE(ReturnType), PYBIND11_TYPE(Parent), f, arg)` +#define PYBIND11_TYPE(...) __VA_ARGS__ + NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 977045d62..9eff45ddf 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1958,18 +1958,18 @@ template function get_overload(const T *this_ptr, const char *name) { } #define PYBIND11_OVERLOAD_NAME(ret_type, cname, name, fn, ...) \ - PYBIND11_OVERLOAD_INT(ret_type, cname, name, __VA_ARGS__) \ + PYBIND11_OVERLOAD_INT(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), name, __VA_ARGS__) \ return cname::fn(__VA_ARGS__) #define PYBIND11_OVERLOAD_PURE_NAME(ret_type, cname, name, fn, ...) \ - PYBIND11_OVERLOAD_INT(ret_type, cname, name, __VA_ARGS__) \ + PYBIND11_OVERLOAD_INT(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), name, __VA_ARGS__) \ pybind11::pybind11_fail("Tried to call pure virtual function \"" #cname "::" name "\""); #define PYBIND11_OVERLOAD(ret_type, cname, fn, ...) \ - PYBIND11_OVERLOAD_NAME(ret_type, cname, #fn, fn, __VA_ARGS__) + PYBIND11_OVERLOAD_NAME(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), #fn, fn, __VA_ARGS__) #define PYBIND11_OVERLOAD_PURE(ret_type, cname, fn, ...) \ - PYBIND11_OVERLOAD_PURE_NAME(ret_type, cname, #fn, fn, __VA_ARGS__) + PYBIND11_OVERLOAD_PURE_NAME(PYBIND11_TYPE(ret_type), PYBIND11_TYPE(cname), #fn, fn, __VA_ARGS__) NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/tests/test_opaque_types.cpp b/tests/test_opaque_types.cpp index 5e83df0f6..0d20d9a01 100644 --- a/tests/test_opaque_types.cpp +++ b/tests/test_opaque_types.cpp @@ -11,10 +11,14 @@ #include #include -using StringList = std::vector; +// IMPORTANT: Disable internal pybind11 translation mechanisms for STL data structures +// +// This also deliberately doesn't use the below StringList type alias to test +// that MAKE_OPAQUE can handle a type containing a `,`. (The `std::allocator` +// bit is just the default `std::vector` allocator). +PYBIND11_MAKE_OPAQUE(std::vector>); -/* IMPORTANT: Disable internal pybind11 translation mechanisms for STL data structures */ -PYBIND11_MAKE_OPAQUE(StringList); +using StringList = std::vector>; TEST_SUBMODULE(opaque_types, m) { // test_string_list diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index 336f5e45c..a69ba153e 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -207,7 +207,9 @@ TEST_SUBMODULE(virtual_functions, m) { void f() override { py::print("PyA.f()"); - PYBIND11_OVERLOAD(void, A, f); + // This convolution just gives a `void`, but tests that PYBIND11_TYPE() works to protect + // a type containing a , + PYBIND11_OVERLOAD(PYBIND11_TYPE(typename std::enable_if::type), A, f); } };