Repository: RosettaCommons/binder (language: python)

Issue:
Multiple questions associated to usage of binder with a large heavily templated project
First of all, thanks for developing this tool; I have been able to successfully apply it in the past week on some projects.

I have couple of questions associated to its application on the project Trilinos ( https://github.com/trilinos/trilinos ), a large C++ project which relies heavily on some template parameters.

Quick context on project: 
- it is composed of several subprojects, called packages, which have some dependencies between them,
- each package is associated to a different team,
- all the packages rely heavily on template.

The installation of the library is as follows:
- in the source directory there are some .hpp and .cpp files, CMake, based on the options of the ETI, generates some .cpp files that are created in the build directory. Some of the files of the source directory are not copied to the build directory.
- the library is built using the CMake generated .cpp files and .hpp files and .cpp files written by the developers.
- the .hpp files and .so libraries are copied to the install directory.

The goal to use binder is to have an automatic way of creating and maintaining  a python interface for the project without requiring the developers of the different packages to maintain the interface themself.

I have the following questions:
1. In the config.cfg file it is possible disable one class or one function one by one; is it possible to disable every classes and functions and then enable some of them one at a time? If so, how can I do that?
2. For our templated functions/classes, we use some ETI strategies to build and link the library. Typically, we rely on CMake to generate some .cpp files in the build directory which are then compiled and linked to the .so lib. How can binder be aware of those ETI? How can we instantiate multiple times the same class with different template values and generate different Python variable names?
3. Currently, we use macros for the ETI such as:
```
#  define TPETRA_MULTIVECTOR_INSTANT(SCALAR,LO,GO,NODE) \
  template class MultiVector< SCALAR , LO , GO , NODE >; \
  template MultiVector< SCALAR , LO , GO , NODE > createCopy( const MultiVector< SCALAR , LO , GO , NODE >& src);
```
that we use as follows in a cpp file generated by CMake:
```
TPETRA_DETAILS_MAKECOLMAP_INSTANT( int, longlong, Kokkos_Compat_KokkosSerialWrapperNode )
```
Can binder work around the macros in this context and understand the fact that the .cpp files do the ETI?

4. In the source codes, we have some classes, functions, and includes that are guarded by some preprocessing checks.
For example, if some TPL is included some classes can be declared and defined, otherwise it is not the case.
When calling binder, is it possible to process those `#ifdef` checks and drop the content of the .hpp file between the begin and the end of the check if not defined?
5. How can we make the binding process iterative by enabling one package at a time using binder instead of using all the files at once and still have some dependencies between packages?
6. In Trilinos, we use a class of the project as an implementation of the shared pointer. I successfully used this class using `PYBIND11_DECLARE_HOLDER_TYPE` with PyBind11 on a smaller project. Is there a way to specify an extract shared pointer class to binder?

Thanks a lot for your time and help!


Discussion / hints:
> 1. In the config.cfg file it is possible disable one class or one function one by one; is it possible to disable every classes and functions and then enable some of them one at a time? If so, how can I do that?

-- yes, this should be possible. If such fine-grained control is required then i guess best approach would be to explicitly disable namespace bindings for target namespaces (say `-namespace A`) and then enable bindings for specific classes/functions in target namespaces with `+class A::MyClass` or `+function A::my_function`. (Depending on how much control you need it might worth investigate both approaches: [a] explicitly disabling bindings for project namesaces and [b] not-requesting bindings for project namespaces). Please see detail info on options here: https://cppbinder.readthedocs.io/en/latest/config.html#config-file-options


> 2. For our templated functions/classes, we use some ETI strategies to build and link the library. Typically, we rely on CMake to generate some .cpp files in the build directory which are then compiled and linked to the .so lib. How can binder be aware of those ETI? 

-- Binder will only be able bind templates that have been _fully instantiated_ (otherwise LLVM it simply will be lacking info to generate bindings). Please note that usually explicit instantiation of template (say `template class MyClass<int>;`) _will not be enough_ and instead something like inline-function-that-will-take-your-class-by-value will be needed (`inline F(MyClass<int>) {};`). Such explicit instantiation will need to be present in `all_include.hpp` file(s) that Binder takes as it main argument. Depending on the structure of your generated .cpp files it might worth trying to simply include generated .cpp files directly from your project `all_include.hpp` file. For my project what i do is the following: when such special template instantiation is needed i added them to main project include files with `#ifdef MyPythonBindings` guards so it does not pollute main .cpp builds. As an idea: since you already generate .cpp files with template instantiation then maybe it will also be possible for CMake to generate set of .hpp files with inline function definitions outlined above and include such .hpp files in Binder input?

> How can we instantiate multiple times the same class with different template values and generate different Python variable names?

-- when you instantiate C++ template class with particular set of template parameters Binder will generate unique name for such class. For example `std::binary_function<float,float,bool>` will have name like `binary_function_float_float_bool_t` while `std::binary_function<string,float,bool>` will be bound as `binary_function_string_float_bool_t`, - does this answer your question?

> 3. Currently, we use macros for the ETI such as:
> ```
> #  define TPETRA_MULTIVECTOR_INSTANT(SCALAR,LO,GO,NODE) \
>   template class MultiVector< SCALAR , LO , GO , NODE >; \
>   template MultiVector< SCALAR , LO , GO , NODE > createCopy( const MultiVector< SCALAR , LO , GO , NODE >& src);
> ```
> ...
> Can binder work around the macros in this context and understand the fact that the .cpp files do the ETI?

-- yes, Binder input (say all-includes.hpp) is treated as regular C++ include file so all rules of processing C++ macros is applied and followed. As i mentioned above: you will probably add inline function definition in such macros to make sure that your templates is fully instantiated. (see answer above for details).


> 4. In the source codes, we have some classes, functions, and includes that are guarded by some preprocessing checks.
>    For example, if some TPL is included some classes can be declared and defined, otherwise it is not the case.
>    When calling binder, is it possible to process those `#ifdef` checks and drop the content of the .hpp file between the begin and the end of the check if not defined?

-- absolutely, all Binder input processed as standard C++ include files including C++ macros.

> 5. How can we make the binding process iterative by enabling one package at a time using binder instead of using all the files at once and still have some dependencies between packages?

-- doing this "perfectly" will be tricky since you will need to keep track of what types each project bind. I do not have a good solution how to do so (for my main project (3M LOC) i decided to just bind it all at once, - it is a huge library though). But if your Python users do not mind reasonable amount of memory overhead (and if most of the time users will only be importing _some_ projects) then easiest way might be to just generate bindings for each project independently. Importing such projects at-once will lead to some amount of memory overhead since we will have binding code duplication for all overlapped types. But if this if this acceptable then it might be the way to go. Before deciding on this i would recommend to double check with minimal example (or maybe contacting Pybind11 team) that bindings same type twice will be treated as "same type" on Python level (it should, assuming that the same compiler and Pybind11 version is used).

> 6. In Trilinos, we use a class of the project as an implementation of the shared pointer. I successfully used this class using `PYBIND11_DECLARE_HOLDER_TYPE` with PyBind11 on a smaller project. Is there a way to specify an extract shared pointer class to binder?

-- right now this is not supported and `std::shared_ptr` is hardcoded into Binder code. It should be trivial to add config option to make this configurable and PR that add such functionality will be welcome!

Hope this helps,


Thanks a @lyskov for your answers!

I have been able to make some progress using your comments.
Now, I will take a look on how to add a config option to make the shared pointer type configurable.

Fix patch:
diff --git a/documentation/config.rst b/documentation/config.rst
index 02a28cd0..6105e7d8 100644
--- a/documentation/config.rst
+++ b/documentation/config.rst
@@ -186,6 +186,7 @@ Config file directives:
 * ``default_call_guard``, optionally specify a call guard applied to all function definitions. See `pybind11 documentation <http://pybind11.readthedocs.io/en/stable/advanced/functions.html#call-guard>`_. Default None.
 
 
+* ``+custom_shared``: specify a custom shared pointer class that Binder should use instead of std::shared_ptr.
 
 
 
diff --git a/source/class.cpp b/source/class.cpp
index ebc280aa..4c0aee43 100644
--- a/source/class.cpp
+++ b/source/class.cpp
@@ -868,10 +868,15 @@ string bind_forward_declaration(CXXRecordDecl const *C, Context &context)
 
 	string const include = relevant_include(C);
 
+	string holder_type = Config::get().holder_type();
+
 	string c = "\t// Forward declaration for: " + qualified_name + " file:" + (include.size() ? include.substr(1, include.size() - 2) : "") + " line:" + line_number(C) + "\n";
 
-	string maybe_holder_type = ", std::shared_ptr<{}>"_format(qualified_name);
-	if( is_inherited_from_enable_shared_from_this(C) ) maybe_holder_type = ", std::shared_ptr<{}>"_format(qualified_name);
+	string maybe_holder_type = ", {}<{}>"_format(holder_type, qualified_name);
+	//Check if the type is a custom shared pointer:
+	if( is_inherited_from_enable_shared_from_this(C) ) {
+		maybe_holder_type = ", {}<{}>"_format(holder_type, qualified_name);
+	}
 	else if( CXXDestructorDecl *d = C->getDestructor() ) {
 		if( d->getAccess() != AS_public ) maybe_holder_type = ", " + qualified_name + '*';
 	}
@@ -1172,12 +1177,16 @@ void ClassBinder::bind(Context &context)
 	string const trampoline_name = callback_structure_constructible ? callback_structure_name(C) : "";
 	string const binding_qualified_name = callback_structure_constructible ? callback_structure_name(C) : qualified_name;
 
-	string maybe_holder_type = ", std::shared_ptr<{}>"_format(qualified_name); // for now enable std::shared_ptr by default
-	if( is_inherited_from_enable_shared_from_this(C) ) maybe_holder_type = ", std::shared_ptr<{}>"_format(qualified_name);
+	string holder_type = Config::get().holder_type();
+
+	string maybe_holder_type = ", {}<{}>"_format(holder_type, qualified_name);
+
+	if( is_inherited_from_enable_shared_from_this(C) ) {
+		maybe_holder_type = ", {}<{}>"_format(holder_type, qualified_name);
+	}
 	else if( CXXDestructorDecl *d = C->getDestructor() ) {
 		if( d->getAccess() != AS_public ) maybe_holder_type = ", " + qualified_name + '*';
 	}
-
 	string maybe_trampoline = callback_structure_constructible ? ", " + binding_qualified_name : "";
 
 	if( named_class )
diff --git a/source/config.cpp b/source/config.cpp
index 3a1780d4..9e11c8d6 100644
--- a/source/config.cpp
+++ b/source/config.cpp
@@ -67,6 +67,8 @@ void Config::read(string const &file_name)
 	string const _binder_for_namespace_{"binder_for_namespace"};
 	string const _add_on_binder_for_namespace_{"add_on_binder_for_namespace"};
 
+	string const _custom_shared_{"custom_shared"};
+
 	string const _default_static_pointer_return_value_policy_{"default_static_pointer_return_value_policy"};
 	string const _default_static_lvalue_reference_return_value_policy_{"default_static_lvalue_reference_return_value_policy"};
 	string const _default_static_rvalue_reference_return_value_policy_{"default_static_rvalue_reference_return_value_policy"};
@@ -172,6 +174,7 @@ void Config::read(string const &file_name)
 				add_on_binder_for_namespaces_[binder_function.first] = binder_function.second;
 			}
 		}
+		else if( token == _custom_shared_ ) holder_type_ = name_without_spaces;
 
 		else if( token == _default_static_pointer_return_value_policy_ ) default_static_pointer_return_value_policy_ = name_without_spaces;
 		else if( token == _default_static_lvalue_reference_return_value_policy_ ) default_static_lvalue_reference_return_value_policy_ = name_without_spaces;
diff --git a/source/config.hpp b/source/config.hpp
index c4130b4a..e7e3d963 100644
--- a/source/config.hpp
+++ b/source/config.hpp
@@ -50,6 +50,7 @@ class Config
 	string default_member_lvalue_reference_return_value_policy_ = "pybind11::return_value_policy::automatic";
 	string default_member_rvalue_reference_return_value_policy_ = "pybind11::return_value_policy::automatic";
 	string default_call_guard_ = "";
+	string holder_type_ = "std::shared_ptr";
 
 public:
 	static Config &get();
@@ -79,6 +80,8 @@ class Config
 	string const &default_member_rvalue_reference_return_value_policy() { return default_member_rvalue_reference_return_value_policy_; }
 	string const &default_call_guard() { return default_call_guard_; }
 
+	string const &holder_type() const { return holder_type_; }
+
 	string prefix;
 
 	std::size_t maximum_file_length;
diff --git a/source/context.cpp b/source/context.cpp
index 8169c1a6..ee093461 100644
--- a/source/context.cpp
+++ b/source/context.cpp
@@ -131,9 +131,9 @@ const char *module_header = R"_(
 {}
 #ifndef BINDER_PYBIND11_TYPE_CASTER
 	#define BINDER_PYBIND11_TYPE_CASTER
-	PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr<T>)
+	{}
 	PYBIND11_DECLARE_HOLDER_TYPE(T, T*)
-	PYBIND11_MAKE_OPAQUE(std::shared_ptr<void>)
+	{}
 #endif
 
 )_";
@@ -429,7 +429,12 @@ void Context::generate(Config const &config)
 
 		if( i < binders.size() ) --i;
 
-		code = generate_include_directives(includes) + fmt::format(module_header, config.includes_code()) + prefix_code + "void " + function_name + module_function_suffix + "\n{\n" + code + "}\n";
+		string const holder_type = Config::get().holder_type();
+
+		string shared_declare = "PYBIND11_DECLARE_HOLDER_TYPE(T, "+holder_type+"<T>)";
+		string shared_make_opaque = "PYBIND11_MAKE_OPAQUE("+holder_type+"<void>)";
+
+		code = generate_include_directives(includes) + fmt::format(module_header, config.includes_code(), shared_declare, shared_make_opaque) + prefix_code + "void " + function_name + module_function_suffix + "\n{\n" + code + "}\n";
 
 		if( O_single_file ) root_module_file_handle << "// File: " << file_name << '\n' << code << "\n\n";
 		else update_source_file(config.prefix, file_name, code);
diff --git a/source/type.cpp b/source/type.cpp
index 479a6f73..7d9a9d24 100644
--- a/source/type.cpp
+++ b/source/type.cpp
@@ -659,7 +659,7 @@ bool is_python_builtin(NamedDecl const *C)
 
 		"std::allocator", "std::__allocator_destructor",
 
-		"std::shared_ptr", "std::enable_shared_from_this", "std::__shared_ptr", // "std::weak_ptr",  "std::__weak_ptr"
+		Config::get().holder_type(), "std::shared_ptr", "std::enable_shared_from_this", "std::__shared_ptr", // "std::weak_ptr",  "std::__weak_ptr"
 		"std::unique_ptr",
 		//"std::__1::shared_ptr", "std::__1::weak_ptr", "std::__1::allocator",
 


Test patch:
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index c4bfd718..74e22a66 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -105,6 +105,7 @@ set( binder_tests
          T42.stl.names.multimap
          T42.stl.names.multiset
          T50.namespace_binder
+         T60.custom_shared
          )
 if (pybind11_VERSION VERSION_LESS 2.5.99)
  message(STATUS "pybind11 version ${pybind11_VERSION} is less than 2.5.99. Some tests will be disabled." )
diff --git a/test/T60.custom_shared.config b/test/T60.custom_shared.config
new file mode 100644
index 00000000..d789aaa8
--- /dev/null
+++ b/test/T60.custom_shared.config
@@ -0,0 +1,1 @@
++custom_shared my_shared_ptr
\ No newline at end of file
diff --git a/test/T60.custom_shared.hpp b/test/T60.custom_shared.hpp
new file mode 100644
index 00000000..b1957d70
--- /dev/null
+++ b/test/T60.custom_shared.hpp
@@ -0,0 +1,43 @@
+// -*- mode:c++;tab-width:2;indent-tabs-mode:t;show-trailing-whitespace:t;rm-trailing-spaces:t -*-
+// vi: set ts=2 noet:
+//
+// Copyright (c) 2016 Sergey Lyskov <sergey.lyskov@jhu.edu>
+//
+// All rights reserved. Use of this source code is governed by a
+// MIT license that can be found in the LICENSE file.
+
+/// @file   binder/test/T01.enum.hpp
+/// @brief  Binder self-test file. Bindings of enum's functionality.
+/// @author Sergey Lyskov
+
+#ifndef _INCLUDED_T60_hpp_
+#define _INCLUDED_T60_hpp_
+
+#include <memory>
+
+template<typename T>
+using my_shared_ptr = std::shared_ptr<T>;
+
+enum E1 { E1_V0, E1_V1 };
+
+enum struct E2_struct { V0, V1 };
+enum class E3_class { V0, V1 };
+
+
+class A
+{
+public:
+	enum AE1 { AE1_V0, AE1_V1 };
+	enum struct AE2_struct { AE3_V0, AE3_V1 };
+	enum class AE3_class { AE2_V0, AE2_V1 };
+
+protected:
+	enum AE3_not_binded { AE3_V0_not_binded, AE3_V1_not_binded };
+	enum class AE4_not_binded { AE4_V0_not_binded, AE4_V1_not_binded };
+
+private:
+	enum AE5_not_binded { AE5_V0_not_binded, AE5_V1_not_binded };
+	enum class AE6_not_binded { AE6_V0_not_binded, AE6_V1_not_binded };
+};
+
+#endif // _INCLUDED_T01_enum_hpp_
diff --git a/test/T60.custom_shared.ref b/test/T60.custom_shared.ref
new file mode 100644
index 00000000..6c5c64d6
--- /dev/null
+++ b/test/T60.custom_shared.ref
@@ -0,0 +1,118 @@
+// File: T60_custom_shared.cpp
+#include <T60.custom_shared.hpp> // A
+#include <T60.custom_shared.hpp> // E1
+#include <T60.custom_shared.hpp> // E2_struct
+#include <T60.custom_shared.hpp> // E3_class
+#include <sstream> // __str__
+
+#include <functional>
+#include <pybind11/pybind11.h>
+#include <string>
+
+#ifndef BINDER_PYBIND11_TYPE_CASTER
+	#define BINDER_PYBIND11_TYPE_CASTER
+	PYBIND11_DECLARE_HOLDER_TYPE(T, my_shared_ptr<T>)
+	PYBIND11_DECLARE_HOLDER_TYPE(T, T*)
+	PYBIND11_MAKE_OPAQUE(my_shared_ptr<void>)
+#endif
+
+void bind_T60_custom_shared(std::function< pybind11::module &(std::string const &namespace_) > &M)
+{
+	// E1 file:T60.custom_shared.hpp line:21
+	pybind11::enum_<E1>(M(""), "E1", pybind11::arithmetic(), "")
+		.value("E1_V0", E1_V0)
+		.value("E1_V1", E1_V1)
+		.export_values();
+
+;
+
+	// E2_struct file:T60.custom_shared.hpp line:23
+	pybind11::enum_<E2_struct>(M(""), "E2_struct", "")
+		.value("V0", E2_struct::V0)
+		.value("V1", E2_struct::V1)
+		.export_values();
+
+;
+
+	// E3_class file:T60.custom_shared.hpp line:24
+	pybind11::enum_<E3_class>(M(""), "E3_class", "")
+		.value("V0", E3_class::V0)
+		.value("V1", E3_class::V1);
+
+;
+
+	{ // A file:T60.custom_shared.hpp line:27
+		pybind11::class_<A, my_shared_ptr<A>> cl(M(""), "A", "");
+		cl.def( pybind11::init( [](){ return new A(); } ) );
+
+		pybind11::enum_<A::AE1>(cl, "AE1", pybind11::arithmetic(), "")
+			.value("AE1_V0", A::AE1_V0)
+			.value("AE1_V1", A::AE1_V1)
+			.export_values();
+
+
+		pybind11::enum_<A::AE2_struct>(cl, "AE2_struct", "")
+			.value("AE3_V0", A::AE2_struct::AE3_V0)
+			.value("AE3_V1", A::AE2_struct::AE3_V1)
+			.export_values();
+
+
+		pybind11::enum_<A::AE3_class>(cl, "AE3_class", "")
+			.value("AE2_V0", A::AE3_class::AE2_V0)
+			.value("AE2_V1", A::AE3_class::AE2_V1);
+
+	}
+}
+
+
+#include <map>
+#include <algorithm>
+#include <functional>
+#include <memory>
+#include <stdexcept>
+#include <string>
+
+#include <pybind11/pybind11.h>
+
+typedef std::function< pybind11::module & (std::string const &) > ModuleGetter;
+
+void bind_T60_custom_shared(std::function< pybind11::module &(std::string const &namespace_) > &M);
+
+
+PYBIND11_MODULE(T60_custom_shared, root_module) {
+	root_module.doc() = "T60_custom_shared module";
+
+	std::map <std::string, pybind11::module> modules;
+	ModuleGetter M = [&](std::string const &namespace_) -> pybind11::module & {
+		auto it = modules.find(namespace_);
+		if( it == modules.end() ) throw std::runtime_error("Attempt to access pybind11::module for namespace " + namespace_ + " before it was created!!!");
+		return it->second;
+	};
+
+	modules[""] = root_module;
+
+	static std::vector<std::string> const reserved_python_words {"nonlocal", "global", };
+
+	auto mangle_namespace_name(
+		[](std::string const &ns) -> std::string {
+			if ( std::find(reserved_python_words.begin(), reserved_python_words.end(), ns) == reserved_python_words.end() ) return ns;
+			else return ns+'_';
+		}
+	);
+
+	std::vector< std::pair<std::string, std::string> > sub_modules {
+	};
+	for(auto &p : sub_modules ) modules[p.first.size() ? p.first+"::"+p.second : p.second] = modules[p.first].def_submodule( mangle_namespace_name(p.second).c_str(), ("Bindings for " + p.first + "::" + p.second + " namespace").c_str() );
+
+	//pybind11::class_<std::shared_ptr<void>>(M(""), "_encapsulated_data_");
+
+	bind_T60_custom_shared(M);
+
+}
+
+// Source list file: TEST/T60_custom_shared.sources
+// T60_custom_shared.cpp
+// T60_custom_shared.cpp
+
+// Modules list file: TEST/T60_custom_shared.modules
+//