From dda59d14451d2adef24fe575d5ce6b1b68bc6eeb Mon Sep 17 00:00:00 2001
From: Matt Kuruc <mkuruc@nvidia.com>
Date: Fri, 20 Oct 2023 15:12:00 -0700
Subject: [PATCH] Replace `BOOST_PP_ITERATE` with templates in
 `makePyConstructor.h`

---
 pxr/base/tf/makePyConstructor.h     | 196 ++++++++++++++--------------
 pxr/base/tf/testenv/testTfPython.py |   2 +-
 2 files changed, 98 insertions(+), 100 deletions(-)

diff --git a/pxr/base/tf/makePyConstructor.h b/pxr/base/tf/makePyConstructor.h
index 38d5caff51..1af0ca9b5d 100644
--- a/pxr/base/tf/makePyConstructor.h
+++ b/pxr/base/tf/makePyConstructor.h
@@ -22,8 +22,6 @@
 // language governing permissions and limitations under the Apache License.
 //
 
-#if !BOOST_PP_IS_ITERATING
-
 #ifndef PXR_BASE_TF_MAKE_PY_CONSTRUCTOR_H
 #define PXR_BASE_TF_MAKE_PY_CONSTRUCTOR_H
 
@@ -38,6 +36,7 @@
 
 #include "pxr/pxr.h"
 #include "pxr/base/tf/api.h"
+#include "pxr/base/tf/functionTraits.h"
 #include "pxr/base/tf/refPtr.h"
 #include "pxr/base/tf/weakPtr.h"
 #include "pxr/base/tf/diagnostic.h"
@@ -48,15 +47,6 @@
 
 #include "pxr/base/arch/demangle.h"
 
-#include <boost/preprocessor/iterate.hpp>
-#include <boost/preprocessor/punctuation/comma_if.hpp>
-#include <boost/preprocessor/repetition/enum.hpp>
-#include <boost/preprocessor/repetition/enum_binary_params.hpp>
-#include <boost/preprocessor/repetition/enum_params.hpp>
-#include <boost/preprocessor/repetition/enum_trailing_binary_params.hpp>
-#include <boost/preprocessor/repetition/enum_trailing_params.hpp>
-#include <boost/preprocessor/repetition/repeat.hpp>
-#include <boost/preprocessor/seq/for_each.hpp>
 #include <boost/python/def_visitor.hpp>
 #include <boost/python/dict.hpp>
 #include <boost/python/errors.hpp>
@@ -66,7 +56,10 @@
 #include <boost/python/tuple.hpp>
 #include <boost/python/type_id.hpp>
 
+#include <array>
 #include <string>
+#include <type_traits>
+#include <utility>
 
 PXR_NAMESPACE_OPEN_SCOPE
 
@@ -299,9 +292,17 @@ struct RefPtrFactory {
     };
 };
 
-template <typename SIG>
+// EXTRA_ARITY is added for InitCtorWithVarArgs backwards compatability.
+// The previous BOOST_PP implementation didn't count the tuple and dict
+// against the arity limit while the new version does. A future change
+// should remove EXTRA_ARITY and increase TF_MAX_ARITY now that the
+// implementations are templated and no longer generated by BOOST_PP
+template <typename SIG, size_t EXTRA_ARITY = 0>
 struct CtorBase {
     typedef SIG Sig;
+    using Traits = TfFunctionTraits<SIG*>;
+    static_assert(Traits::Arity <= (TF_MAX_ARITY + EXTRA_ARITY));
+
     static Sig *_func;
     static void SetFunc(Sig *func) {
         if (!_func)
@@ -316,21 +317,14 @@ struct CtorBase {
     }
 };
 
-template <typename SIG> SIG *CtorBase<SIG>::_func = 0;
+template <typename SIG, size_t EXTRA_ARITY>
+SIG *CtorBase<SIG, EXTRA_ARITY>::_func = nullptr;
 
-// The following preprocessor code repeatedly includes this file to generate
-// specializations of Ctor taking 0 through TF_MAX_ARITY parameters.
 template <typename SIG> struct InitCtor;
 template <typename SIG> struct InitCtorWithBackReference;
 template <typename SIG> struct InitCtorWithVarArgs;
 template <typename SIG> struct NewCtor;
 template <typename SIG> struct NewCtorWithClassReference;
-#define BOOST_PP_ITERATION_LIMITS (0, TF_MAX_ARITY)
-#define BOOST_PP_FILENAME_1 "pxr/base/tf/makePyConstructor.h"
-#include BOOST_PP_ITERATE()
-/* comment needed for scons dependency scanner
-#include "pxr/base/tf/makePyConstructor.h"
-*/
 
 }
 
@@ -427,27 +421,14 @@ struct Tf_PySequenceToListConverterRefPtrFactory {
     }
 };
 
-PXR_NAMESPACE_CLOSE_SCOPE
-
-#endif // PXR_BASE_TF_MAKE_PY_CONSTRUCTOR_H
-
-#else // BOOST_PP_IS_ITERATING
-
-#define N BOOST_PP_ITERATION()
-
-#define SIGNATURE R (BOOST_PP_ENUM_PARAMS(N, A))
-#define PARAMLIST BOOST_PP_ENUM_TRAILING_BINARY_PARAMS(N, A, a)
-#define ARGLIST BOOST_PP_ENUM_PARAMS(N, a)
-
-// This generates multi-argument specializations for Tf_MakePyConstructor::Ctor.
-// One nice thing about this style of PP repetition is that the debugger will
-// actually step you over these lines for any instantiation of Ctor.
+namespace Tf_MakePyConstructor {
 
-template <typename R BOOST_PP_ENUM_TRAILING_PARAMS(N, typename A)>
-struct InitCtor<SIGNATURE> : CtorBase<SIGNATURE> {
-    typedef CtorBase<SIGNATURE> Base;
+template <typename R, typename... Args>
+struct InitCtor<R(Args...)> : CtorBase<R(Args...)> {
+    typedef CtorBase<R(Args...)> Base;
     typedef typename Base::Sig Sig;
-    InitCtor(Sig *func) { Base::SetFunc(func); }
+
+    InitCtor(Sig* func) { Base::SetFunc(func); }
 
     template <typename CLS>
     static bp::object init_callable() {
@@ -460,23 +441,23 @@ struct InitCtor<SIGNATURE> : CtorBase<SIGNATURE> {
     }
 
     template <typename CLS>
-    static void __init__(object &self PARAMLIST) {
+    static void __init__(object &self, Args... args) {
         TfErrorMark m;
-        Install<CLS>(self, Base::_func(ARGLIST), m);
+        Install<CLS>(self, Base::_func(args...), m);
     }
 };
 
-template <typename R BOOST_PP_ENUM_TRAILING_PARAMS(N, typename A)>
-struct NewCtor<SIGNATURE> : CtorBase<SIGNATURE> {
-    typedef CtorBase<SIGNATURE> Base;
+template <typename R, typename... Args>
+struct NewCtor<R(Args...)> : CtorBase<R(Args...)> {
+    typedef CtorBase<R(Args...)> Base;
     typedef typename Base::Sig Sig;
     NewCtor(Sig *func) { Base::SetFunc(func); }
 
     template <class CLS>
-    static bp::object __new__(object &cls PARAMLIST) {
+    static bp::object __new__(object &cls, Args... args) {
         typedef typename CLS::metadata::held_type HeldType;
         TfErrorMark m;
-        R r((Base::_func(ARGLIST)));
+        R r((Base::_func(args...)));
         HeldType h((r));
         if (TfPyConvertTfErrorsToPythonException(m))
             bp::throw_error_already_set();
@@ -494,24 +475,30 @@ struct NewCtor<SIGNATURE> : CtorBase<SIGNATURE> {
     }
 };
 
-#define VAR_SIGNATURE                                   \
-    R (BOOST_PP_ENUM_PARAMS(N, A) BOOST_PP_COMMA_IF(N)  \
-       const bp::tuple&, const bp::dict&)
-
-#define FORMAT_STR(z, n, data) "%s, "
-#define ARG_TYPE_STR_A(z, n, data) bp::type_id<A##n>().name()
+template <typename R, typename... Args>
+struct InitCtorWithVarArgs<R(Args...)> :
+    // Pad the arity for backwards compatability
+    CtorBase<R(Args...), /*EXTRA_ARITY*/ 2> {
+    typedef CtorBase<R(Args...), /*EXTRA_ARITY*/ 2> Base;
+    typedef typename Base::Sig Sig;
 
-#define EXTRACT_REQ_ARG_A(z, n, data)                                     \
-    /* The n'th required arg is stored at n + 1 in the positional args */ \
-    /* tuple as the 0'th element is always the self object */             \
-    bp::extract<typename std::remove_reference_t<A##n>>(data[n + 1])
+    // Ideally, Arity would be pulled from Base::Traits, but
+    // compilers have inconsistently allowed this. Redefine
+    // Arity as a workaround for now.
+    using Arity = TfMetaLength<Args...>;
 
-template <typename R BOOST_PP_ENUM_TRAILING_PARAMS(N, typename A)>
-struct InitCtorWithVarArgs<VAR_SIGNATURE> : CtorBase<VAR_SIGNATURE> {
-    typedef CtorBase<VAR_SIGNATURE> Base;
-    typedef typename Base::Sig Sig;
     InitCtorWithVarArgs(Sig *func) { Base::SetFunc(func); }
 
+    static_assert((Arity::value >= 2) &&
+                  (std::is_same_v<
+                    const bp::tuple&,
+                    typename Base::Traits::template NthArg<(Arity::value-2)>>) &&
+                  (std::is_same_v<
+                    const bp::dict&,
+                    typename Base::Traits::template NthArg<(Arity::value-1)>>),
+                  "InitCtorWithVarArgs requires a function of form "
+                  "(..., const tuple&, const dict&)");
+
     template <typename CLS>
     static bp::object init_callable() {
         // Specify min_args as 1 to account for just the 'self' argument.
@@ -529,19 +516,35 @@ struct InitCtorWithVarArgs<VAR_SIGNATURE> : CtorBase<VAR_SIGNATURE> {
             /* min_args = */ 1);
     }
 
-    template <typename CLS>
-    static bp::object __init__(const bp::tuple& args, const bp::dict& kwargs) {
+    template <typename CLS, size_t... I>
+    static bp::object __init__impl(const bp::tuple& args,
+                                   const bp::dict& kwargs,
+                                   std::index_sequence<I...>) {
         TfErrorMark m;
 
-        const unsigned int numArgs = bp::len(args);
-        if (numArgs - 1 < N) {
+        // We know that there are at least two args because the specialization only
+        // matches against (..., *args, **kwargs)
+        const unsigned int expectedNamedArgs = Arity::value - 2;
+        // self is included in the tuple, so it should always be at least 1
+        const unsigned int positionalArgs = bp::len(args) - 1;
+        if (positionalArgs < expectedNamedArgs) {
+            std::array<std::string, Arity::value - 2>
+                positionalArgTypes = {{
+                 (bp::type_id<typename Base::Traits::template NthArg<I>>().name())...
+            }};
+            std::string joinedTypes = TfStringJoin(
+                std::begin(positionalArgTypes),
+                std::end(positionalArgTypes), ", "
+            );
+            if (!joinedTypes.empty()) {
+                joinedTypes += ", ";
+            }
             // User didn't provide enough positional arguments for the factory
             // function. Complain.
             TfPyThrowTypeError(
                 TfStringPrintf(
                     "Arguments to __init__ did not match C++ signature:\n"
-                    "\t__init__(" BOOST_PP_REPEAT(N, FORMAT_STR, 0) "...)"
-                    BOOST_PP_COMMA_IF(N) BOOST_PP_ENUM(N, ARG_TYPE_STR_A, 0)
+                    "\t__init__(%s...)", joinedTypes.c_str()
                 )
             );
             return bp::object();
@@ -550,32 +553,34 @@ struct InitCtorWithVarArgs<VAR_SIGNATURE> : CtorBase<VAR_SIGNATURE> {
         Install<CLS>(
             // self object for new instance is the first arg to __init__
             args[0],
-
-            // Slice the first N arguments from positional arguments as
-            // those are the required arguments for the factory function.
             Base::_func(
-                BOOST_PP_ENUM(N, EXTRACT_REQ_ARG_A, args) BOOST_PP_COMMA_IF(N)
-                bp::tuple(args.slice(N + 1, numArgs)), kwargs),
+                bp::extract<
+                    std::remove_reference_t<
+                        typename Base::Traits::template NthArg<I>>>(args[I + 1])...,
+                bp::tuple(args.slice(expectedNamedArgs + 1, bp::len(args))), kwargs),
             m);
 
         return bp::object();
     }
 
-};
+    template <typename CLS>
+    static bp::object __init__(const bp::tuple& args,
+                               const bp::dict& kwargs) {
+        return __init__impl<CLS>(
+            args, kwargs, std::make_index_sequence<Arity::value - 2>());
 
-#if N > 0
-#undef PARAMLIST
-#define PARAMLIST BOOST_PP_ENUM_BINARY_PARAMS(N, A, a)
+    }
+};
 
 // This is a variant of Ctor which includes a back reference to self
 // (the Python object being initialized) in the args passed to the
 // constructor.  This is used to expose the factory methods for classes
 // which we expect to subclass in Python.  When the constructor is called,
 // it can examine self and initialize itself appropriately.
-
-template <typename R BOOST_PP_ENUM_TRAILING_PARAMS(N, typename A)>
-struct InitCtorWithBackReference<SIGNATURE> : CtorBase<SIGNATURE> {
-    typedef CtorBase<SIGNATURE> Base;
+template <typename R, typename SelfRef, typename... Args>
+struct InitCtorWithBackReference<R(SelfRef, Args...)> :
+    CtorBase<R(SelfRef, Args...)> {
+    typedef CtorBase<R(SelfRef, Args...)> Base;
     typedef typename Base::Sig Sig;
     InitCtorWithBackReference(Sig *func) { Base::SetFunc(func); }
 
@@ -590,23 +595,24 @@ struct InitCtorWithBackReference<SIGNATURE> : CtorBase<SIGNATURE> {
     }
 
     template <typename CLS>
-    static void __init__(PARAMLIST) {
+    static void __init__(SelfRef self, Args... args) {
         TfErrorMark m;
-        Install<CLS>(a0, Base::_func(ARGLIST), m);
+        Install<CLS>(self, Base::_func(self, args...), m);
     }
 };
 
-template <typename R BOOST_PP_ENUM_TRAILING_PARAMS(N, typename A)>
-struct NewCtorWithClassReference<SIGNATURE> : CtorBase<SIGNATURE> {
-    typedef CtorBase<SIGNATURE> Base;
+template <typename R, typename ClsRef, typename... Args>
+struct NewCtorWithClassReference<R(ClsRef, Args...)> :
+    CtorBase<R(ClsRef, Args...)> {
+    typedef CtorBase<R(ClsRef, Args...)> Base;
     typedef typename Base::Sig Sig;
     NewCtorWithClassReference(Sig *func) { Base::SetFunc(func); }
 
     template <class CLS>
-    static bp::object __new__(PARAMLIST) {
+    static bp::object __new__(ClsRef cls, Args... args) {
         typedef typename CLS::metadata::held_type HeldType;
         TfErrorMark m;
-        R r(Base::_func(ARGLIST));
+        R r(Base::_func(cls, args...));
         HeldType h(r);
         if (TfPyConvertTfErrorsToPythonException(m))
             bp::throw_error_already_set();
@@ -617,22 +623,14 @@ struct NewCtorWithClassReference<SIGNATURE> : CtorBase<SIGNATURE> {
 
         bp::detail::initialize_wrapper(ret.ptr(), get_pointer(h));
         // make the object have the right class.
-        bp::setattr(ret, "__class__", a0);
+        bp::setattr(ret, "__class__", cls);
 
         InstallPolicy<R>::PostInstall(ret, r, h.GetUniqueIdentifier());
         return ret;
     }
 };
+}
 
-#endif
-
-#undef N
-#undef SIGNATURE
-#undef PARAMLIST
-#undef ARGLIST
-#undef VAR_SIGNATURE
-#undef FORMAT_STR
-#undef ARG_TYPE_STR_A
-#undef EXTRACT_REQ_ARG_A
+PXR_NAMESPACE_CLOSE_SCOPE
 
-#endif // BOOST_PP_IS_ITERATING
+#endif // PXR_BASE_TF_MAKE_PY_CONSTRUCTOR_H
diff --git a/pxr/base/tf/testenv/testTfPython.py b/pxr/base/tf/testenv/testTfPython.py
index 1b22af0cb7..3074857d12 100644
--- a/pxr/base/tf/testenv/testTfPython.py
+++ b/pxr/base/tf/testenv/testTfPython.py
@@ -487,7 +487,7 @@ def test_TfPyObjWrapper(self):
         self.assertEqual(4, Tf._RoundTripWrapperIndexTest([1,2,3,4], -1))
 
     def test_TfMakePyConstructorWithVarArgs(self):
-        with self.assertRaises(TypeError):
+        with self.assertRaisesRegex(TypeError, "__init__\(bool, \.\.\.\)"):
             Tf._ClassWithVarArgInit()
 
         def CheckResults(c, allowExtraArgs, args, kwargs):