From 1b21bd391338a75c8715e54944f3983704d2b792 Mon Sep 17 00:00:00 2001 From: Lucas Czech Date: Sun, 27 Jul 2025 02:18:26 +0200 Subject: [PATCH 1/5] Run clang-format on all source files --- source/binder.cpp | 10 ++-- source/binder.hpp | 24 +++++--- source/class.cpp | 121 ++++++++++++++++++----------------------- source/config.cpp | 45 +++++++-------- source/config.hpp | 8 ++- source/context.cpp | 7 ++- source/enum.cpp | 13 ++--- source/function.cpp | 50 +++++++++-------- source/main.cpp | 7 ++- source/options.cpp | 2 +- source/stl_binders.hpp | 7 +-- source/type.cpp | 58 +++++++++++++------- source/util.cpp | 16 +++--- 13 files changed, 186 insertions(+), 182 deletions(-) diff --git a/source/binder.cpp b/source/binder.cpp index e409dc2f..841128da 100644 --- a/source/binder.cpp +++ b/source/binder.cpp @@ -27,15 +27,15 @@ namespace binder { bool IncludeSet::add_decl(clang::NamedDecl const *D, int level) { - auto it_inserted = stack_.insert( {D, level} ); - auto & it = it_inserted.first; - auto & inserted = it_inserted.second; - if(inserted) return true; + auto it_inserted = stack_.insert({D, level}); + auto &it = it_inserted.first; + auto &inserted = it_inserted.second; + if( inserted ) return true; else { if( it->second <= level ) return false; else { it->second = level; - //it.value() = level; + // it.value() = level; return true; } } diff --git a/source/binder.hpp b/source/binder.hpp index cbd8614a..c65f697a 100644 --- a/source/binder.hpp +++ b/source/binder.hpp @@ -16,9 +16,9 @@ #include #include -#include #include -//#include +#include +// #include #include #include @@ -48,9 +48,9 @@ class IncludeSet private: std::vector includes_; - //using StackType = std::unordered_map; + // using StackType = std::unordered_map; using StackType = llvm::DenseMap; - //using StackType = tsl::robin_map; + // using StackType = tsl::robin_map; StackType stack_; @@ -59,10 +59,18 @@ class IncludeSet enum RequestFlags : int8_t { - none=0, skipping = 1, binding = 2, + none = 0, + skipping = 1, + binding = 2, }; -inline RequestFlags operator|(RequestFlags a, RequestFlags b) { return static_cast(static_cast(a) | static_cast(b)); } -inline RequestFlags operator&(RequestFlags a, RequestFlags b) { return static_cast(static_cast(a) & static_cast(b)); } +inline RequestFlags operator|(RequestFlags a, RequestFlags b) +{ + return static_cast(static_cast(a) | static_cast(b)); +} +inline RequestFlags operator&(RequestFlags a, RequestFlags b) +{ + return static_cast(static_cast(a) & static_cast(b)); +} /// Bindings Generator - represent object that can generate binding info for function, class, enum or data variable class Binder @@ -92,7 +100,7 @@ class Binder /// check if user supplied config requested binding for the given declaration and if so request it - virtual void request_bindings_and_skipping(Config const &, RequestFlags flags = RequestFlags::skipping | RequestFlags::binding) = 0; + virtual void request_bindings_and_skipping(Config const &, RequestFlags flags = RequestFlags::skipping | RequestFlags::binding) = 0; /// extract include needed for this generator and add it to includes vector virtual void add_relevant_includes(IncludeSet &) const = 0; diff --git a/source/class.cpp b/source/class.cpp index 2963d6d7..fb764cb9 100644 --- a/source/class.cpp +++ b/source/class.cpp @@ -20,7 +20,7 @@ #include #include -//#include +// #include using namespace llvm; @@ -53,8 +53,8 @@ string template_specialization(clang::CXXRecordDecl const *C) // outs() << " template argument: " << template_argument_to_string(t->getTemplateArgs()[i]) << "\n"; // templ += template_argument_to_string(t->getTemplateArgs()[i]) + ","; std::string template_arg = template_argument_to_string(t->getTemplateArgs()[i]); - if ((template_arg[0] == '<') and (template_arg[template_arg.size()-1]=='>')) template_arg = template_arg.substr(1, template_arg.size()-2); - if (template_arg.size()>0) templ += template_arg + ","; + if( (template_arg[0] == '<') and (template_arg[template_arg.size() - 1] == '>') ) template_arg = template_arg.substr(1, template_arg.size() - 2); + if( template_arg.size() > 0 ) templ += template_arg + ","; // if( t->getTemplateArgs()[i].ArgKind() == TemplateArgument::ArgKind::Integral ) outs() << " template arg:" << t->getTemplateArgs()[i].<< "\n"; // outs() << expresion_to_string( t->getTemplateArgs()[i].getAsExpr() ) << "\n"; @@ -141,7 +141,7 @@ bool is_bindable(FieldDecl *f) { if( f->getType()->isAnyPointerType() or f->getType()->isReferenceType() or f->getType()->isArrayType() ) return false; - //if( !is_field_assignable(f) ) return false; + // if( !is_field_assignable(f) ) return false; if( f->isAnonymousStructOrUnion() ) return false; @@ -228,7 +228,7 @@ bool is_bindable(clang::CXXRecordDecl const *C) if( it != cache.end() ) return it->second; else { bool r = is_bindable_raw(C); - cache.insert( {C, r} ); + cache.insert({C, r}); return r; } @@ -441,7 +441,7 @@ void add_relevant_includes(clang::CXXRecordDecl const *C, IncludeSet &includes, inline void add_includes_to_set(std::vector const &from, IncludeSet &to) { - for(auto const &i : from) to.add_include(i); + for( auto const &i : from ) to.add_include(i); } void add_relevant_includes_cached(clang::CXXRecordDecl const *C, IncludeSet &includes) @@ -523,10 +523,10 @@ bool ClassBinder::bindable() const /// check if user requested binding for the given declaration -void ClassBinder::request_bindings_and_skipping(Config const & config, RequestFlags flags) +void ClassBinder::request_bindings_and_skipping(Config const &config, RequestFlags flags) { - if( (flags&RequestFlags::skipping) and is_skipping_requested(C, config) ) Binder::request_skipping(); - else if( (flags&RequestFlags::binding) and is_binding_requested(C, config) ) Binder::request_bindings(); + if( (flags & RequestFlags::skipping) and is_skipping_requested(C, config) ) Binder::request_skipping(); + else if( (flags & RequestFlags::binding) and is_binding_requested(C, config) ) Binder::request_bindings(); } @@ -549,16 +549,14 @@ void ClassBinder::add_relevant_includes(IncludeSet &includes) const includes.add_include(" // __str__"); } -string generate_opaque_declaration_if_needed(string const & qualified_name, string const & qualified_name_without_template) +string generate_opaque_declaration_if_needed(string const &qualified_name, string const &qualified_name_without_template) { // pybind11 container lists https://pybind11.readthedocs.io/en/stable/advanced/cast/stl.html - static vector stl_containers {"std::vector", "std::deque", "std::list", "std::array", "std::valarray", "std::set", "std::unordered_set", "std::map", "std::unordered_map"}; + static vector stl_containers{"std::vector", "std::deque", "std::list", "std::array", "std::valarray", "std::set", "std::unordered_set", "std::map", "std::unordered_map"}; if( begins_with(qualified_name_without_template, "std::") ) { auto it = std::find(stl_containers.begin(), stl_containers.end(), qualified_name_without_template); - if( it != stl_containers.end() ) { - return "PYBIND11_MAKE_OPAQUE(" + qualified_name + ");\n"; - } + if( it != stl_containers.end() ) { return "PYBIND11_MAKE_OPAQUE(" + qualified_name + ");\n"; } } return ""; @@ -576,9 +574,7 @@ string binding_public_data_members(CXXRecordDecl const *C) if( UsingShadowDecl *us = dyn_cast(*s) ) { if( FieldDecl *f = dyn_cast(us->getTargetDecl()) ) { auto config = Config::get(); - if ( config.is_field_skipping_requested(f->getQualifiedNameAsString())) { - continue; - } + if( config.is_field_skipping_requested(f->getQualifiedNameAsString()) ) { continue; } if( is_bindable(f) ) c += "\tcl" + bind_data_member(f, class_qualified_name(C)) + ";\n"; } } @@ -589,11 +585,9 @@ string binding_public_data_members(CXXRecordDecl const *C) for( auto d = C->decls_begin(); d != C->decls_end(); ++d ) { if( FieldDecl *f = dyn_cast(*d) ) { - //outs() << "Class: " << class_qualified_name(C); f->dump(); outs() << "\n"; + // outs() << "Class: " << class_qualified_name(C); f->dump(); outs() << "\n"; auto config = Config::get(); - if ( config.is_field_skipping_requested(f->getQualifiedNameAsString()) ) { - continue; - } + if( config.is_field_skipping_requested(f->getQualifiedNameAsString()) ) { continue; } if( f->getAccess() == AS_public and is_bindable(f) ) c += "\tcl" + bind_data_member(f, class_qualified_name(C)) + ";\n"; } } @@ -604,7 +598,7 @@ string binding_public_data_members(CXXRecordDecl const *C) inline string callback_structure_name(CXXRecordDecl const *C) { string ns = replace_(namespace_from_named_decl(C), "::", "_"); - return mangle_type_name( "PyCallBack_" + (ns.empty() ? "" : ns + '_') + python_class_name(C), false ); + return mangle_type_name("PyCallBack_" + (ns.empty() ? "" : ns + '_') + python_class_name(C), false); } @@ -752,7 +746,7 @@ string bind_member_functions_for_call_back(CXXRecordDecl const *C, string const if( return_type.find(',') != std::string::npos ) { string return_type_alias = "_binder_ret_" + std::to_string(ret_id); ++ret_id; - if (begins_with(return_type,"class ")) return_type = return_type.substr(6); + if( begins_with(return_type, "class ") ) return_type = return_type.substr(6); c += "\tusing {} = {};\n"_format(return_type_alias, return_type); return_type = std::move(return_type_alias); } @@ -770,8 +764,8 @@ string bind_member_functions_for_call_back(CXXRecordDecl const *C, string const c += "\t{} {}({}){} {}override {{"_format(return_type, m->getNameAsString(), std::get<0>(args), m->isConst() ? " const" : "", exception_specification); string member_function_name = namespace_from_named_decl(C); - if ( member_function_name.length() > 0 ) member_function_name += "::"; - member_function_name += C->getNameAsString() + "::" + m->getNameAsString(); + if( member_function_name.length() > 0 ) member_function_name += "::"; + member_function_name += C->getNameAsString() + "::" + m->getNameAsString(); string custom_function_info = Config::get().is_custom_trampoline_function_requested(member_function_name); if( custom_function_info == "" ) { c += indent(fmt::format(call_back_function_body_template, class_name, /*class_qualified_name(C), */ python_name, std::get<1>(args), return_type), "\t\t"); @@ -785,8 +779,7 @@ string bind_member_functions_for_call_back(CXXRecordDecl const *C, string const else { string input_args = std::get<1>(args); c += "\n\t\treturn {}<{},{}>(this, \"{}\", \"{}\""_format(custom_function_info, C->getNameAsString(), callback_structure_name(C), class_name, m->getNameAsString()); - if ( input_args.length() > 0 ) - c += ", {}"_format(std::get<1>(args)); + if( input_args.length() > 0 ) c += ", {}"_format(std::get<1>(args)); c += ");\n"; } c += "\t}\n"; @@ -853,8 +846,8 @@ string binding_public_member_functions(CXXRecordDecl const *C, bool callback_str if( is_bindable(m) and !is_skipping_requested(m, Config::get()) and !isa(m) and !isa(m) and !is_const_overload(m) ) { // Create a new CXXRecordDecl and insert base method into inherited class so bind_function correctly resolve parent namespace for function as 'child::' instead of // 'base::' CXXRecordDecl NC(*C); CXXMethodDecl *nm = CXXMethodDecl::Create(m->getParentASTContext(), &NC, m->getLocStart(), m->getNameInfo(), m->getType(), - // m->getTypeSourceInfo(), m->getStorageClass(), m->isInlineSpecified(), m->isConstexpr() , m->getLocStart()); it looks like LLVM will - // delete this object when parent CXXRecordDecl is destroyed so commenting out for now... // delete nm; + // m->getTypeSourceInfo(), m->getStorageClass(), m->isInlineSpecified(), m->isConstexpr() , m->getLocStart()); it looks like LLVM + // will delete this object when parent CXXRecordDecl is destroyed so commenting out for now... // delete nm; c += bind_function("\tcl", m, context, C, /*always_use_lambda=*/true); } } @@ -933,10 +926,8 @@ string bind_forward_declaration(CXXRecordDecl const *C, Context &context) 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 = ", {}<{}>"_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); - } + // 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 + '*'; } @@ -1036,7 +1027,8 @@ string bind_constructor(ConstructorBindingInfo const &CBI, uint args_to_bind, bo if( O_annotate_functions ) { clang::FunctionDecl const *F = CBI.T; string const include = relevant_include(F); - c += "\t// function-signature: " + function_qualified_name(F, true) + "(" + function_arguments(F) + ") file:" + (include.size() ? include.substr(1, include.size() - 2) : "") + " line:" + line_number(F) + "\n"; + c += "\t// function-signature: " + function_qualified_name(F, true) + "(" + function_arguments(F) + ") file:" + (include.size() ? include.substr(1, include.size() - 2) : "") + + " line:" + line_number(F) + "\n"; } if( args_to_bind == CBI.T->getNumParams() and not CBI.T->isVariadic() ) { @@ -1079,9 +1071,10 @@ string bind_default_constructor(ConstructorBindingInfo const &CBI) // CXXRecordD string code; if( O_annotate_functions ) { clang::FunctionDecl const *F = CBI.T; - if(F) { + if( F ) { string const include = relevant_include(F); - code += "\t// function-signature: " + function_qualified_name(F, true) + "(" + function_arguments(F) + ") file:" + (include.size() ? include.substr(1, include.size() - 2) : "") + " line:" + line_number(F) + "\n"; + code += "\t// function-signature: " + function_qualified_name(F, true) + "(" + function_arguments(F) + ") file:" + (include.size() ? include.substr(1, include.size() - 2) : "") + + " line:" + line_number(F) + "\n"; } else { code += "\t// function-signature: " + CBI.class_qualified_name + "()\n"; @@ -1108,7 +1101,8 @@ string bind_copy_constructor(ConstructorBindingInfo const &CBI) // CXXConstructo if( O_annotate_functions ) { clang::FunctionDecl const *F = CBI.T; string const include = relevant_include(F); - code += "\t// function-signature: " + function_qualified_name(F, true) + "(" + function_arguments(F) + ") file:" + (include.size() ? include.substr(1, include.size() - 2) : "") + " line:" + line_number(F) + "\n"; + code += "\t// function-signature: " + function_qualified_name(F, true) + "(" + function_arguments(F) + ") file:" + (include.size() ? include.substr(1, include.size() - 2) : "") + + " line:" + line_number(F) + "\n"; } // CXXRecordDecl const *C = T->getParent(); @@ -1133,21 +1127,18 @@ string bind_copy_constructor(ConstructorBindingInfo const &CBI) // CXXConstructo string const_bit; CBI.T->isCopyConstructor(typequals); - if( typequals == Qualifiers::TQ::Const ) { - const_bit += " const"; - } + if( typequals == Qualifiers::TQ::Const ) { const_bit += " const"; } if( CBI.trampoline ) { if( CBI.C->isAbstract() ) code += "\tcl.def(pybind11::init<{}{} &>());\n"_format(CBI.trampoline_qualified_name, const_bit); else { // not yet supported by Pybind11? return "\tcl.def( pybind11::init( []({0} const &o){{ return new {0}(o); }}, []({1} const &o){{ return new {1}(o); }} ) // );\n"_format(CBI.class_qualified_name, CBI.binding_qualified_name); - code += \ - "\tcl.def( pybind11::init( []({0}{1} &o){{ return new {0}(o); }} ) );\n"_format(CBI.trampoline_qualified_name, const_bit) + - (CBI.T->getAccess() == AS_public ? "\tcl.def( pybind11::init( []({0}{1} &o){{ return new {0}(o); }} ) );\n"_format(CBI.class_qualified_name, const_bit) : ""); + code += "\tcl.def( pybind11::init( []({0}{1} &o){{ return new {0}(o); }} ) );\n"_format(CBI.trampoline_qualified_name, const_bit) + + (CBI.T->getAccess() == AS_public ? "\tcl.def( pybind11::init( []({0}{1} &o){{ return new {0}(o); }} ) );\n"_format(CBI.class_qualified_name, const_bit) : ""); } } - else code += "\tcl.def( pybind11::init( []({0}{1} &o){{ return new {0}(o); }} ) );\n"_format(CBI.class_qualified_name, const_bit); + else code += "\tcl.def( pybind11::init( []({0}{1} &o){{ return new {0}(o); }} ) );\n"_format(CBI.class_qualified_name, const_bit); return code; } @@ -1173,18 +1164,18 @@ std::string ClassBinder::bind_repr(Context &context, Config const &config) { string c; string qualified_name = class_qualified_name(C); - if( config.is_function_skipping_requested(qualified_name + "::__str__") or config.is_function_skipping_requested( standard_name(C->getQualifiedNameAsString() + "::__str__" ) ) ) return c; + if( config.is_function_skipping_requested(qualified_name + "::__str__") or config.is_function_skipping_requested(standard_name(C->getQualifiedNameAsString() + "::__str__")) ) return c; if( FunctionDecl const *F = context.global_insertion_operator(C) ) { - //outs() << "Found insertion operator for: " << class_qualified_name(C) << "\n"; - //outs() << "insertion operator: " << F->getNameInfo().getAsString() << " qn: " << F->getQualifiedNameAsString() << " dn:" << F->getDeclName() << "\n"; + // outs() << "Found insertion operator for: " << class_qualified_name(C) << "\n"; + // outs() << "insertion operator: " << F->getNameInfo().getAsString() << " qn: " << F->getQualifiedNameAsString() << " dn:" << F->getDeclName() << "\n"; string maybe_using_decl; string ns = namespace_from_named_decl(F); - if(ns.size()) maybe_using_decl = " using namespace {};"_format(ns); + if( ns.size() ) maybe_using_decl = " using namespace {};"_format(ns); - //c += "\n\tcl.def(\"__str__\", []({} const &o) -> std::string {{ std::ostringstream s; {}(s, o); return s.str(); }} );\n"_format(qualified_name, F->getQualifiedNameAsString()); + // c += "\n\tcl.def(\"__str__\", []({} const &o) -> std::string {{ std::ostringstream s; {}(s, o); return s.str(); }} );\n"_format(qualified_name, F->getQualifiedNameAsString()); c += "\n\tcl.def(\"__str__\", []({} const &o) -> std::string {{ std::ostringstream s;{} s << o; return s.str(); }} );\n"_format(qualified_name, maybe_using_decl); prefix_includes_.push_back(F); @@ -1242,7 +1233,7 @@ void ClassBinder::bind(Context &context) string const qualified_name = class_qualified_name(C); string const qualified_name_without_template = standard_name(C->getQualifiedNameAsString()); - //prefix_code_ += generate_opaque_declaration_if_needed(qualified_name, qualified_name_without_template); + // prefix_code_ += generate_opaque_declaration_if_needed(qualified_name, qualified_name_without_template); std::map const &external_binders = Config::get().binders(); if( external_binders.count(qualified_name_without_template) ) { @@ -1281,9 +1272,7 @@ void ClassBinder::bind(Context &context) 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); - } + 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 + '*'; } @@ -1291,25 +1280,19 @@ void ClassBinder::bind(Context &context) // Add module local if requested for the namespace std::string module_local_annotation = ""; - if (Config::get().is_module_local_requested(namespace_from_named_decl(C))) - module_local_annotation = ", pybind11::module_local()"; + if( Config::get().is_module_local_requested(namespace_from_named_decl(C)) ) module_local_annotation = ", pybind11::module_local()"; // Add buffer protocol if requested std::string buffer_protocol_annotation = ""; - if (Config::get().is_buffer_protocol_requested(qualified_name_without_template)) - buffer_protocol_annotation = ", pybind11::buffer_protocol()"; + if( Config::get().is_buffer_protocol_requested(qualified_name_without_template) ) buffer_protocol_annotation = ", pybind11::buffer_protocol()"; std::string extra_annotation = module_local_annotation + buffer_protocol_annotation; if( named_class ) { - if (Config::get().is_smart_holder_requested(qualified_name_without_template)) { - c += '\t' + - R"(PYBIND11_TYPE_CASTER_BASE_HOLDER({} {}))"_format(qualified_name, maybe_holder_type) + - '\n'; - } + if( Config::get().is_smart_holder_requested(qualified_name_without_template) ) { c += '\t' + R"(PYBIND11_TYPE_CASTER_BASE_HOLDER({} {}))"_format(qualified_name, maybe_holder_type) + '\n'; } c += '\t' + R"(pybind11::class_<{}{}{}{}> cl({}, "{}", "{}"{});)"_format(qualified_name, maybe_holder_type, maybe_trampoline, maybe_base_classes(context), module_variable_name, python_class_name(C), - generate_documentation_string_for_declaration(C), extra_annotation) + + generate_documentation_string_for_declaration(C), extra_annotation) + '\n'; } // c += "\tpybind11::handle cl_type = cl;\n\n"; @@ -1325,14 +1308,14 @@ void ClassBinder::bind(Context &context) if( t->getAccess() == AS_public and !t->isMoveConstructor() and is_bindable(*t) and !is_skipping_requested(*t, Config::get()) /*and t->doesThisDeclarationHaveABody()*/ ) { ConstructorBindingInfo CBI = {C, *t, trampoline, qualified_name, trampoline_name, context}; - if( t->isCopyConstructor() /*and not copy_constructor_processed*/ and !is_skipping_requested(*t, Config::get() ) ) { + if( t->isCopyConstructor() /*and not copy_constructor_processed*/ and !is_skipping_requested(*t, Config::get()) ) { // constructors += "\tcl.def(pybind11::init<{} const &>());\n"_format(binding_qualified_name); //(*t) -> dump(); - //constructors += "// CC " + standard_name(t->getQualifiedNameAsString()) + "\n"; - //constructors += "// CC " + function_qualified_name(*t, true) + "\n"; + // constructors += "// CC " + standard_name(t->getQualifiedNameAsString()) + "\n"; + // constructors += "// CC " + function_qualified_name(*t, true) + "\n"; constructors += bind_copy_constructor(CBI); - //constructors += "// CC \n"; - // copy_constructor_processed = true; + // constructors += "// CC \n"; + // copy_constructor_processed = true; } else if( t->isDefaultConstructor() and t->getNumParams() == 0 ) constructors += bind_default_constructor(CBI); // workaround for Pybind11-2.2 issues else constructors += bind_constructor(CBI); @@ -1390,7 +1373,7 @@ void ClassBinder::bind(Context &context) if( EnumDecl *e = dyn_cast(*d) ) { if( e->getAccess() == AS_public and is_bindable(e) ) { if( is_skipping_requested(e, Config::get()) ) { - //outs() << "Skipping inner class Enum: " << e->getQualifiedNameAsString() << "\n"; + // outs() << "Skipping inner class Enum: " << e->getQualifiedNameAsString() << "\n"; } else { c += '\n'; diff --git a/source/config.cpp b/source/config.cpp index 4c70aa9c..c68e4361 100644 --- a/source/config.cpp +++ b/source/config.cpp @@ -18,8 +18,8 @@ #include -#include #include +#include #include using namespace llvm; @@ -102,7 +102,7 @@ void Config::read(string const &file_name) string line; while( std::getline(f, line) ) { - if( line.empty() or line[0] == '#' ) continue; + if( line.empty() or line[0] == '#' ) continue; if( line.back() == '\r' ) { line.pop_back(); @@ -139,7 +139,7 @@ void Config::read(string const &file_name) } else if( token == _python_builtin_ ) { - if (bind) python_builtins.insert(name_without_spaces); + if( bind ) python_builtins.insert(name_without_spaces); else not_python_builtins.insert(name_without_spaces); } else if( token == _function_ ) { @@ -173,14 +173,10 @@ void Config::read(string const &file_name) } } else if( token == _buffer_protocol_ ) { - if(bind) { - buffer_protocols.push_back(name_without_spaces); - } + if( bind ) { buffer_protocols.push_back(name_without_spaces); } } - else if( token == _module_local_namespace_) { - if(bind) { - module_local_namespaces_to_add.push_back(name_without_spaces); - } + else if( token == _module_local_namespace_ ) { + if( bind ) { module_local_namespaces_to_add.push_back(name_without_spaces); } else { module_local_namespaces_to_skip.push_back(name_without_spaces); } @@ -212,18 +208,15 @@ void Config::read(string const &file_name) auto binder_function = split_in_two(name, "Invalid line for add_on_binder_for_namespace specification! Must be: name_of_type + + name_of_binder. Got: " + line); add_on_binder_for_namespaces_[binder_function.first] = trim(binder_function.second); } - } else if ( token == _field_ ) { + } + else if( token == _field_ ) { - if (!bind) { - fields_to_skip.push_back(name_without_spaces); - } + if( !bind ) { fields_to_skip.push_back(name_without_spaces); } } else if( token == _custom_shared_ ) holder_type_ = name_without_spaces; else if( token == _smart_holder_ ) { - if(bind) { - smart_held_classes.push_back(name_without_spaces); - } + if( bind ) { smart_held_classes.push_back(name_without_spaces); } } else if( token == _pybind11_include_file_ ) { @@ -243,7 +236,8 @@ void Config::read(string const &file_name) else if( token == _trampoline_member_function_binder_ ) { if( bind ) { - auto member_function_name_and_function_name = split_in_two(name, "Invalid line for trampoline_member_function_binder specification! Must be: qualified_class_name::member_funtion_name + + name_of_function. Got: " + line); + auto member_function_name_and_function_name = split_in_two( + name, "Invalid line for trampoline_member_function_binder specification! Must be: qualified_class_name::member_funtion_name + + name_of_function. Got: " + line); custom_trampoline_functions_[member_function_name_and_function_name.first] = member_function_name_and_function_name.second; } } @@ -440,17 +434,16 @@ bool Config::is_module_local_requested(string const &namespace_) const auto module_local_all = std::find(module_local_namespaces_to_add.begin(), module_local_namespaces_to_add.end(), namespace_all); if( module_local_all != module_local_namespaces_to_add.end() ) { auto module_local_to_skip = std::find(module_local_namespaces_to_skip.begin(), module_local_namespaces_to_skip.end(), namespace_); - if( module_local_to_skip != module_local_namespaces_to_skip.end()) { - return false; - } + if( module_local_to_skip != module_local_namespaces_to_skip.end() ) { return false; } return true; } auto module_local_to_add = std::find(module_local_namespaces_to_add.begin(), module_local_namespaces_to_add.end(), namespace_); - if( module_local_to_add != module_local_namespaces_to_add.end()) { + if( module_local_to_add != module_local_namespaces_to_add.end() ) { auto module_local_to_skip = std::find(module_local_namespaces_to_skip.begin(), module_local_namespaces_to_skip.end(), namespace_); - if( module_local_to_skip != module_local_namespaces_to_skip.end()) { - throw std::runtime_error("Could not determent if namespace '" + namespace_ + "' should use module_local or not... please resolve the conlficting options +module_local_namespace and -module_local_namespace!!!"); + if( module_local_to_skip != module_local_namespaces_to_skip.end() ) { + throw std::runtime_error("Could not determent if namespace '" + namespace_ + + "' should use module_local or not... please resolve the conlficting options +module_local_namespace and -module_local_namespace!!!"); } return true; } @@ -486,8 +479,8 @@ string Config::includes_code() const { std::ostringstream s; for( auto &i : includes_to_add ) s << "#include " << i << "\n"; - if (O_include_pybind11_stl) s << "#include \n"; - if (s.tellp() != std::streampos(0)) s << '\n'; + if( O_include_pybind11_stl ) s << "#include \n"; + if( s.tellp() != std::streampos(0) ) s << '\n'; return s.str(); } diff --git a/source/config.hpp b/source/config.hpp index 0c6da2f7..692fc872 100644 --- a/source/config.hpp +++ b/source/config.hpp @@ -14,8 +14,8 @@ #ifndef _INCLUDED_config_hpp_ #define _INCLUDED_config_hpp_ -#include #include +#include #include #include @@ -32,8 +32,10 @@ class Config Config() {} - Config(string const &root_module_, std::vector namespaces_to_bind_, std::vector namespaces_to_skip_, string const &prefix_, std::size_t maximum_file_length_, bool skip_line_number_) - : root_module(root_module_), namespaces_to_bind(namespaces_to_bind_), namespaces_to_skip(namespaces_to_skip_), prefix(prefix_), maximum_file_length(maximum_file_length_), skip_line_number(skip_line_number_) + Config(string const &root_module_, std::vector namespaces_to_bind_, std::vector namespaces_to_skip_, string const &prefix_, std::size_t maximum_file_length_, + bool skip_line_number_) + : root_module(root_module_), namespaces_to_bind(namespaces_to_bind_), namespaces_to_skip(namespaces_to_skip_), prefix(prefix_), maximum_file_length(maximum_file_length_), + skip_line_number(skip_line_number_) { } diff --git a/source/context.cpp b/source/context.cpp index 901750a0..dc2517c3 100644 --- a/source/context.cpp +++ b/source/context.cpp @@ -437,11 +437,12 @@ void Context::generate(Config const &config) string const holder_type = Config::get().holder_type(); - string shared_declare = "PYBIND11_DECLARE_HOLDER_TYPE(T, "+holder_type+", false)"; - string shared_make_opaque = "PYBIND11_MAKE_OPAQUE("+holder_type+")"; + string shared_declare = "PYBIND11_DECLARE_HOLDER_TYPE(T, " + holder_type + ", false)"; + string shared_make_opaque = "PYBIND11_MAKE_OPAQUE(" + holder_type + ")"; string const pybind11_include = "#include <" + Config::get().pybind11_include_file() + ">"; - code = generate_include_directives(includes) + fmt::format(module_header, pybind11_include, config.includes_code(), shared_declare, shared_make_opaque) + prefix_code + "void " + function_name + module_function_suffix + "\n{\n" + code + "}\n"; + code = generate_include_directives(includes) + fmt::format(module_header, pybind11_include, 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/enum.cpp b/source/enum.cpp index b538d739..f938d9d6 100644 --- a/source/enum.cpp +++ b/source/enum.cpp @@ -59,10 +59,10 @@ bool is_bindable(EnumDecl const *E) /// check if user requested binding for the given declaration bool is_binding_requested(clang::EnumDecl const *E, Config const &config) { - if( config.is_enum_binding_requested( E->getQualifiedNameAsString() ) ) return true; + if( config.is_enum_binding_requested(E->getQualifiedNameAsString()) ) return true; bool bind = config.is_namespace_binding_requested(namespace_from_named_decl(E)); - //for( auto &t : get_type_dependencies(E) ) bind &= !is_skipping_requested(t, config); + // for( auto &t : get_type_dependencies(E) ) bind &= !is_skipping_requested(t, config); return bind; } @@ -76,7 +76,7 @@ bool is_skipping_requested(clang::EnumDecl const *E, Config const &config) bool skip = config.is_namespace_skipping_requested(namespace_from_named_decl(E)); - //for( auto &t : get_type_dependencies(E) ) skip |= is_skipping_requested(t, config); + // for( auto &t : get_type_dependencies(E) ) skip |= is_skipping_requested(t, config); return skip; } @@ -123,8 +123,7 @@ std::string bind_enum(std::string const &module, EnumDecl const *E) // Add module local if requested for the namespace std::string module_local_annotation = ""; - if (Config::get().is_module_local_requested(namespace_from_named_decl(E))) - module_local_annotation = ", pybind11::module_local()"; + if( Config::get().is_module_local_requested(namespace_from_named_decl(E)) ) module_local_annotation = ", pybind11::module_local()"; string r = "\tpybind11::enum_<{}>({}, \"{}\"{}, \"{}\"{})\n"_format(qualified_name, module, name, maybe_arithmetic, generate_documentation_string_for_declaration(E), module_local_annotation); @@ -160,8 +159,8 @@ bool EnumBinder::bindable() const /// check if user requested binding for the given declaration void EnumBinder::request_bindings_and_skipping(Config const &config, RequestFlags flags) { - if( (flags&RequestFlags::skipping) and is_skipping_requested(E, config) ) Binder::request_skipping(); - else if( (flags&RequestFlags::binding) and is_binding_requested(E, config) ) Binder::request_bindings(); + if( (flags & RequestFlags::skipping) and is_skipping_requested(E, config) ) Binder::request_skipping(); + else if( (flags & RequestFlags::binding) and is_binding_requested(E, config) ) Binder::request_bindings(); } diff --git a/source/function.cpp b/source/function.cpp index 62b46c5d..dfdf3b39 100644 --- a/source/function.cpp +++ b/source/function.cpp @@ -23,7 +23,7 @@ #include -//#include +// #include #include @@ -44,8 +44,9 @@ namespace binder { // Return the python operator that maps to the C++ operator; returns "" if no mapping exists // This correctly handles operators that have multiple meanings depending on their argument count // For example, operator_(this, other) maps to __sub__ while operator-(this) maps to __neg__ -string cpp_python_operator(const FunctionDecl & F) { - static std::map> const m { +string cpp_python_operator(const FunctionDecl &F) +{ + static std::map> const m{ {"operator+", {"__pos__", "__add__"}}, // {"operator-", {"__neg__", "__sub__"}}, // {"operator*", {"dereference", "__mul__"}}, // @@ -78,10 +79,10 @@ string cpp_python_operator(const FunctionDecl & F) { {"operator--", {"pre_decrement", "post_decrement"}}, // {"operator->", {"arrow"}} // - }; - const auto & found = m.find(F.getNameAsString()); - if (found != m.end()) { - const auto & vec = found->second; + }; + const auto &found = m.find(F.getNameAsString()); + if( found != m.end() ) { + const auto &vec = found->second; const auto n = F.getNumParams(); return n < vec.size() ? vec[n] : vec.back(); } @@ -95,7 +96,7 @@ string function_arguments(clang::FunctionDecl const *record) string r; for( uint i = 0; i < record->getNumParams(); ++i ) { - //r += standard_name(record->getParamDecl(i)->getOriginalType().getCanonicalType().getAsString()); + // r += standard_name(record->getParamDecl(i)->getOriginalType().getCanonicalType().getAsString()); r += standard_name(record->getParamDecl(i)->getOriginalType()); if( i + 1 != record->getNumParams() ) r += ", "; } @@ -226,9 +227,7 @@ string template_specialization(FunctionDecl const *F) // generate string represetiong class name that could be used in python string python_function_name(FunctionDecl const *F) { - if( F->isOverloadedOperator() ) { - return cpp_python_operator(*F); - } + if( F->isOverloadedOperator() ) { return cpp_python_operator(*F); } else { // if( auto m = dyn_cast(F) ) { // } @@ -246,7 +245,7 @@ string python_function_name(FunctionDecl const *F) // Generate function pointer type string for given function: void (*)(int, doule)_ or void (ClassName::*)(int, doule)_ for memeber function string function_pointer_type(FunctionDecl const *F) { - //F->dump(); + // F->dump(); string r; string prefix, maybe_const; if( auto m = dyn_cast(F) ) { @@ -254,7 +253,7 @@ string function_pointer_type(FunctionDecl const *F) maybe_const = m->isConst() ? " const" : ""; } - //r += standard_name(F->getReturnType().getCanonicalType().getAsString()); + // r += standard_name(F->getReturnType().getCanonicalType().getAsString()); r += standard_name(F->getReturnType()); r += " ({}*)("_format(prefix); @@ -275,8 +274,8 @@ string function_qualified_name(FunctionDecl const *F, bool omit_return_type) string maybe_const; if( auto m = dyn_cast(F) ) maybe_const = m->isConst() ? " const" : ""; - string r = (omit_return_type ? "" : standard_name(F->getReturnType()) + " ") + standard_name(F->getQualifiedNameAsString() + template_specialization(F)) + "(" + - function_arguments(F) + ")" + maybe_const; + string r = + (omit_return_type ? "" : standard_name(F->getReturnType()) + " ") + standard_name(F->getQualifiedNameAsString() + template_specialization(F)) + "(" + function_arguments(F) + ")" + maybe_const; fix_boolean_types(r); return r; @@ -331,8 +330,10 @@ bool is_skipping_requested(FunctionDecl const *F, Config const &config) string qualified_name = standard_name(F->getQualifiedNameAsString()); string qualified_name_with_args_info_and_template_specialization = function_qualified_name(F, true); - if( config.is_function_skipping_requested(qualified_name_with_args_info_and_template_specialization) ) return true; // qualified function name + parameter and template info was requested for skipping - if( config.is_function_binding_requested(qualified_name_with_args_info_and_template_specialization) ) return false; // qualified function name + parameter and template info was requested for binding + if( config.is_function_skipping_requested(qualified_name_with_args_info_and_template_specialization) ) + return true; // qualified function name + parameter and template info was requested for skipping + if( config.is_function_binding_requested(qualified_name_with_args_info_and_template_specialization) ) + return false; // qualified function name + parameter and template info was requested for binding if( config.is_function_skipping_requested(qualified_name) ) return true; // qualified function name was requested for skipping if( config.is_function_binding_requested(qualified_name) ) return false; // qualified function name was requested for binding @@ -373,7 +374,7 @@ string bind_function(FunctionDecl const *F, uint args_to_bind, bool request_bind if( m and m->isStatic() ) { maybe_static = "_static"; function_name = Config::get().prefix_for_static_member_functions() + function_name; - //outs() << "STATIC: " << function_qualified_name << " → " << function_name << "\n"; + // outs() << "STATIC: " << function_qualified_name << " → " << function_name << "\n"; } string function, documentation; @@ -382,8 +383,8 @@ string bind_function(FunctionDecl const *F, uint args_to_bind, bool request_bind documentation = generate_documentation_string_for_declaration(F); if( documentation.size() ) documentation += "\\n\\n"; - documentation += "C++: " + standard_name(F->getQualifiedNameAsString() + "(" + function_arguments(F) + ')' + (m and m->isConst() ? " const" : "") + " --> " + - standard_name(F->getReturnType() ) ); + documentation += + "C++: " + standard_name(F->getQualifiedNameAsString() + "(" + function_arguments(F) + ')' + (m and m->isConst() ? " const" : "") + " --> " + standard_name(F->getReturnType())); } else { pair args = function_arguments_for_lambda(F, args_to_bind); @@ -454,7 +455,8 @@ string bind_function(string const &module, FunctionDecl const *F, Context &conte if( O_annotate_functions ) { string const include = relevant_include(F); - code += "\t// function-signature: " + function_qualified_name(F) + "(" + function_arguments(F) + ") file:" + (include.size() ? include.substr(1, include.size() - 2) : "") + " line:" + line_number(F) + "\n"; + code += "\t// function-signature: " + function_qualified_name(F) + "(" + function_arguments(F) + ") file:" + (include.size() ? include.substr(1, include.size() - 2) : "") + + " line:" + line_number(F) + "\n"; } int num_params = F->getNumParams(); @@ -544,7 +546,7 @@ bool is_bindable(FunctionDecl const *F) if( it != cache.end() ) return it->second; else { bool r = is_bindable_raw(F); - cache.insert( {F, r} ); + cache.insert({F, r}); return r; } @@ -601,8 +603,8 @@ bool FunctionBinder::bindable() const /// check if user requested binding for the given declaration void FunctionBinder::request_bindings_and_skipping(Config const &config, RequestFlags flags) { - if( (flags&RequestFlags::skipping) and is_skipping_requested(F, config) ) Binder::request_skipping(); - else if( (flags&RequestFlags::binding) and is_binding_requested(F, config) ) Binder::request_bindings(); + if( (flags & RequestFlags::skipping) and is_skipping_requested(F, config) ) Binder::request_skipping(); + else if( (flags & RequestFlags::binding) and is_binding_requested(F, config) ) Binder::request_bindings(); } diff --git a/source/main.cpp b/source/main.cpp index 5db6d642..f3e06314 100644 --- a/source/main.cpp +++ b/source/main.cpp @@ -28,8 +28,8 @@ #include #include #include -#include #include +#include using namespace clang::tooling; using namespace llvm; @@ -226,9 +226,10 @@ class BinderFrontendAction : public ASTFrontendAction int main(int argc, const char **argv) { #if( LLVM_VERSION_MAJOR < 6 ) - llvm::cl::SetVersionPrinter([]() { std::cout<< "binder " << BINDER_VERSION_STRING << "\nLLVM " << LLVM_VERSION_MAJOR << "." << LLVM_VERSION_MINOR << "." << LLVM_VERSION_PATCH << "\n"; }); + llvm::cl::SetVersionPrinter([]() { std::cout << "binder " << BINDER_VERSION_STRING << "\nLLVM " << LLVM_VERSION_MAJOR << "." << LLVM_VERSION_MINOR << "." << LLVM_VERSION_PATCH << "\n"; }); #else - llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) { OS << "binder " << BINDER_VERSION_STRING << "\nLLVM " << LLVM_VERSION_MAJOR << "." << LLVM_VERSION_MINOR << "." << LLVM_VERSION_PATCH << "\n"; }); + llvm::cl::SetVersionPrinter( + [](llvm::raw_ostream &OS) { OS << "binder " << BINDER_VERSION_STRING << "\nLLVM " << LLVM_VERSION_MAJOR << "." << LLVM_VERSION_MINOR << "." << LLVM_VERSION_PATCH << "\n"; }); #endif #if( LLVM_VERSION_MAJOR < 13 ) CommonOptionsParser op(argc, argv, BinderToolCategory); diff --git a/source/options.cpp b/source/options.cpp index 703782f1..b2d3bf03 100644 --- a/source/options.cpp +++ b/source/options.cpp @@ -33,7 +33,7 @@ cl::opt O_flat("flat", cl::desc("When specified generated files into singl cl::init(false), cl::cat(BinderToolCategory)); cl::opt O_include_pybind11_stl("include-pybind11-stl", cl::desc("When specified bindings for STL classes in will be used instead of generating custom STL bindings."), - cl::init(false), cl::cat(BinderToolCategory)); + cl::init(false), cl::cat(BinderToolCategory)); cl::opt O_root_module("root-module", cl::desc("Name of root module"), /*cl::init("example"),*/ cl::cat(BinderToolCategory)); diff --git a/source/stl_binders.hpp b/source/stl_binders.hpp index 3cd28f39..e8c8273c 100644 --- a/source/stl_binders.hpp +++ b/source/stl_binders.hpp @@ -64,15 +64,14 @@ template class vector_binder template class map_binder { public: - map_binder(pybind11::module &m, std::string const &key_name, std::string const &value_name, std::string const & comparator_name, std::string const & allocator_name) + map_binder(pybind11::module &m, std::string const &key_name, std::string const &value_name, std::string const &comparator_name, std::string const &allocator_name) { using Map = std::map; using holder_type = std::shared_ptr< std::map >; using Class_ = pybind11::class_; - std::string maybe_extra = - ( (comparator_name == "std_less_" + key_name + "_t") ? "" : ("_" + comparator_name) ) + - ( (allocator_name == "std_allocator_std_pair_const_" + key_name + '_' + value_name + "_t") ? "" : ("_" + allocator_name) ); + std::string maybe_extra = ((comparator_name == "std_less_" + key_name + "_t") ? "" : ("_" + comparator_name)) + + ((allocator_name == "std_allocator_std_pair_const_" + key_name + '_' + value_name + "_t") ? "" : ("_" + allocator_name)); Class_ cl = pybind11::bind_map(m, "map_" + key_name + '_' + value_name + maybe_extra); } diff --git a/source/type.cpp b/source/type.cpp index ab52d34e..530fc053 100644 --- a/source/type.cpp +++ b/source/type.cpp @@ -20,12 +20,12 @@ #include #include #include -#include #include +#include -//#include +// #include -//#include +// #include #include #include @@ -458,10 +458,10 @@ void request_bindings(clang::QualType const &qt, Context &context) /// return standard string representation of a given type std::string standard_name(clang::QualType const &qt) { - //qt.getUnqualifiedType().dump(); - //qt->getCanonicalTypeInternal().dump(); - //qt.getTypePtr()->dump(); - //outs() << qt->getTypeClassName() << '\n'; + // qt.getUnqualifiedType().dump(); + // qt->getCanonicalTypeInternal().dump(); + // qt.getTypePtr()->dump(); + // outs() << qt->getTypeClassName() << '\n'; string r = qt.getAsString(); @@ -482,7 +482,9 @@ std::string standard_name(clang::QualType const &qt) // if( standard_names.count(r) ) return r; // else return standard_name(qt.getCanonicalType().getAsString()); - static std::set standard_names_to_ignore {"std::size_t", }; // "std::array::size_type", + static std::set standard_names_to_ignore{ + "std::size_t", + }; // "std::array::size_type", if( standard_names_to_ignore.count(r) ) return r; else return standard_name(qt.getCanonicalType().getAsString()); } @@ -688,18 +690,30 @@ bool is_python_builtin(NamedDecl const *C) static std::set const known_builtin = { //"std::nullptr_t", "nullptr_t", - "std::basic_string", "std::initializer_list", "std::__1::basic_string", + "std::basic_string", + "std::initializer_list", + "std::__1::basic_string", - "std::allocator", "std::__allocator_destructor", + "std::allocator", + "std::__allocator_destructor", - Config::get().holder_type(), "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", - "std::pair", "std::tuple", + "std::pair", + "std::tuple", //"std::__1::pair", "std::__1::tuple", - "std::_Rb_tree_iterator", "std::_Rb_tree_const_iterator", "__gnu_cxx::__normal_iterator", "std::_List_iterator", "std::_List_const_iterator", "std::__list_node", + "std::_Rb_tree_iterator", + "std::_Rb_tree_const_iterator", + "__gnu_cxx::__normal_iterator", + "std::_List_iterator", + "std::_List_const_iterator", + "std::__list_node", "std::__wrap_iter", //"std::__1::__wrap_iter", @@ -711,11 +725,16 @@ bool is_python_builtin(NamedDecl const *C) "std::__value_type", - "std::__detail::_Hash_node_base", "std::__detail::_Hash_node", "std::__detail::_Node_iterator", "std::__detail::_Node_iterator_base", "std::__detail::_Node_const_iterator", + "std::__detail::_Hash_node_base", + "std::__detail::_Hash_node", + "std::__detail::_Node_iterator", + "std::__detail::_Node_iterator_base", + "std::__detail::_Node_const_iterator", "std::__hash_value_type", - "std::function", "std::complex", + "std::function", + "std::complex", // pybind11 types // https://pybind11.readthedocs.io/en/stable/advanced/pycpp/object.html @@ -773,14 +792,11 @@ bool is_python_builtin(NamedDecl const *C) }; // Not builtin's - if (Config::get().not_python_builtins.count(name)) - return false; + if( Config::get().not_python_builtins.count(name) ) return false; // Builtins - if (Config::get().python_builtins.count(name) || known_builtin.count(name)) - return true; + if( Config::get().python_builtins.count(name) || known_builtin.count(name) ) return true; // STL - if (O_include_pybind11_stl && stl_builtin.count(name)) - return true; + if( O_include_pybind11_stl && stl_builtin.count(name) ) return true; return false; } diff --git a/source/util.cpp b/source/util.cpp index 2b425a52..b94ebb41 100644 --- a/source/util.cpp +++ b/source/util.cpp @@ -22,7 +22,7 @@ #include #include -//#include +// #include #include #include #include @@ -69,7 +69,7 @@ void replace_reverse(string &r, string const &from, string const &to) void replace(string &r, string const &from, string const &to) { std::string::size_type i = 0; - while( ( i = r.find(from, i) ) != std::string::npos ) { + while( (i = r.find(from, i)) != std::string::npos ) { r.replace(i, from.size(), to); i += to.size(); } @@ -115,12 +115,12 @@ std::string trim(std::string const &s) { static std::string const whitespaces = " \t"; - auto begin = s.find_first_not_of(whitespaces); - if(begin == std::string::npos) return ""; + auto begin = s.find_first_not_of(whitespaces); + if( begin == std::string::npos ) return ""; auto end = s.find_last_not_of(whitespaces); - return s.substr(begin, end - begin + 1); + return s.substr(begin, end - begin + 1); } @@ -135,7 +135,7 @@ void update_source_file(std::string const &prefix, std::string const &file_name, // std::experimental::filesystem::create_directories(path); string command_line = "mkdir -p " + path; - if (system(command_line.c_str()) != 0) throw std::runtime_error("ERROR: Command \"" + command_line + "\" failed"); + if( system(command_line.c_str()) != 0 ) throw std::runtime_error("ERROR: Command \"" + command_line + "\" failed"); string full_file_name = prefix + '/' + file_name; std::ifstream f(full_file_name); @@ -240,7 +240,7 @@ string template_argument_to_string(clang::TemplateArgument const &t) t.print(Policy, s, true); #endif - string r = s.str(); + string r = s.str(); // // simplify result if argument is a small number, taking care of cases like 1L or 1UL // if( r.size() < 5 and std::isdigit(r[0]) ) { @@ -256,7 +256,7 @@ string template_argument_to_string(clang::TemplateArgument const &t) string line_number(NamedDecl const *decl) { bool l_skip_line_number = Config::get().skip_line_number; - if (l_skip_line_number) return std::string(""); + if( l_skip_line_number ) return std::string(""); ASTContext &ast_context(decl->getASTContext()); SourceManager &sm(ast_context.getSourceManager()); From 84263aec13079ac8749d3c3f7e56821bf62cf381 Mon Sep 17 00:00:00 2001 From: Lucas Czech Date: Mon, 28 Jul 2025 01:12:54 +0200 Subject: [PATCH 2/5] Add more support for return value policies (RVP). - Add RVP defaults for free functions. - Add RVP default for assignment operators. - Add RVP customization per function. - Add test case and documentation for all of the above. - Add some new headings and refinements to config doc. - Add checks for correct +/- in config token parsing. --- documentation/config.rst | 114 ++++++++++++++++----- source/config.cpp | 123 ++++++++++++++++++---- source/config.hpp | 33 ++++-- source/function.cpp | 147 +++++++++++++++++++++++++-- source/function.hpp | 8 ++ test/T62.return_value_policy.config | 24 +++++ test/T62.return_value_policy.hpp | 127 +++++++++++++++++++++++ test/T62.return_value_policy.ref.cpp | 109 ++++++++++++++++++++ 8 files changed, 621 insertions(+), 64 deletions(-) create mode 100644 test/T62.return_value_policy.config create mode 100644 test/T62.return_value_policy.hpp create mode 100644 test/T62.return_value_policy.ref.cpp diff --git a/documentation/config.rst b/documentation/config.rst index 9298effd..fe8b253a 100644 --- a/documentation/config.rst +++ b/documentation/config.rst @@ -1,15 +1,14 @@ Configuration ############# -Binder provides two ways to supply configuration options: command-line and config file. +Binder provides two ways to supply configuration options: command-line options for overall options and a config file for specific details. Command line options ==================== -``--root-module`` specify name of generated Python root module. This name is also used as prefix for various Binder output -files. Typically the following files will be generated: ``.cpp``, ``.sources``, +``--root-module`` specify name of generated Python root module. This name is also used as prefix for various Binder output files. Typically the following files will be generated: ``.cpp``, ``.sources``, ``.modules``. @@ -53,14 +52,15 @@ files. Typically the following files will be generated: ``.cpp``, ` Config file options =================== -Config file is text file containing either comment-line (starts with #) or directive line started with either ``+`` or ``-`` signs -followed by a directive's name and optional parameters. Some directives will accept only the ``+`` while others could be used with +The config file is a text file containing either a comment-line (starts with ``#``) or a directive line started with either ``+`` or ``-`` signs followed by a directive's name and optional parameters. Some directives will accept only the ``+`` while others can be used with both prefixes. -Config file directives: ------------------------ +Selective bindings +------------------ -* ``namespace``, specify if functions/classes/enums from particular namespace should be bound. Could be used with both ``+`` and ``-`` +These directives select which bindings shall be generated or skipped. + +* ``namespace``, specify if functions/classes/enums from particular namespace should be bound. Can be used with both ``+`` and ``-`` prefixes. This directive works recursively so for example if you specify ``+namespace root`` and later ``-namespace root::a`` then all objects in ``root`` will be bound with exception of ``root::a`` and its descendants. @@ -88,16 +88,25 @@ Config file directives: -class utility::pointer::ReferenceCount -class std::__weak_ptr -* ``field``, specify if a particular field should be bound. +* ``field``, specify if a particular field should be skipped (only with ``-``). .. code-block:: bash -field MyClass::some_field +* ``function``, specify if particular function should be bound (``+``) or skipped (``-``). This can be used for both template and normal function. Functions can be specified by just their name, or for more specific control, via their full argument list. To get the exact rendering of the argument list correct, it is recommended to run Binder first to see its output, and then use the generated function name here. + +.. code-block:: bash + + -function ObjexxFCL::FArray::operator-= + -function core::id::swap + -function aaa::foo(const std::string &) + + * ``python_builtin``, specify if particular class/struct should be considered a python builtin and assume existing bindings for it already exist. The purpose of this directive is to allow developer to allow developers to toggle if bindings for types like ``std::optional`` or ``pybind11::dict`` should be - generated, or if binder should assume such bindings already exist somewhere else. Alternatively, a developer could declare a type as not-builtin if they + generated, or if binder should assume such bindings already exist somewhere else. Alternatively, a developer can declare a type as not-builtin if they would prefer to force binder to generate bindings for it. Note that removing a builtin (``-python_builtin abc``) always overrides everything else (such as adding a builtin via ``+python_builtin abc``). .. code-block:: bash @@ -106,18 +115,13 @@ Config file directives: +python_builtin std::vector +Header includes +--------------- -* ``function``, specify if particular function should be bound. This could be used for both template and normal function. - -.. code-block:: bash - - -function ObjexxFCL::FArray::operator-= - -function core::id::swap - - +These directives allow to include additional headers in the generated code. * ``include``, directive to control C++ include directives. Force Binder to either skip adding particular include into generated - source files (``-`` prefix) or force Binder to always add some include files into each generated file. Normally Binder could + source files (``-`` prefix) or force Binder to always add some include files into each generated file. Normally Binder can automatically determine which C++ header files is needed in order to specify type/functions but in some cases it might be useful to be able to control this process. For example forcing some includes is particularly useful when you want to provide custom-binder-functions with either ``+binder`` or ``+add_on_binder`` directives. @@ -148,6 +152,11 @@ Config file directives: +include_for_namespace aaaa::bbbb +Additional bindings +------------------- + +These directives allow to specify add-ons and other custom binding code to augment or replace the generated code. + * ``binder``, specify custom binding function for particular concrete or template class. In the example below all specializations of template std::vector will be handled by ``binder::vector_binder`` function. For template classes binder @@ -175,7 +184,7 @@ Config file directives: * ``+binder_for_namespace``, similar to ``binder``: specify custom binding function for namespace. Call to specified function will be generated - _instead_ of generating bindings for namaspace. Where expected type signature of specified function should be `void f(pybind11::module &)` + `instead` of generating bindings for namaspace. Where expected type signature of specified function should be `void f(pybind11::module &)` .. code-block:: bash @@ -191,37 +200,88 @@ Config file directives: +add_on_binder_for_namespace aaaa::bbbb binder_for_namespace_aaaa_bbbb +Return Value Policy +------------------- + +By default, Binder will add a return value policy statement whereever needed, but keep it at the defaults. These directives allow to overwrite this behavior for particular types of functions. + -* ``default_static_pointer_return_value_policy``, specify return value policy for static functions returning pointer to objects. Default is +For class member functions: + + +* ``default_static_pointer_return_value_policy``, specify the default return value policy for static member functions returning a pointer to an object. Default is `pybind11::return_value_policy::automatic`. -* ``default_static_lvalue_reference_return_value_policy``, specify return value policy for static functions returning l-value reference. Default +* ``default_static_lvalue_reference_return_value_policy``, specify the default return value policy for static member functions returning an l-value reference. Default is `pybind11::return_value_policy::automatic`. -* ``default_static_rvalue_reference_return_value_policy``, specify return value policy for static functions returning r-value reference. Default +* ``default_static_rvalue_reference_return_value_policy``, specify the default return value policy for static member functions returning an r-value reference. Default is `pybind11::return_value_policy::automatic`. -* ``default_member_pointer_return_value_policy``, specify return value policy for member functions returning pointer to objects. Default is +* ``default_member_pointer_return_value_policy``, specify the default return value policy for member functions returning a pointer to an object. Default is `pybind11::return_value_policy::automatic`. -* ``default_member_lvalue_reference_return_value_policy``, specify return value policy for member functions returning l-value reference. Default +* ``default_member_lvalue_reference_return_value_policy``, specify the default return value policy for member functions returning an l-value reference. Default is `pybind11::return_value_policy::automatic`. -* ``default_member_rvalue_reference_return_value_policy``, specify return value policy for member functions returning r-value reference. Default +* ``default_member_rvalue_reference_return_value_policy``, specify the default return value policy for member functions returning an r-value reference. Default is `pybind11::return_value_policy::automatic`. -* ``default_call_guard``, optionally specify a call guard applied to all function definitions. See `pybind11 documentation `_. Default None. + +* ``default_member_assignment_operator_return_value_policy``, specify the default return value policy for assignment operators (`operator=`, `operator+=`, `operator-=`, `operator*=`, `operator/=`, `operator%=`, `operator<<=`, `operator>>=`, `operator&=`, `operator|=`, `operator^=`). This is used only if all the following conditions are met: A non-static member function, taking exactly one parameter, and returning an lvalue reference to the class type itself. In particular, the `default` copy and move assignment operators fulfill these requirements. Typically, other such assignemnt operators also end in ``return *this;``, returning a reference to the assigned-to instance. This is thus a special case where the above return value policy defaults might not be the correct choice. Default is empty, in which case this is not used, in order to maintain backwards compatibility with previous Binder verions. It is recommended to set this to `pybind11::return_value_policy::reference_internal`. .. code-block:: bash +default_member_pointer_return_value_policy pybind11::return_value_policy::reference +default_member_lvalue_reference_return_value_policy pybind11::return_value_policy::reference_internal +default_member_rvalue_reference_return_value_policy pybind11::return_value_policy::move + + +For free functions, Binder offers separate defaults. This is for instance useful for functions that create objects and return a pointer to them. Returning references from free functions is likely not common, but offered for completeness as well. + + +* ``default_function_pointer_return_value_policy``, specify the default return value policy for free functions returning a pointer to an object. Default is + `pybind11::return_value_policy::automatic`. + + +* ``default_function_lvalue_reference_return_value_policy``, specify the default return value policy for free functions returning an l-value reference. Default + is `pybind11::return_value_policy::automatic`. + + +* ``default_function_rvalue_reference_return_value_policy``, specify the default return value policy for free functions returning an r-value reference. Default + is `pybind11::return_value_policy::automatic`. + +.. code-block:: bash + + +default_function_pointer_return_value_policy pybind11::return_value_policy::move + +default_function_lvalue_reference_return_value_policy pybind11::return_value_policy::reference + +default_function_rvalue_reference_return_value_policy pybind11::return_value_policy::move + + +In verbose mode, Binder prints functions that use any of those default return value policies. This is meant as a check for developers to see if any of them need custom policies instead. + +* ``return_value_policy``, specify a custom return value policy for a member function or free function. This overwrites the default above in cases that need customization. This can be specified for all overloads of a function at once, or separately per overload by providing the fully specified name with arguments (and template specializations). + +.. code-block:: bash + + +return_value_policy aaa::foo pybind11::return_value_policy::move + +return_value_policy aaa::foo(const std::string &) pybind11::return_value_policy::copy + +return_value_policy aaa::A::bar pybind11::return_value_policy::reference + +return_value_policy aaa::A::baz(int, int) pybind11::return_value_policy::reference_internal + + +Miscellaneous +------------- + +* ``default_call_guard``, optionally specify a call guard applied to all function definitions. See `pybind11 documentation `_. Default None. + +.. code-block:: bash + +default_call_guard pybind11::gil_scoped_release * ``+custom_shared``: specify a custom shared pointer class that Binder should use instead of ``std::shared_ptr``. diff --git a/source/config.cpp b/source/config.cpp index c68e4361..33fbd41a 100644 --- a/source/config.cpp +++ b/source/config.cpp @@ -28,16 +28,21 @@ using std::string; namespace binder { -/// Split string in two by ether space or tab character +/// Split string in two by either the last space or last tab character std::pair split_in_two(string const &s, string const &error_string) { - size_t space = s.find(' '); - size_t tab = s.find('\t'); - - size_t split = std::min(space, tab); - - if( split == string::npos ) throw std::runtime_error(error_string); - else return std::make_pair(s.substr(0, split), s.substr(split + 1)); + // We search for the _last_ occurrence here, as for instance tokens such as + // trampoline_member_function_binder or return_value_policy take a function + // name as their argument, which might include spaces if it is a fully + // specified name, e.g., `foo(const std::string &)`. + // For all other tokens, there is only a single delimiter, + // so there the order of search does not matter. + size_t space = s.rfind(' '); + size_t tab = s.rfind('\t'); + size_t split = std::max(space == string::npos ? 0 : space, tab == string::npos ? 0 : tab); + + if( space == string::npos and tab == string::npos ) throw std::runtime_error(error_string); + else return std::make_pair(trim(s.substr(0, split)), trim(s.substr(split + 1))); } @@ -89,6 +94,14 @@ void Config::read(string const &file_name) string const _default_member_lvalue_reference_return_value_policy_{"default_member_lvalue_reference_return_value_policy"}; string const _default_member_rvalue_reference_return_value_policy_{"default_member_rvalue_reference_return_value_policy"}; + string const _default_member_assignment_operator_return_value_policy_{"default_member_assignment_operator_return_value_policy"}; + + string const _default_function_pointer_return_value_policy_{"default_function_pointer_return_value_policy"}; + string const _default_function_lvalue_reference_return_value_policy_{"default_function_lvalue_reference_return_value_policy"}; + string const _default_function_rvalue_reference_return_value_policy_{"default_function_rvalue_reference_return_value_policy"}; + + string const _return_value_policy_{"return_value_policy"}; + string const _trampoline_member_function_binder_{"trampoline_member_function_binder"}; string const _default_call_guard_{"default_call_guard"}; @@ -174,6 +187,7 @@ void Config::read(string const &file_name) } else if( token == _buffer_protocol_ ) { if( bind ) { buffer_protocols.push_back(name_without_spaces); } + else throw std::runtime_error("buffer_protocol must be '+' configuration."); } else if( token == _module_local_namespace_ ) { if( bind ) { module_local_namespaces_to_add.push_back(name_without_spaces); } @@ -187,6 +201,7 @@ void Config::read(string const &file_name) auto binder_function = split_in_two(name, "Invalid line for binder specification! Must be: name_of_type + + name_of_binder. Got: " + line); binders_[binder_function.first] = trim(binder_function.second); } + else throw std::runtime_error("binder must be '+' configuration."); } else if( token == _add_on_binder_ ) { @@ -194,6 +209,7 @@ void Config::read(string const &file_name) auto binder_function = split_in_two(name, "Invalid line for add_on_binder specification! Must be: name_of_type + + name_of_binder. Got: " + line); add_on_binders_[binder_function.first] = trim(binder_function.second); } + else throw std::runtime_error("add_on_binder must be '+' configuration."); } else if( token == _binder_for_namespace_ ) { @@ -201,6 +217,7 @@ void Config::read(string const &file_name) auto binder_function = split_in_two(name, "Invalid line for binder_for_namespace specification! Must be: name_of_type + + name_of_binder. Got: " + line); binder_for_namespaces_[binder_function.first] = trim(binder_function.second); } + else throw std::runtime_error("binder_for_namespace must be '+' configuration."); } else if( token == _add_on_binder_for_namespace_ ) { @@ -208,31 +225,91 @@ void Config::read(string const &file_name) auto binder_function = split_in_two(name, "Invalid line for add_on_binder_for_namespace specification! Must be: name_of_type + + name_of_binder. Got: " + line); add_on_binder_for_namespaces_[binder_function.first] = trim(binder_function.second); } + else throw std::runtime_error("add_on_binder_for_namespace must be '+' configuration."); } else if( token == _field_ ) { if( !bind ) { fields_to_skip.push_back(name_without_spaces); } + else throw std::runtime_error("field must be '-' configuration."); + } + else if( token == _custom_shared_ ) { + if( bind ) { holder_type_ = name_without_spaces; } + else throw std::runtime_error("custom_shared must be '+' configuration."); } - else if( token == _custom_shared_ ) holder_type_ = name_without_spaces; else if( token == _smart_holder_ ) { if( bind ) { smart_held_classes.push_back(name_without_spaces); } + else throw std::runtime_error("smart_holder must be '+' configuration."); } else if( token == _pybind11_include_file_ ) { - pybind11_include_file_ = name_without_spaces; + if( bind ) { pybind11_include_file_ = name_without_spaces; } + else throw std::runtime_error("pybind11_include_file must be '+' configuration."); } - 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; - else if( token == _default_static_rvalue_reference_return_value_policy_ ) default_static_rvalue_reference_return_value_policy_ = name_without_spaces; + else if( token == _default_static_pointer_return_value_policy_ ) { + if( bind ) { default_static_pointer_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_static_pointer_return_value_policy must be '+' configuration."); + } + else if( token == _default_static_lvalue_reference_return_value_policy_ ) { + if( bind ) { default_static_lvalue_reference_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_static_lvalue_reference_return_value_policy must be '+' configuration."); + } + else if( token == _default_static_rvalue_reference_return_value_policy_ ) { + if( bind ) { default_static_rvalue_reference_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_static_rvalue_reference_return_value_policy must be '+' configuration."); + } - else if( token == _default_member_pointer_return_value_policy_ ) default_member_pointer_return_value_policy_ = name_without_spaces; - else if( token == _default_member_lvalue_reference_return_value_policy_ ) default_member_lvalue_reference_return_value_policy_ = name_without_spaces; - else if( token == _default_member_rvalue_reference_return_value_policy_ ) default_member_rvalue_reference_return_value_policy_ = name_without_spaces; - else if( token == _default_call_guard_ ) default_call_guard_ = name_without_spaces; + else if( token == _default_member_pointer_return_value_policy_ ) { + if( bind ) { default_member_pointer_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_member_pointer_return_value_policy must be '+' configuration."); + } + else if( token == _default_member_lvalue_reference_return_value_policy_ ) { + if( bind ) { default_member_lvalue_reference_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_member_lvalue_reference_return_value_policy must be '+' configuration."); + } + else if( token == _default_member_rvalue_reference_return_value_policy_ ) { + if( bind ) { default_member_rvalue_reference_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_member_rvalue_reference_return_value_policy must be '+' configuration."); + } - else if( token == _prefix_for_static_member_functions_ ) prefix_for_static_member_functions_ = name_without_spaces; + else if( token == _default_member_assignment_operator_return_value_policy_ ) { + if( bind ) { default_member_assignment_operator_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_member_assignment_operator_return_value_policy must be '+' configuration."); + } + + else if( token == _default_function_pointer_return_value_policy_ ) { + if( bind ) { default_function_pointer_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_function_pointer_return_value_policy must be '+' configuration."); + } + else if( token == _default_function_lvalue_reference_return_value_policy_ ) { + if( bind ) { default_function_lvalue_reference_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_function_lvalue_reference_return_value_policy must be '+' configuration."); + } + else if( token == _default_function_rvalue_reference_return_value_policy_ ) { + if( bind ) { default_function_rvalue_reference_return_value_policy_ = name_without_spaces; } + else throw std::runtime_error("default_function_rvalue_reference_return_value_policy must be '+' configuration."); + } + + else if( token == _return_value_policy_ ) { + if( bind ) { + auto binder_function = split_in_two(name, "Invalid line for return_value_policy specification! Must be: name_of_function + + name_of_return_value_policy. Got: " + line); + return_value_policy_[binder_function.first] = binder_function.second; + } + else { + throw std::runtime_error("return_value_policy must be '+' configuration."); + } + } + + else if( token == _default_call_guard_ ) { + if( bind ) { default_call_guard_ = name_without_spaces; } + else throw std::runtime_error("default_call_guard must be '+' configuration."); + } + + else if( token == _prefix_for_static_member_functions_ ) { + if( bind ) { prefix_for_static_member_functions_ = name_without_spaces; } + else throw std::runtime_error("prefix_for_static_member_functions must be '+' configuration."); + } else if( token == _trampoline_member_function_binder_ ) { if( bind ) { @@ -240,6 +317,7 @@ void Config::read(string const &file_name) name, "Invalid line for trampoline_member_function_binder specification! Must be: qualified_class_name::member_funtion_name + + name_of_function. Got: " + line); custom_trampoline_functions_[member_function_name_and_function_name.first] = member_function_name_and_function_name.second; } + else throw std::runtime_error("trampoline_member_function_binder must be '+' configuration."); } else { @@ -475,6 +553,15 @@ bool Config::is_include_skipping_requested(string const &include) const } +string Config::get_return_value_policy(string const &function) const +{ + string const func = trim(function); + auto rvp_it = return_value_policy_.find(func); + if( rvp_it == return_value_policy_.end() ) return ""; + return rvp_it->second; +} + + string Config::includes_code() const { std::ostringstream s; diff --git a/source/config.hpp b/source/config.hpp index 692fc872..f744c5b0 100644 --- a/source/config.hpp +++ b/source/config.hpp @@ -52,6 +52,14 @@ class Config string default_member_pointer_return_value_policy_ = "pybind11::return_value_policy::automatic"; 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_member_assignment_operator_return_value_policy_ = ""; + std::map return_value_policy_; + + string default_function_pointer_return_value_policy_ = "pybind11::return_value_policy::automatic"; + string default_function_lvalue_reference_return_value_policy_ = "pybind11::return_value_policy::automatic"; + string default_function_rvalue_reference_return_value_policy_ = "pybind11::return_value_policy::automatic"; + string default_call_guard_ = ""; string holder_type_ = "std::shared_ptr"; string pybind11_include_file_ = "pybind11/pybind11.h"; @@ -81,16 +89,23 @@ class Config std::map > const &class_includes() const { return class_includes_; } std::map > const &namespace_includes() const { return namespace_includes_; } - string const &default_static_pointer_return_value_policy() { return default_static_pointer_return_value_policy_; } - string const &default_static_lvalue_reference_return_value_policy() { return default_static_lvalue_reference_return_value_policy_; } - string const &default_static_rvalue_reference_return_value_policy() { return default_static_rvalue_reference_return_value_policy_; } + string const &default_static_pointer_return_value_policy() const { return default_static_pointer_return_value_policy_; } + string const &default_static_lvalue_reference_return_value_policy() const { return default_static_lvalue_reference_return_value_policy_; } + string const &default_static_rvalue_reference_return_value_policy() const { return default_static_rvalue_reference_return_value_policy_; } + + string const &default_member_pointer_return_value_policy() const { return default_member_pointer_return_value_policy_; } + string const &default_member_lvalue_reference_return_value_policy() const { return default_member_lvalue_reference_return_value_policy_; } + string const &default_member_rvalue_reference_return_value_policy() const { return default_member_rvalue_reference_return_value_policy_; } - string const &default_member_pointer_return_value_policy() { return default_member_pointer_return_value_policy_; } - string const &default_member_lvalue_reference_return_value_policy() { return default_member_lvalue_reference_return_value_policy_; } - 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 &default_member_assignment_operator_return_value_policy() const { return default_member_assignment_operator_return_value_policy_; } - string const &prefix_for_static_member_functions() { return prefix_for_static_member_functions_; } + string const &default_function_pointer_return_value_policy() const { return default_function_pointer_return_value_policy_; } + string const &default_function_lvalue_reference_return_value_policy() const { return default_function_lvalue_reference_return_value_policy_; } + string const &default_function_rvalue_reference_return_value_policy() const { return default_function_rvalue_reference_return_value_policy_; } + + string const &default_call_guard() const { return default_call_guard_; } + + string const &prefix_for_static_member_functions() const { return prefix_for_static_member_functions_; } string const &holder_type() const { return holder_type_; } string const &pybind11_include_file() const { return pybind11_include_file_; } @@ -121,6 +136,8 @@ class Config bool is_include_skipping_requested(string const &include) const; + string get_return_value_policy(string const &function) const; + string is_custom_trampoline_function_requested(string const &function__) const; string includes_code() const; diff --git a/source/function.cpp b/source/function.cpp index dfdf3b39..6e0cf240 100644 --- a/source/function.cpp +++ b/source/function.cpp @@ -20,6 +20,7 @@ #include #include +#include #include @@ -27,6 +28,8 @@ #include +using llvm::errs; +using llvm::outs; using namespace llvm; using namespace clang; @@ -361,6 +364,137 @@ bool is_skipping_requested(FunctionDecl const *F, Config const &config) } +/// Returns whether F is an overloaded assignment operator (operator= or any of the compound assigns) +bool is_valid_assignment_operator(const clang::FunctionDecl *F) +{ + // For any of the assignment operators (operator= or any of the compound assigns), we declare it to be a valid (typical) assignment operator if it is declared as a non-static member, taking + // exactly one parameter, and returning an lvalue reference to the class type itself. In that case, we want to apply a specialized default return value policy to it. + + // Must be a non‑static C++ member function + auto *m = dyn_cast(F); + if( !m || m->isStatic() ) return false; + + // Must be an overloaded operator + if( !m->isOverloadedOperator() ) return false; + + // Only accept assignment & compound‑assignment operators + using Op = clang::OverloadedOperatorKind; + switch( m->getOverloadedOperator() ) { + case Op::OO_Equal: // operator= + case Op::OO_PlusEqual: // operator+= + case Op::OO_MinusEqual: // operator-= + case Op::OO_StarEqual: // operator*= + case Op::OO_SlashEqual: // operator/= + case Op::OO_PercentEqual: // operator%= + case Op::OO_LessLessEqual: // operator<<= + case Op::OO_GreaterGreaterEqual: // operator>>= + case Op::OO_AmpEqual: // operator&= + case Op::OO_PipeEqual: // operator|= + case Op::OO_CaretEqual: // operator^= + break; + default: + return false; + } + + // Must take exactly one argument + if( m->getNumParams() != 1 ) return false; + + // Return type must be an lvalue reference + clang::QualType ret_ty = m->getReturnType(); + if( !ret_ty->isLValueReferenceType() ) return false; + + // The referenced type must be the class’s own type + clang::QualType pointeeTy = ret_ty->getPointeeType().getCanonicalType(); + clang::ASTContext &Ctx = m->getASTContext(); + clang::QualType classTy = Ctx.getRecordType(m->getParent()).getCanonicalType(); + if( !Ctx.hasSameType(pointeeTy, classTy) ) return false; + + return true; +} + + +/// get the return value policy string for a function +std::string get_rv_policy_for_function(clang::FunctionDecl const *F, Config const &config) +{ + // Return value of this function, and a string noting which type of function we are dealing with. + // If left empty, this means that the function does not require a return value policy. + string rvp = ""; + string rvp_default_name; + + // Check if any default return value policy is applicable. + CXXMethodDecl const *m = dyn_cast(F); + if( is_valid_assignment_operator(F) && !config.default_member_assignment_operator_return_value_policy().empty() ) { rvp = ", " + config.default_member_assignment_operator_return_value_policy(); } + else if( m and !m->isStatic() ) { + // Member functions + if( F->getReturnType()->isPointerType() ) { + rvp = ", " + config.default_member_pointer_return_value_policy(); + rvp_default_name = "default_member_pointer_return_value_policy"; + } + else if( F->getReturnType()->isLValueReferenceType() ) { + rvp = ", " + config.default_member_lvalue_reference_return_value_policy(); + rvp_default_name = "default_member_lvalue_reference_return_value_policy"; + } + else if( F->getReturnType()->isRValueReferenceType() ) { + rvp = ", " + config.default_member_rvalue_reference_return_value_policy(); + rvp_default_name = "default_member_rvalue_reference_return_value_policy"; + } + } + else if( m and m->isStatic() ) { + // Static member functions + if( F->getReturnType()->isPointerType() ) { + rvp = ", " + config.default_static_pointer_return_value_policy(); + rvp_default_name = "default_static_pointer_return_value_policy"; + } + else if( F->getReturnType()->isLValueReferenceType() ) { + rvp = ", " + config.default_static_lvalue_reference_return_value_policy(); + rvp_default_name = "default_static_lvalue_reference_return_value_policy"; + } + else if( F->getReturnType()->isRValueReferenceType() ) { + rvp = ", " + config.default_static_rvalue_reference_return_value_policy(); + rvp_default_name = "default_static_rvalue_reference_return_value_policy"; + } + } + else { + // Free functions + if( F->getReturnType()->isPointerType() ) { + rvp = ", " + config.default_function_pointer_return_value_policy(); + rvp_default_name = "default_function_pointer_return_value_policy"; + } + else if( F->getReturnType()->isLValueReferenceType() ) { + rvp = ", " + config.default_function_lvalue_reference_return_value_policy(); + rvp_default_name = "default_function_lvalue_reference_return_value_policy"; + } + else if( F->getReturnType()->isRValueReferenceType() ) { + rvp = ", " + config.default_function_rvalue_reference_return_value_policy(); + rvp_default_name = "default_function_rvalue_reference_return_value_policy"; + } + } + + // Check if there is a custom rv policy for this function (for its general and specialized names, such that fully specified names have precedence over qualified names). + string const qualified_name = standard_name(F->getQualifiedNameAsString()); + string const specified_name = function_qualified_name(F, true); + string custom_rvp = config.get_return_value_policy(specified_name); + if( custom_rvp.empty() ) { custom_rvp = config.get_return_value_policy(qualified_name); } + + // If any of the function names has a custom rv policy, we use it. + if( !custom_rvp.empty() ) { + // Warn if the function is not of a type that should have an rv policy. + if( rvp_default_name.empty() ) + errs() << "Function " << specified_name << " has a custom return value policy specified in the config (" << custom_rvp + << "), but based on its return value type is not a function that should have a policy specified"; + + // Here, the config has specified an rv policy for the function, so we overwrite the default. + rvp = ", " + custom_rvp; + } + else if( !rvp_default_name.empty() ) { + // In verbose mode, we print functions that use the default rv policy, to allow devs to quickly identify them and check if they need customization in the config. + if( O_verbose ) outs() << "Function " << specified_name << " uses " << rvp_default_name << "\n"; + } + + return rvp; +} + + // Generate binding for given function: .def("foo", (std::string (aaaa::A::*)(int) ) &aaaa::A::foo, "doc") string bind_function(FunctionDecl const *F, uint args_to_bind, bool request_bindings_f, Context &context, CXXRecordDecl const *parent, bool always_use_lambda) { @@ -416,17 +550,8 @@ string bind_function(FunctionDecl const *F, uint args_to_bind, bool request_bind } } - string maybe_return_policy = ""; - if( m and !m->isStatic() ) { - if( F->getReturnType()->isPointerType() ) maybe_return_policy = ", " + Config::get().default_member_pointer_return_value_policy(); - else if( F->getReturnType()->isLValueReferenceType() ) maybe_return_policy = ", " + Config::get().default_member_lvalue_reference_return_value_policy(); - else if( F->getReturnType()->isRValueReferenceType() ) maybe_return_policy = ", " + Config::get().default_member_rvalue_reference_return_value_policy(); - } - else { - if( F->getReturnType()->isPointerType() ) maybe_return_policy = ", " + Config::get().default_static_pointer_return_value_policy(); - else if( F->getReturnType()->isLValueReferenceType() ) maybe_return_policy = ", " + Config::get().default_static_lvalue_reference_return_value_policy(); - else if( F->getReturnType()->isRValueReferenceType() ) maybe_return_policy = ", " + Config::get().default_static_rvalue_reference_return_value_policy(); - } + // Get the return value policy for the function, or empty if not needed. + string const maybe_return_policy = get_rv_policy_for_function(F, Config::get()); // string r = R"(.def{}("{}", ({}) &{}{}, "doc")"_format(maybe_static, function_name, function_pointer_type(F), function_qualified_name, template_specialization(F)); string r = R"(.def{}("{}", {}, "{}"{})"_format(maybe_static, function_name, function, documentation, maybe_return_policy); diff --git a/source/function.hpp b/source/function.hpp index 6e401443..f81d7bae 100644 --- a/source/function.hpp +++ b/source/function.hpp @@ -62,6 +62,14 @@ bool is_binding_requested(clang::FunctionDecl const *F, Config const &config); bool is_skipping_requested(clang::FunctionDecl const *F, Config const &config); +/// Returns if F is an overloaded assignment operator (operator= or any of the compound assigns) +bool is_valid_assignment_operator(const clang::FunctionDecl *F); + + +/// get the return value policy string for a function +std::string get_rv_policy_for_function(clang::FunctionDecl const *F, Config const &config); + + // Generate binding for given function: .def("foo", (std::string (aaaa::A::*)(int) ) &aaaa::A::foo, "doc") // If function have default arguments generate set of bindings by creating separate bindings for each argument with default. // if parent is not nullptr then bind function as-if it a member of that CXXRecordDecl (for handling visibility changes with 'using' directive) diff --git a/test/T62.return_value_policy.config b/test/T62.return_value_policy.config new file mode 100644 index 00000000..b971039b --- /dev/null +++ b/test/T62.return_value_policy.config @@ -0,0 +1,24 @@ +# Static members ++default_static_pointer_return_value_policy pybind11::return_value_policy::move ++default_static_lvalue_reference_return_value_policy pybind11::return_value_policy::reference_internal ++default_static_rvalue_reference_return_value_policy pybind11::return_value_policy::move + +# Non-static members ++default_member_pointer_return_value_policy pybind11::return_value_policy::move ++default_member_lvalue_reference_return_value_policy pybind11::return_value_policy::reference_internal ++default_member_rvalue_reference_return_value_policy pybind11::return_value_policy::move + +# Assignment operators ++default_member_assignment_operator_return_value_policy pybind11::return_value_policy::reference_internal + +# Free functions ++default_function_pointer_return_value_policy pybind11::return_value_policy::move ++default_function_lvalue_reference_return_value_policy pybind11::return_value_policy::copy ++default_function_rvalue_reference_return_value_policy pybind11::return_value_policy::move + +# Custom policies ++return_value_policy B::operator-= pybind11::return_value_policy::automatic ++return_value_policy B::get_custom pybind11::return_value_policy::reference ++return_value_policy B::get_custom(int, int) pybind11::return_value_policy::copy ++return_value_policy get_custom pybind11::return_value_policy::reference ++return_value_policy get_custom(int, int) pybind11::return_value_policy::copy diff --git a/test/T62.return_value_policy.hpp b/test/T62.return_value_policy.hpp new file mode 100644 index 00000000..83664c06 --- /dev/null +++ b/test/T62.return_value_policy.hpp @@ -0,0 +1,127 @@ +// -*- mode:c++;tab-width:2;indent-tabs-mode:t;show-trailing-whitespace:t;rm-trailing-spaces:t -*- +// vi: set ts=2 noet: +// +// Copyright (c) 2025 Lucas Czech +// +// All rights reserved. Use of this source code is governed by a +// MIT license that can be found in the LICENSE file. + +#pragma once + +#include + +struct A { + int x; + int y; +}; + +struct B { + static A a_static; + A a_member; + + // Test static members + + static A* get_static_pointer() + { + return new A(); + } + + static A& get_static_lvalue_ref() + { + return a_static; + } + + static A&& get_static_rvalue_ref() + { + return std::move( a_static ); + } + + // Test non-static members + + A* get_member_pointer() + { + return new A(); + } + + A& get_member_lvalue_ref() + { + return a_member; + } + + A&& get_member_rvalue_ref() + { + return std::move( a_member ); + } + + // Test customizations + + A const& get_custom() + { + return a_member; + } + + A const& get_custom( int x, int y ) + { + (void) x; + (void) y; + return a_member; + } + + // Test assignment operators + + B& operator=( B const& other ) = default; + B& operator=( B&& other ) = default; + + B& operator +=( int i ) + { + a_member.x += i; + return *this; + } + + B& operator -=( int i ) + { + a_member.x -= i; + return *this; + } + +}; + +// Test the defaults for free functions + +inline A* get_pointer() +{ + return new A(); +} + +inline A& get_lvalue_ref() +{ + static A a{}; + return a; +} + +inline A&& get_rvalue_ref() +{ + static A a{}; + return std::move( a ); +} + +// Test customization for free functions + +inline A const& get_custom() +{ + static A a{ 0, 0 }; + return a; +} + +inline A const& get_custom( int x, int y ) +{ + // Ignores x and y after the first call. Not recommended in practice - just for testing. + static A a{ x, y }; + return a; +} + +// Static inits. This should of course be in a compilation unit instead of this header, +// but to keep the test setup simple, we assume here that this header is only every going +// to be included once in the compilation unit of the test, and not linked elsewhere. +// Do not try this at home! +A B::a_static = A{}; diff --git a/test/T62.return_value_policy.ref.cpp b/test/T62.return_value_policy.ref.cpp new file mode 100644 index 00000000..6e967c4b --- /dev/null +++ b/test/T62.return_value_policy.ref.cpp @@ -0,0 +1,109 @@ +// File: T62_return_value_policy.cpp +#include // A +#include // B +#include // get_custom +#include // get_lvalue_ref +#include // get_pointer +#include // __str__ + +#include +#include +#include + +#ifndef BINDER_PYBIND11_TYPE_CASTER + #define BINDER_PYBIND11_TYPE_CASTER + PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr, false) + PYBIND11_DECLARE_HOLDER_TYPE(T, T*, false) + PYBIND11_MAKE_OPAQUE(std::shared_ptr) +#endif + +void bind_T62_return_value_policy(std::function< pybind11::module &(std::string const &namespace_) > &M) +{ + { // A file:T62.return_value_policy.hpp line: + pybind11::class_> cl(M(""), "A", ""); + cl.def( pybind11::init( [](A const &o){ return new A(o); } ) ); + cl.def( pybind11::init( [](){ return new A(); } ) ); + cl.def_readwrite("x", &A::x); + cl.def_readwrite("y", &A::y); + cl.def("assign", (struct A & (A::*)(const struct A &)) &A::operator=, "C++: A::operator=(const struct A &) --> struct A &", pybind11::return_value_policy::reference_internal, pybind11::arg("")); + } + { // B file:T62.return_value_policy.hpp line: + pybind11::class_> cl(M(""), "B", ""); + cl.def( pybind11::init( [](){ return new B(); } ) ); + cl.def_readwrite("a_member", &B::a_member); + cl.def_static("get_static_pointer", (struct A * (*)()) &B::get_static_pointer, "C++: B::get_static_pointer() --> struct A *", pybind11::return_value_policy::move); + cl.def_static("get_static_lvalue_ref", (struct A & (*)()) &B::get_static_lvalue_ref, "C++: B::get_static_lvalue_ref() --> struct A &", pybind11::return_value_policy::reference_internal); + cl.def("get_member_pointer", (struct A * (B::*)()) &B::get_member_pointer, "C++: B::get_member_pointer() --> struct A *", pybind11::return_value_policy::move); + cl.def("get_member_lvalue_ref", (struct A & (B::*)()) &B::get_member_lvalue_ref, "C++: B::get_member_lvalue_ref() --> struct A &", pybind11::return_value_policy::reference_internal); + cl.def("get_custom", (const struct A & (B::*)()) &B::get_custom, "C++: B::get_custom() --> const struct A &", pybind11::return_value_policy::reference); + cl.def("get_custom", (const struct A & (B::*)(int, int)) &B::get_custom, "C++: B::get_custom(int, int) --> const struct A &", pybind11::return_value_policy::copy, pybind11::arg("x"), pybind11::arg("y")); + cl.def("assign", (struct B & (B::*)(const struct B &)) &B::operator=, "C++: B::operator=(const struct B &) --> struct B &", pybind11::return_value_policy::reference_internal, pybind11::arg("other")); + cl.def("__iadd__", (struct B & (B::*)(int)) &B::operator+=, "C++: B::operator+=(int) --> struct B &", pybind11::return_value_policy::reference_internal, pybind11::arg("i")); + cl.def("__isub__", (struct B & (B::*)(int)) &B::operator-=, "C++: B::operator-=(int) --> struct B &", pybind11::return_value_policy::automatic, pybind11::arg("i")); + } + // get_pointer() file:T62.return_value_policy.hpp line: + M("").def("get_pointer", (struct A * (*)()) &get_pointer, "C++: get_pointer() --> struct A *", pybind11::return_value_policy::move); + + // get_lvalue_ref() file:T62.return_value_policy.hpp line: + M("").def("get_lvalue_ref", (struct A & (*)()) &get_lvalue_ref, "C++: get_lvalue_ref() --> struct A &", pybind11::return_value_policy::copy); + + // get_custom() file:T62.return_value_policy.hpp line: + M("").def("get_custom", (const struct A & (*)()) &get_custom, "C++: get_custom() --> const struct A &", pybind11::return_value_policy::reference); + + // get_custom(int, int) file:T62.return_value_policy.hpp line: + M("").def("get_custom", (const struct A & (*)(int, int)) &get_custom, "C++: get_custom(int, int) --> const struct A &", pybind11::return_value_policy::copy, pybind11::arg("x"), pybind11::arg("y")); + +} + + +#include +#include +#include +#include +#include +#include + +#include + +using ModuleGetter = std::function< pybind11::module & (std::string const &) >; + +void bind_T62_return_value_policy(std::function< pybind11::module &(std::string const &namespace_) > &M); + + +PYBIND11_MODULE(T62_return_value_policy, root_module) { + root_module.doc() = "T62_return_value_policy module"; + + std::map 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 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; + return ns+'_'; + } + ); + + std::vector< std::pair > sub_modules { + }; + for(auto &p : sub_modules ) modules[ p.first.empty() ? p.second : p.first+"::"+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_>(M(""), "_encapsulated_data_"); + + bind_T62_return_value_policy(M); + +} + +// Source list file: TEST/T62_return_value_policy.sources +// T62_return_value_policy.cpp +// T62_return_value_policy.cpp + +// Modules list file: TEST/T62_return_value_policy.modules +// From 6c3d039de9d58c6658f543ddaf0a55f3b88d521e Mon Sep 17 00:00:00 2001 From: Lucas Czech Date: Tue, 29 Jul 2025 11:14:19 +0200 Subject: [PATCH 3/5] Fix rv policy for overloaded functions with custom policies. Before, there was a problematic edge case: a function with a custom rv policy set in the config for the unspecific function name, which has overloads where one returns some reference type, and hence needs an rv policy, but another overload that returns a non-reference type, would receive an rv policy in the bindings for both overloads. Now, the overload that does not require an rv policy to be set does not get one in the bindings. --- source/function.cpp | 73 ++++++++++++++-------------- test/T62.return_value_policy.hpp | 12 +++++ test/T62.return_value_policy.ref.cpp | 4 ++ 3 files changed, 52 insertions(+), 37 deletions(-) diff --git a/source/function.cpp b/source/function.cpp index 6e0cf240..76aaf475 100644 --- a/source/function.cpp +++ b/source/function.cpp @@ -416,57 +416,58 @@ bool is_valid_assignment_operator(const clang::FunctionDecl *F) /// get the return value policy string for a function std::string get_rv_policy_for_function(clang::FunctionDecl const *F, Config const &config) { - // Return value of this function, and a string noting which type of function we are dealing with. - // If left empty, this means that the function does not require a return value policy. - string rvp = ""; - string rvp_default_name; + // Prepare the return value policy string, and a string noting which type of function we are dealing with (for user output). + string rvp_string = ""; + string rvp_name; // Check if any default return value policy is applicable. CXXMethodDecl const *m = dyn_cast(F); - if( is_valid_assignment_operator(F) && !config.default_member_assignment_operator_return_value_policy().empty() ) { rvp = ", " + config.default_member_assignment_operator_return_value_policy(); } + if( is_valid_assignment_operator(F) && !config.default_member_assignment_operator_return_value_policy().empty() ) { + rvp_string = ", " + config.default_member_assignment_operator_return_value_policy(); + } else if( m and !m->isStatic() ) { // Member functions if( F->getReturnType()->isPointerType() ) { - rvp = ", " + config.default_member_pointer_return_value_policy(); - rvp_default_name = "default_member_pointer_return_value_policy"; + rvp_string = ", " + config.default_member_pointer_return_value_policy(); + rvp_name = "default_member_pointer_return_value_policy"; } else if( F->getReturnType()->isLValueReferenceType() ) { - rvp = ", " + config.default_member_lvalue_reference_return_value_policy(); - rvp_default_name = "default_member_lvalue_reference_return_value_policy"; + rvp_string = ", " + config.default_member_lvalue_reference_return_value_policy(); + rvp_name = "default_member_lvalue_reference_return_value_policy"; } else if( F->getReturnType()->isRValueReferenceType() ) { - rvp = ", " + config.default_member_rvalue_reference_return_value_policy(); - rvp_default_name = "default_member_rvalue_reference_return_value_policy"; + rvp_string = ", " + config.default_member_rvalue_reference_return_value_policy(); + rvp_name = "default_member_rvalue_reference_return_value_policy"; } } else if( m and m->isStatic() ) { // Static member functions if( F->getReturnType()->isPointerType() ) { - rvp = ", " + config.default_static_pointer_return_value_policy(); - rvp_default_name = "default_static_pointer_return_value_policy"; + rvp_string = ", " + config.default_static_pointer_return_value_policy(); + rvp_name = "default_static_pointer_return_value_policy"; } else if( F->getReturnType()->isLValueReferenceType() ) { - rvp = ", " + config.default_static_lvalue_reference_return_value_policy(); - rvp_default_name = "default_static_lvalue_reference_return_value_policy"; + rvp_string = ", " + config.default_static_lvalue_reference_return_value_policy(); + rvp_name = "default_static_lvalue_reference_return_value_policy"; } else if( F->getReturnType()->isRValueReferenceType() ) { - rvp = ", " + config.default_static_rvalue_reference_return_value_policy(); - rvp_default_name = "default_static_rvalue_reference_return_value_policy"; + rvp_string = ", " + config.default_static_rvalue_reference_return_value_policy(); + rvp_name = "default_static_rvalue_reference_return_value_policy"; } } else { // Free functions if( F->getReturnType()->isPointerType() ) { - rvp = ", " + config.default_function_pointer_return_value_policy(); - rvp_default_name = "default_function_pointer_return_value_policy"; + rvp_string = ", " + config.default_function_pointer_return_value_policy(); + rvp_name = "default_function_pointer_return_value_policy"; } else if( F->getReturnType()->isLValueReferenceType() ) { - rvp = ", " + config.default_function_lvalue_reference_return_value_policy(); - rvp_default_name = "default_function_lvalue_reference_return_value_policy"; + rvp_string = ", " + config.default_function_lvalue_reference_return_value_policy(); + rvp_name = "default_function_lvalue_reference_return_value_policy"; } else if( F->getReturnType()->isRValueReferenceType() ) { - rvp = ", " + config.default_function_rvalue_reference_return_value_policy(); - rvp_default_name = "default_function_rvalue_reference_return_value_policy"; + rvp_string = ", " + config.default_function_rvalue_reference_return_value_policy(); + rvp_name = "default_function_rvalue_reference_return_value_policy"; } } @@ -476,22 +477,20 @@ std::string get_rv_policy_for_function(clang::FunctionDecl const *F, Config cons string custom_rvp = config.get_return_value_policy(specified_name); if( custom_rvp.empty() ) { custom_rvp = config.get_return_value_policy(qualified_name); } - // If any of the function names has a custom rv policy, we use it. - if( !custom_rvp.empty() ) { - // Warn if the function is not of a type that should have an rv policy. - if( rvp_default_name.empty() ) - errs() << "Function " << specified_name << " has a custom return value policy specified in the config (" << custom_rvp - << "), but based on its return value type is not a function that should have a policy specified"; - - // Here, the config has specified an rv policy for the function, so we overwrite the default. - rvp = ", " + custom_rvp; - } - else if( !rvp_default_name.empty() ) { - // In verbose mode, we print functions that use the default rv policy, to allow devs to quickly identify them and check if they need customization in the config. - if( O_verbose ) outs() << "Function " << specified_name << " uses " << rvp_default_name << "\n"; + // Check if the function requires an rv policy at all, which is the case if the rvp_string was set. If so, we either use the custom one if given, or the appropriate default. + // Cases where there is a custom rv policy given, but the function does not actually need an rv policy are silently ignored here. This can for instance happen if there is an overloaded function, + // one of which returns a reference, and the other not. In these cases, only the first needs to have the rv policy set, and the other not. + if( !rvp_string.empty() ) { + if( !custom_rvp.empty() ) { rvp_string = ", " + custom_rvp; } + else if( !rvp_name.empty() ) { + // In verbose mode, we print functions that use the default rv policy, to allow devs to quickly identify them and check if they need customization in the config. + // This is not printed for assignment operators (where rvp_name is left empty), as we assume this to be a special case where we are always okay using the default, as it has to be set + // explicitly in the config. + if( O_verbose ) outs() << "Function " << specified_name << " uses " << rvp_name << "\n"; + } } - return rvp; + return rvp_string; } diff --git a/test/T62.return_value_policy.hpp b/test/T62.return_value_policy.hpp index 83664c06..33ddd02d 100644 --- a/test/T62.return_value_policy.hpp +++ b/test/T62.return_value_policy.hpp @@ -67,6 +67,12 @@ struct B { return a_member; } + int get_custom( int x ) + { + // An overload that does not return a reference, and hence should not get an rv policy set. + return x; + } + // Test assignment operators B& operator=( B const& other ) = default; @@ -120,6 +126,12 @@ inline A const& get_custom( int x, int y ) return a; } +inline int get_custom( int x ) +{ + // An overload that does not return a reference, and hence should not get an rv policy set. + return x; +} + // Static inits. This should of course be in a compilation unit instead of this header, // but to keep the test setup simple, we assume here that this header is only every going // to be included once in the compilation unit of the test, and not linked elsewhere. diff --git a/test/T62.return_value_policy.ref.cpp b/test/T62.return_value_policy.ref.cpp index 6e967c4b..27d60cf5 100644 --- a/test/T62.return_value_policy.ref.cpp +++ b/test/T62.return_value_policy.ref.cpp @@ -37,6 +37,7 @@ void bind_T62_return_value_policy(std::function< pybind11::module &(std::string cl.def("get_member_lvalue_ref", (struct A & (B::*)()) &B::get_member_lvalue_ref, "C++: B::get_member_lvalue_ref() --> struct A &", pybind11::return_value_policy::reference_internal); cl.def("get_custom", (const struct A & (B::*)()) &B::get_custom, "C++: B::get_custom() --> const struct A &", pybind11::return_value_policy::reference); cl.def("get_custom", (const struct A & (B::*)(int, int)) &B::get_custom, "C++: B::get_custom(int, int) --> const struct A &", pybind11::return_value_policy::copy, pybind11::arg("x"), pybind11::arg("y")); + cl.def("get_custom", (int (B::*)(int)) &B::get_custom, "C++: B::get_custom(int) --> int", pybind11::arg("x")); cl.def("assign", (struct B & (B::*)(const struct B &)) &B::operator=, "C++: B::operator=(const struct B &) --> struct B &", pybind11::return_value_policy::reference_internal, pybind11::arg("other")); cl.def("__iadd__", (struct B & (B::*)(int)) &B::operator+=, "C++: B::operator+=(int) --> struct B &", pybind11::return_value_policy::reference_internal, pybind11::arg("i")); cl.def("__isub__", (struct B & (B::*)(int)) &B::operator-=, "C++: B::operator-=(int) --> struct B &", pybind11::return_value_policy::automatic, pybind11::arg("i")); @@ -53,6 +54,9 @@ void bind_T62_return_value_policy(std::function< pybind11::module &(std::string // get_custom(int, int) file:T62.return_value_policy.hpp line: M("").def("get_custom", (const struct A & (*)(int, int)) &get_custom, "C++: get_custom(int, int) --> const struct A &", pybind11::return_value_policy::copy, pybind11::arg("x"), pybind11::arg("y")); + // get_custom(int) file:T62.return_value_policy.hpp line: + M("").def("get_custom", (int (*)(int)) &get_custom, "C++: get_custom(int) --> int", pybind11::arg("x")); + } From c15363c1f0239c0ec96c674ec076284c8551cb75 Mon Sep 17 00:00:00 2001 From: Lucas Czech Date: Mon, 4 Aug 2025 23:18:03 +0200 Subject: [PATCH 4/5] Add return value policy for class directive --- documentation/config.rst | 10 ++- source/config.cpp | 22 +++++- source/config.hpp | 3 +- source/function.cpp | 95 ++++++++++++++---------- test/T62.return_value_policy.config | 28 +++++++ test/T62.return_value_policy.hpp | 105 +++++++++++++++++++++++++++ test/T62.return_value_policy.ref.cpp | 64 ++++++++++++++++ 7 files changed, 285 insertions(+), 42 deletions(-) diff --git a/documentation/config.rst b/documentation/config.rst index fe8b253a..ccaa0cd2 100644 --- a/documentation/config.rst +++ b/documentation/config.rst @@ -265,7 +265,15 @@ For free functions, Binder offers separate defaults. This is for instance useful In verbose mode, Binder prints functions that use any of those default return value policies. This is meant as a check for developers to see if any of them need custom policies instead. -* ``return_value_policy``, specify a custom return value policy for a member function or free function. This overwrites the default above in cases that need customization. This can be specified for all overloads of a function at once, or separately per overload by providing the fully specified name with arguments (and template specializations). +* ``return_value_policy_for_class``, specify a custom return value policy for all functions in a class that require one (i.e., that return a reference or pointer type). For class templates, it is also possible to specify a policy for all instantiations (by leaving out the template arguments), and then refine this by overwriting the policy for specific instantiations again. + +.. code-block:: bash + + +return_value_policy_for_class aaa::A pybind11::return_value_policy::move + +return_value_policy_for_class aaa::A pybind11::return_value_policy::copy + + +* ``return_value_policy``, specify a custom return value policy for a member function or free function. This overwrites the default or class-specific policies above. This can be specified for all overloads of a function at once, and again separately refined per overload by providing the fully specified name with arguments (and potentially template specializations). .. code-block:: bash diff --git a/source/config.cpp b/source/config.cpp index 33fbd41a..06a489e4 100644 --- a/source/config.cpp +++ b/source/config.cpp @@ -101,6 +101,7 @@ void Config::read(string const &file_name) string const _default_function_rvalue_reference_return_value_policy_{"default_function_rvalue_reference_return_value_policy"}; string const _return_value_policy_{"return_value_policy"}; + string const _return_value_policy_for_class_{"return_value_policy_for_class"}; string const _trampoline_member_function_binder_{"trampoline_member_function_binder"}; @@ -301,6 +302,16 @@ void Config::read(string const &file_name) } } + else if( token == _return_value_policy_for_class_ ) { + if( bind ) { + auto binder_function = split_in_two(name, "Invalid line for return_value_policy_for_class specification! Must be: name_of_class + + name_of_return_value_policy. Got: " + line); + return_value_policy_for_class_[binder_function.first] = binder_function.second; + } + else { + throw std::runtime_error("return_value_policy_for_class must be '+' configuration."); + } + } + else if( token == _default_call_guard_ ) { if( bind ) { default_call_guard_ = name_without_spaces; } else throw std::runtime_error("default_call_guard must be '+' configuration."); @@ -555,13 +566,20 @@ bool Config::is_include_skipping_requested(string const &include) const string Config::get_return_value_policy(string const &function) const { - string const func = trim(function); - auto rvp_it = return_value_policy_.find(func); + auto rvp_it = return_value_policy_.find( trim( function )); if( rvp_it == return_value_policy_.end() ) return ""; return rvp_it->second; } +string Config::get_return_value_policy_for_class(string const &name) const +{ + auto rvp_it = return_value_policy_for_class_.find( trim( name )); + if( rvp_it == return_value_policy_for_class_.end() ) return ""; + return rvp_it->second; +} + + string Config::includes_code() const { std::ostringstream s; diff --git a/source/config.hpp b/source/config.hpp index f744c5b0..c17477a5 100644 --- a/source/config.hpp +++ b/source/config.hpp @@ -54,7 +54,7 @@ class Config string default_member_rvalue_reference_return_value_policy_ = "pybind11::return_value_policy::automatic"; string default_member_assignment_operator_return_value_policy_ = ""; - std::map return_value_policy_; + std::map return_value_policy_, return_value_policy_for_class_; string default_function_pointer_return_value_policy_ = "pybind11::return_value_policy::automatic"; string default_function_lvalue_reference_return_value_policy_ = "pybind11::return_value_policy::automatic"; @@ -137,6 +137,7 @@ class Config bool is_include_skipping_requested(string const &include) const; string get_return_value_policy(string const &function) const; + string get_return_value_policy_for_class(string const &name) const; string is_custom_trampoline_function_requested(string const &function__) const; diff --git a/source/function.cpp b/source/function.cpp index 76aaf475..d3f6ed68 100644 --- a/source/function.cpp +++ b/source/function.cpp @@ -417,80 +417,99 @@ bool is_valid_assignment_operator(const clang::FunctionDecl *F) std::string get_rv_policy_for_function(clang::FunctionDecl const *F, Config const &config) { // Prepare the return value policy string, and a string noting which type of function we are dealing with (for user output). - string rvp_string = ""; - string rvp_name; + string rvp_result = ""; - // Check if any default return value policy is applicable. + // We generally go from most generic to most specific way of specifying custom rv policies here. + // First, check if any default return value policy is applicable. This also tells us if the function needs an rv policy to begin with. CXXMethodDecl const *m = dyn_cast(F); if( is_valid_assignment_operator(F) && !config.default_member_assignment_operator_return_value_policy().empty() ) { - rvp_string = ", " + config.default_member_assignment_operator_return_value_policy(); + rvp_result = ", " + config.default_member_assignment_operator_return_value_policy(); } else if( m and !m->isStatic() ) { // Member functions if( F->getReturnType()->isPointerType() ) { - rvp_string = ", " + config.default_member_pointer_return_value_policy(); - rvp_name = "default_member_pointer_return_value_policy"; + rvp_result = ", " + config.default_member_pointer_return_value_policy(); } else if( F->getReturnType()->isLValueReferenceType() ) { - rvp_string = ", " + config.default_member_lvalue_reference_return_value_policy(); - rvp_name = "default_member_lvalue_reference_return_value_policy"; + rvp_result = ", " + config.default_member_lvalue_reference_return_value_policy(); } else if( F->getReturnType()->isRValueReferenceType() ) { - rvp_string = ", " + config.default_member_rvalue_reference_return_value_policy(); - rvp_name = "default_member_rvalue_reference_return_value_policy"; + rvp_result = ", " + config.default_member_rvalue_reference_return_value_policy(); } } else if( m and m->isStatic() ) { // Static member functions if( F->getReturnType()->isPointerType() ) { - rvp_string = ", " + config.default_static_pointer_return_value_policy(); - rvp_name = "default_static_pointer_return_value_policy"; + rvp_result = ", " + config.default_static_pointer_return_value_policy(); } else if( F->getReturnType()->isLValueReferenceType() ) { - rvp_string = ", " + config.default_static_lvalue_reference_return_value_policy(); - rvp_name = "default_static_lvalue_reference_return_value_policy"; + rvp_result = ", " + config.default_static_lvalue_reference_return_value_policy(); } else if( F->getReturnType()->isRValueReferenceType() ) { - rvp_string = ", " + config.default_static_rvalue_reference_return_value_policy(); - rvp_name = "default_static_rvalue_reference_return_value_policy"; + rvp_result = ", " + config.default_static_rvalue_reference_return_value_policy(); } } else { // Free functions if( F->getReturnType()->isPointerType() ) { - rvp_string = ", " + config.default_function_pointer_return_value_policy(); - rvp_name = "default_function_pointer_return_value_policy"; + rvp_result = ", " + config.default_function_pointer_return_value_policy(); } else if( F->getReturnType()->isLValueReferenceType() ) { - rvp_string = ", " + config.default_function_lvalue_reference_return_value_policy(); - rvp_name = "default_function_lvalue_reference_return_value_policy"; + rvp_result = ", " + config.default_function_lvalue_reference_return_value_policy(); } else if( F->getReturnType()->isRValueReferenceType() ) { - rvp_string = ", " + config.default_function_rvalue_reference_return_value_policy(); - rvp_name = "default_function_rvalue_reference_return_value_policy"; + rvp_result = ", " + config.default_function_rvalue_reference_return_value_policy(); } } + // If no rv policy was set, then the function does not need one, so we skip all custom ones. + if( rvp_result.empty() ) { + return rvp_result; + } + string custom_rvp; + bool uses_custom_rvp = false; + + // Now check if there is a policy set for the whole class (if the function belongs to one). + // We first check if there is a custom rv policy for the specific class instantiation, and if not, also check if there is one for the class name without template arguments. + if( m ) { + auto C = m->getParent(); // the CXXRecordDecl that the function belongs to + string const qualified_name_without_template = standard_name(C->getQualifiedNameAsString()); + custom_rvp = config.get_return_value_policy_for_class( qualified_name_without_template ); + if( !custom_rvp.empty() ) { + rvp_result = ", " + custom_rvp; + uses_custom_rvp = true; + } + string const qualified_name = class_qualified_name(C); + custom_rvp = config.get_return_value_policy_for_class( qualified_name ); + if( !custom_rvp.empty() ) { + rvp_result = ", " + custom_rvp; + uses_custom_rvp = true; + } + } + + // Now we get more specific and look for the particular function, overriding any previous results if we find something. // Check if there is a custom rv policy for this function (for its general and specialized names, such that fully specified names have precedence over qualified names). string const qualified_name = standard_name(F->getQualifiedNameAsString()); - string const specified_name = function_qualified_name(F, true); - string custom_rvp = config.get_return_value_policy(specified_name); - if( custom_rvp.empty() ) { custom_rvp = config.get_return_value_policy(qualified_name); } - - // Check if the function requires an rv policy at all, which is the case if the rvp_string was set. If so, we either use the custom one if given, or the appropriate default. - // Cases where there is a custom rv policy given, but the function does not actually need an rv policy are silently ignored here. This can for instance happen if there is an overloaded function, - // one of which returns a reference, and the other not. In these cases, only the first needs to have the rv policy set, and the other not. - if( !rvp_string.empty() ) { - if( !custom_rvp.empty() ) { rvp_string = ", " + custom_rvp; } - else if( !rvp_name.empty() ) { - // In verbose mode, we print functions that use the default rv policy, to allow devs to quickly identify them and check if they need customization in the config. - // This is not printed for assignment operators (where rvp_name is left empty), as we assume this to be a special case where we are always okay using the default, as it has to be set - // explicitly in the config. - if( O_verbose ) outs() << "Function " << specified_name << " uses " << rvp_name << "\n"; - } + custom_rvp = config.get_return_value_policy(qualified_name); + if( !custom_rvp.empty() ) { + rvp_result = ", " + custom_rvp; + uses_custom_rvp = true; + } + string const qualified_name_with_args_info_and_template_specialization = function_qualified_name(F, true); + custom_rvp = config.get_return_value_policy(qualified_name_with_args_info_and_template_specialization); + if( !custom_rvp.empty() ) { + rvp_result = ", " + custom_rvp; + uses_custom_rvp = true; + } + + // In verbose mode, we print functions that use the default rv policy, to allow devs to quickly identify them and check if they need customization in the config. + // This is not printed for assignment operators (where rvp_name is left empty), as we assume this to be a special case where we are always okay using the default, as it has to be set + // explicitly in the config. + if( !uses_custom_rvp ) { + if( O_verbose ) outs() << "Function " << qualified_name_with_args_info_and_template_specialization << " uses default return value policy\n"; } - return rvp_string; + return rvp_result; } diff --git a/test/T62.return_value_policy.config b/test/T62.return_value_policy.config index b971039b..99d4f226 100644 --- a/test/T62.return_value_policy.config +++ b/test/T62.return_value_policy.config @@ -1,3 +1,7 @@ +# ------------------------------------------------------------------------------------------------- +# Basics +# ------------------------------------------------------------------------------------------------- + # Static members +default_static_pointer_return_value_policy pybind11::return_value_policy::move +default_static_lvalue_reference_return_value_policy pybind11::return_value_policy::reference_internal @@ -22,3 +26,27 @@ +return_value_policy B::get_custom(int, int) pybind11::return_value_policy::copy +return_value_policy get_custom pybind11::return_value_policy::reference +return_value_policy get_custom(int, int) pybind11::return_value_policy::copy + +# ------------------------------------------------------------------------------------------------- +# Function templates +# ------------------------------------------------------------------------------------------------- + ++return_value_policy my_func_a pybind11::return_value_policy::automatic ++return_value_policy my_func_b(int &) pybind11::return_value_policy::automatic + +# ------------------------------------------------------------------------------------------------- +# Class templates +# ------------------------------------------------------------------------------------------------- + +# Class that gets a custom policy as a whole. ++return_value_policy_for_class C pybind11::return_value_policy::copy + +# Class where one getter gets a custom policy overwriting the custom policy for the class. ++return_value_policy_for_class D pybind11::return_value_policy::copy ++return_value_policy D::get_x() pybind11::return_value_policy::move + +# Class template where all template instantiations get a custom policy for the class. ++return_value_policy_for_class E pybind11::return_value_policy::copy + +# Class template where only the int instantiation gets a custom policy, while the std::string instantiation remains at defaults. ++return_value_policy_for_class F pybind11::return_value_policy::copy diff --git a/test/T62.return_value_policy.hpp b/test/T62.return_value_policy.hpp index 33ddd02d..ef29019c 100644 --- a/test/T62.return_value_policy.hpp +++ b/test/T62.return_value_policy.hpp @@ -8,8 +8,13 @@ #pragma once +#include #include +// ------------------------------------------------------------------------------------------------- +// Basics +// ------------------------------------------------------------------------------------------------- + struct A { int x; int y; @@ -137,3 +142,103 @@ inline int get_custom( int x ) // to be included once in the compilation unit of the test, and not linked elsewhere. // Do not try this at home! A B::a_static = A{}; + +// ------------------------------------------------------------------------------------------------- +// Function templates +// ------------------------------------------------------------------------------------------------- + +// Function that gets a policy for all intatiations. +template +T& my_func_a( T& in ) +{ + return in; +} +template int& my_func_a(int&); +template std::string& my_func_a(std::string&); + +// Function that gets a policy only for int. +template +T& my_func_b( T& in ) +{ + return in; +} +template int& my_func_b(int&); +template std::string& my_func_b(std::string&); + +// ------------------------------------------------------------------------------------------------- +// Class templates +// ------------------------------------------------------------------------------------------------- + +// Class that gets a custom policy as a whole. +struct C +{ + int x; + + int& get_x() + { + return x; + } + + int const& get_x( int y ) const + { + (void) y; + return x; + } +}; + +// Class where one getter gets a custom policy overwriting the custom policy for the class. +struct D +{ + int x; + + int& get_x() + { + return x; + } + + int const& get_x( int y ) const + { + (void) y; + return x; + } +}; + +// Class template where all template instantiations get a custom policy for the class. +template +struct E{ + int x; + + int& get_x() + { + return x; + } + + int const& get_x( int y ) const + { + (void) y; + return x; + } +}; + +template struct E; +template struct E; + +// Class template where only the int instantiation gets a custom policy, while the std::string instantiation remains at defaults. +template +struct F{ + int x; + + int& get_x() + { + return x; + } + + int const& get_x( int y ) const + { + (void) y; + return x; + } +}; + +template struct F; +template struct F; diff --git a/test/T62.return_value_policy.ref.cpp b/test/T62.return_value_policy.ref.cpp index 27d60cf5..f7d0b70b 100644 --- a/test/T62.return_value_policy.ref.cpp +++ b/test/T62.return_value_policy.ref.cpp @@ -1,10 +1,20 @@ // File: T62_return_value_policy.cpp #include // A #include // B +#include // C +#include // D +#include // E +#include // F #include // get_custom #include // get_lvalue_ref #include // get_pointer +#include // my_func_a +#include // my_func_b +#include // __gnu_cxx::__normal_iterator +#include // std::allocator #include // __str__ +#include // std::basic_string +#include // std::char_traits #include #include @@ -57,6 +67,60 @@ void bind_T62_return_value_policy(std::function< pybind11::module &(std::string // get_custom(int) file:T62.return_value_policy.hpp line: M("").def("get_custom", (int (*)(int)) &get_custom, "C++: get_custom(int) --> int", pybind11::arg("x")); + // my_func_a(int &) file:T62.return_value_policy.hpp line: + M("").def("my_func_a", (int & (*)(int &)) &my_func_a, "C++: my_func_a(int &) --> int &", pybind11::return_value_policy::automatic, pybind11::arg("in")); + + // my_func_a(std::string &) file:T62.return_value_policy.hpp line: + M("").def("my_func_a", (std::string & (*)(std::string &)) &my_func_a, "C++: my_func_a(std::string &) --> std::string &", pybind11::return_value_policy::automatic, pybind11::arg("in")); + + // my_func_b(int &) file:T62.return_value_policy.hpp line: + M("").def("my_func_b", (int & (*)(int &)) &my_func_b, "C++: my_func_b(int &) --> int &", pybind11::return_value_policy::automatic, pybind11::arg("in")); + + // my_func_b(std::string &) file:T62.return_value_policy.hpp line: + M("").def("my_func_b", (std::string & (*)(std::string &)) &my_func_b, "C++: my_func_b(std::string &) --> std::string &", pybind11::return_value_policy::copy, pybind11::arg("in")); + + { // C file:T62.return_value_policy.hpp line: + pybind11::class_> cl(M(""), "C", ""); + cl.def( pybind11::init( [](){ return new C(); } ) ); + cl.def_readwrite("x", &C::x); + cl.def("get_x", (int & (C::*)()) &C::get_x, "C++: C::get_x() --> int &", pybind11::return_value_policy::copy); + cl.def("get_x", (const int & (C::*)(int) const) &C::get_x, "C++: C::get_x(int) const --> const int &", pybind11::return_value_policy::copy, pybind11::arg("y")); + } + { // D file:T62.return_value_policy.hpp line: + pybind11::class_> cl(M(""), "D", ""); + cl.def( pybind11::init( [](){ return new D(); } ) ); + cl.def_readwrite("x", &D::x); + cl.def("get_x", (int & (D::*)()) &D::get_x, "C++: D::get_x() --> int &", pybind11::return_value_policy::move); + cl.def("get_x", (const int & (D::*)(int) const) &D::get_x, "C++: D::get_x(int) const --> const int &", pybind11::return_value_policy::copy, pybind11::arg("y")); + } + { // E file:T62.return_value_policy.hpp line: + pybind11::class_, std::shared_ptr>> cl(M(""), "E_int_t", ""); + cl.def( pybind11::init( [](){ return new E(); } ) ); + cl.def_readwrite("x", &E::x); + cl.def("get_x", (int & (E::*)()) &E::get_x, "C++: E::get_x() --> int &", pybind11::return_value_policy::copy); + cl.def("get_x", (const int & (E::*)(int) const) &E::get_x, "C++: E::get_x(int) const --> const int &", pybind11::return_value_policy::copy, pybind11::arg("y")); + } + { // E file:T62.return_value_policy.hpp line: + pybind11::class_, std::shared_ptr>> cl(M(""), "E_std_string_t", ""); + cl.def( pybind11::init( [](){ return new E(); } ) ); + cl.def_readwrite("x", &E::x); + cl.def("get_x", (int & (E::*)()) &E::get_x, "C++: E::get_x() --> int &", pybind11::return_value_policy::copy); + cl.def("get_x", (const int & (E::*)(int) const) &E::get_x, "C++: E::get_x(int) const --> const int &", pybind11::return_value_policy::copy, pybind11::arg("y")); + } + { // F file:T62.return_value_policy.hpp line: + pybind11::class_, std::shared_ptr>> cl(M(""), "F_int_t", ""); + cl.def( pybind11::init( [](){ return new F(); } ) ); + cl.def_readwrite("x", &F::x); + cl.def("get_x", (int & (F::*)()) &F::get_x, "C++: F::get_x() --> int &", pybind11::return_value_policy::copy); + cl.def("get_x", (const int & (F::*)(int) const) &F::get_x, "C++: F::get_x(int) const --> const int &", pybind11::return_value_policy::copy, pybind11::arg("y")); + } + { // F file:T62.return_value_policy.hpp line: + pybind11::class_, std::shared_ptr>> cl(M(""), "F_std_string_t", ""); + cl.def( pybind11::init( [](){ return new F(); } ) ); + cl.def_readwrite("x", &F::x); + cl.def("get_x", (int & (F::*)()) &F::get_x, "C++: F::get_x() --> int &", pybind11::return_value_policy::reference_internal); + cl.def("get_x", (const int & (F::*)(int) const) &F::get_x, "C++: F::get_x(int) const --> const int &", pybind11::return_value_policy::reference_internal, pybind11::arg("y")); + } } From 5111313a361c84671c6c3dedcb7ea73dd60436ed Mon Sep 17 00:00:00 2001 From: Lucas Czech Date: Tue, 5 Aug 2025 14:17:57 +0200 Subject: [PATCH 5/5] Fix special case of assignment operators taking precedence over class rv policy --- documentation/config.rst | 2 +- source/function.cpp | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/documentation/config.rst b/documentation/config.rst index ccaa0cd2..d26c120f 100644 --- a/documentation/config.rst +++ b/documentation/config.rst @@ -265,7 +265,7 @@ For free functions, Binder offers separate defaults. This is for instance useful In verbose mode, Binder prints functions that use any of those default return value policies. This is meant as a check for developers to see if any of them need custom policies instead. -* ``return_value_policy_for_class``, specify a custom return value policy for all functions in a class that require one (i.e., that return a reference or pointer type). For class templates, it is also possible to specify a policy for all instantiations (by leaving out the template arguments), and then refine this by overwriting the policy for specific instantiations again. +* ``return_value_policy_for_class``, specify a custom return value policy for all functions in a class that require one (i.e., that return a reference or pointer type). For class templates, it is also possible to specify a policy for all instantiations (by leaving out the template arguments), and then refine this by overwriting the policy for specific instantiations again. Note that this class-wide declaration does not overwrite the special case for assignment operators (`default_member_assignment_operator_return_value_policy`); if you need to change the policy for the assignment operators in a class, either change the default, or specify them individual as explained next. .. code-block:: bash diff --git a/source/function.cpp b/source/function.cpp index d3f6ed68..87975bfe 100644 --- a/source/function.cpp +++ b/source/function.cpp @@ -418,12 +418,14 @@ std::string get_rv_policy_for_function(clang::FunctionDecl const *F, Config cons { // Prepare the return value policy string, and a string noting which type of function we are dealing with (for user output). string rvp_result = ""; + bool is_assignment_operator = false; // We generally go from most generic to most specific way of specifying custom rv policies here. // First, check if any default return value policy is applicable. This also tells us if the function needs an rv policy to begin with. CXXMethodDecl const *m = dyn_cast(F); if( is_valid_assignment_operator(F) && !config.default_member_assignment_operator_return_value_policy().empty() ) { rvp_result = ", " + config.default_member_assignment_operator_return_value_policy(); + is_assignment_operator = true; } else if( m and !m->isStatic() ) { // Member functions @@ -471,7 +473,8 @@ std::string get_rv_policy_for_function(clang::FunctionDecl const *F, Config cons // Now check if there is a policy set for the whole class (if the function belongs to one). // We first check if there is a custom rv policy for the specific class instantiation, and if not, also check if there is one for the class name without template arguments. - if( m ) { + // This is skipped for assignment opererators, as they are treated specially. They can be overwritten by a specification for the exact function below though. + if( m && !is_assignment_operator ) { auto C = m->getParent(); // the CXXRecordDecl that the function belongs to string const qualified_name_without_template = standard_name(C->getQualifiedNameAsString()); custom_rvp = config.get_return_value_policy_for_class( qualified_name_without_template ); @@ -505,8 +508,8 @@ std::string get_rv_policy_for_function(clang::FunctionDecl const *F, Config cons // In verbose mode, we print functions that use the default rv policy, to allow devs to quickly identify them and check if they need customization in the config. // This is not printed for assignment operators (where rvp_name is left empty), as we assume this to be a special case where we are always okay using the default, as it has to be set // explicitly in the config. - if( !uses_custom_rvp ) { - if( O_verbose ) outs() << "Function " << qualified_name_with_args_info_and_template_specialization << " uses default return value policy\n"; + if( !uses_custom_rvp && O_verbose ) { + outs() << "Function " << qualified_name_with_args_info_and_template_specialization << " uses default return value policy\n"; } return rvp_result;