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

@@ -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

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
+:func:`PYBIND11_DECLARE_HOLDER_TYPE` and ``PYBIND11_OVERLOAD_*``. Since these
-``PYBIND11_OVERLOAD_*``. Since these are "just" macros that are evaluated
+are "just" macros that are evaluated in the preprocessor (which has no concept
-in the preprocessor (which has no concept of types), they *will* get confused
+of types), they *will* get confused by commas in a template argument; for
-by commas in a template argument such as ``PYBIND11_OVERLOAD(MyReturnValue<T1,
+example, consider:
-T2>, 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
+.. code-block:: cpp
-another name and use it in the macro to avoid this problem.
+
+    PYBIND11_OVERLOAD(MyReturnType<T1, T2>, Class<T3, T4>, 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<T1, T2>;
+    using ClassType = Class<T3, T4>;
+    PYBIND11_OVERLOAD(ReturnType, ClassType, func);
+
+    // Version 2: using the PYBIND11_TYPE macro:
+    PYBIND11_OVERLOAD(PYBIND11_TYPE(MyReturnType<T1, T2>),
+                      PYBIND11_TYPE(Class<T3, T4>), func)
+
+The ``PYBIND11_MAKE_OPAQUE`` macro does *not* require the above workarounds.
 
 .. _gil:
 

include/pybind11/cast.h

@@ -2054,9 +2054,13 @@ object object_api<Derived>::call(Args &&...args) const {
 
 NAMESPACE_END(detail)
 
-#define PYBIND11_MAKE_OPAQUE(Type) \
+#define PYBIND11_MAKE_OPAQUE(...) \
     namespace pybind11 { namespace detail { \
-        template<> class type_caster<Type> : public type_caster_base<Type> { }; \
+        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<A, B>), PYBIND11_TYPE(Parent<C, D>), f, arg)`
+#define PYBIND11_TYPE(...) __VA_ARGS__
+
 NAMESPACE_END(PYBIND11_NAMESPACE)

include/pybind11/pybind11.h

@@ -1958,18 +1958,18 @@ template <class T> 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)
 

tests/test_opaque_types.cpp

@@ -11,10 +11,14 @@
 #include <pybind11/stl.h>
 #include <vector>
 
-using StringList = std::vector<std::string>;
+// 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<std::string, std::allocator<std::string>>);
 
-/* IMPORTANT: Disable internal pybind11 translation mechanisms for STL data structures */
+using StringList = std::vector<std::string, std::allocator<std::string>>;
-PYBIND11_MAKE_OPAQUE(StringList);
 
 TEST_SUBMODULE(opaque_types, m) {
     // test_string_list

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<true, void>::type), A, f);
         }
     };