Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve operator resolution #129

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve operator resolution #129

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c13c00a
Improve operator resolution
Vipul-Cariappa Feb 22, 2025
005777f
Apply suggestions from code review
Vipul-Cariappa Feb 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 113 additions & 44 deletions clingwrapper/src/clingwrapper.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1492,18 +1492,22 @@ Cppyy::TCppMethod_t Cppyy::GetMethodTemplate(
std::string pureName;
std::string explicit_params;

if (name.find('<') != std::string::npos) {
if ((name.find("operator<") != 0) &&
(name.find('<') != std::string::npos)) {
pureName = name.substr(0, name.find('<'));
size_t start = name.find('<');
size_t end = name.rfind('>');
explicit_params = name.substr(start + 1, end - start - 1);
}

else pureName = name;
} else
pureName = name;

std::vector<Cppyy::TCppMethod_t> unresolved_candidate_methods;
Cpp::GetClassTemplatedMethods(pureName, scope,
unresolved_candidate_methods);
if (unresolved_candidate_methods.empty()) {
// try operators
Cppyy::GetClassOperators(scope, pureName, unresolved_candidate_methods);
}

// CPyCppyy assumes that we attempt instantiation here
std::vector<Cpp::TemplateArgInfo> arg_types;
Expand All @@ -1523,32 +1527,70 @@ Cppyy::TCppMethod_t Cppyy::GetMethodTemplate(

}

//
// static inline
// std::string type_remap(const std::string& n1, const std::string& n2)
// {
// // Operator lookups of (C++ string, Python str) should succeed for the combos of
// // string/str, wstring/str, string/unicode and wstring/unicode; since C++ does not have a
// // operator+(std::string, std::wstring), we'll have to look up the same type and rely on
// // the converters in CPyCppyy/_cppyy.
// if (n1 == "str" || n1 == "unicode") {
// if (n2 == "std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >")
// return n2; // match like for like
// return "std::string"; // probably best bet
// } else if (n1 == "float") {
// return "double"; // debatable, but probably intended
// } else if (n1 == "complex") {
// return "std::complex<double>";
// }
// return n1;
// }
static inline std::string type_remap(const std::string& n1,
const std::string& n2) {
// Operator lookups of (C++ string, Python str) should succeed for the
// combos of string/str, wstring/str, string/unicode and wstring/unicode;
// since C++ does not have a operator+(std::string, std::wstring), we'll
// have to look up the same type and rely on the converters in
// CPyCppyy/_cppyy.
if (n1 == "str" || n1 == "unicode") {
// if (n2 ==
// "std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t>
// >")
// return n2; // match like for like
return "std::string"; // probably best bet
} else if (n1 == "float") {
return "double"; // debatable, but probably intended
} else if (n1 == "complex") {
return "std::complex<double>";
}
return n1;
}

void Cppyy::GetClassOperators(Cppyy::TCppScope_t klass,
const std::string& opname,
std::vector<TCppScope_t>& operators) {
if (opname == "operator+")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something we can fold into an API: const char * GetOperatorKindAsString(Cpp::OperatorKind K);

Cpp::GetOperator(klass, Cpp::Operator::OP_Plus, operators);
else if (opname == "operator-")
Cpp::GetOperator(klass, Cpp::Operator::OP_Minus, operators);
else if (opname == "operator*")
Cpp::GetOperator(klass, Cpp::Operator::OP_Star, operators);
else if (opname == "operator/")
Cpp::GetOperator(klass, Cpp::Operator::OP_Slash, operators);
else if (opname == "operator<")
Cpp::GetOperator(klass, Cpp::Operator::OP_Less, operators);
else if (opname == "operator<=")
Cpp::GetOperator(klass, Cpp::Operator::OP_LessEqual, operators);
else if (opname == "operator>")
Cpp::GetOperator(klass, Cpp::Operator::OP_Greater, operators);
else if (opname == "operator>=")
Cpp::GetOperator(klass, Cpp::Operator::OP_GreaterEqual, operators);
// FIXME: enabling `==` and `!=` requires friend operators
// else if (opname == "operator==")
// Cpp::GetOperator(klass, Cpp::Operator::OP_EqualEqual, operators);
// else if (opname == "operator!=")
// Cpp::GetOperator(klass, Cpp::Operator::OP_ExclaimEqual, operators);
else if (opname == "operator<<")
Cpp::GetOperator(klass, Cpp::Operator::OP_LessLess, operators);
else if (opname == "operator>>")
Cpp::GetOperator(klass, Cpp::Operator::OP_GreaterGreater, operators);
else if (opname == "operator&")
Cpp::GetOperator(klass, Cpp::Operator::OP_Amp, operators);
else if (opname == "operator|")
Cpp::GetOperator(klass, Cpp::Operator::OP_Pipe, operators);
}

Cppyy::TCppMethod_t Cppyy::GetGlobalOperator(
TCppType_t scope, const std::string& lc, const std::string& rc, const std::string& opname)
{
if ((lc.find('<') != std::string::npos) || (rc.find('<') != std::string::npos)) {
// arguments of templated types
return nullptr;
std::string rc_type = type_remap(rc, lc);
std::string lc_type = type_remap(lc, rc);
bool is_templated = false;
if ((lc_type.find('<') != std::string::npos) ||
(rc_type.find('<') != std::string::npos)) {
is_templated = true;
}

std::vector<TCppScope_t> overloads;
Expand All @@ -1568,10 +1610,11 @@ Cppyy::TCppMethod_t Cppyy::GetGlobalOperator(
Cpp::GetOperator(scope, Cpp::Operator::OP_Greater, overloads);
else if (opname == ">=")
Cpp::GetOperator(scope, Cpp::Operator::OP_GreaterEqual, overloads);
else if (opname == "==")
Cpp::GetOperator(scope, Cpp::Operator::OP_EqualEqual, overloads);
else if (opname == "!=")
Cpp::GetOperator(scope, Cpp::Operator::OP_ExclaimEqual, overloads);
// FIXME: enabling `==` and `!=` requires friend operators
// else if (opname == "==")
// Cpp::GetOperator(scope, Cpp::Operator::OP_EqualEqual, overloads);
// else if (opname == "!=")
// Cpp::GetOperator(scope, Cpp::Operator::OP_ExclaimEqual, overloads);
else if (opname == "<<")
Cpp::GetOperator(scope, Cpp::Operator::OP_LessLess, overloads);
else if (opname == ">>")
Expand All @@ -1581,25 +1624,51 @@ Cppyy::TCppMethod_t Cppyy::GetGlobalOperator(
else if (opname == "|")
Cpp::GetOperator(scope, Cpp::Operator::OP_Pipe, overloads);

std::vector<Cppyy::TCppMethod_t> unresolved_candidate_methods;
for (auto overload: overloads) {
if (Cpp::IsTemplatedFunction(overload))
continue;

TCppType_t lhs_type = Cpp::GetFunctionArgType(overload, 0);
if (lc != Cpp::GetTypeAsString(Cpp::GetUnderlyingType(lhs_type)))
if (Cpp::IsTemplatedFunction(overload)) {
unresolved_candidate_methods.push_back(overload);
continue;

if ((!rc.empty()) && (Cpp::GetFunctionNumArgs(overload) == 2)) {
TCppType_t rhs_type = Cpp::GetFunctionArgType(overload, 1);
if (rc != Cpp::GetTypeAsString(Cpp::GetUnderlyingType(rhs_type)))
continue;
} else {
continue;
}
TCppType_t lhs_type = Cpp::GetFunctionArgType(overload, 0);
if (lc_type !=
Cpp::GetTypeAsString(Cpp::GetUnderlyingType(lhs_type)))
continue;

return overload;
if (!rc_type.empty()) {
if (Cpp::GetFunctionNumArgs(overload) != 2)
continue;
TCppType_t rhs_type = Cpp::GetFunctionArgType(overload, 1);
if (rc_type !=
Cpp::GetTypeAsString(Cpp::GetUnderlyingType(rhs_type)))
continue;
}
return overload;
}
}
if (is_templated) {
std::string lc_template = lc_type.substr(
lc_type.find("<") + 1, lc_type.rfind(">") - lc_type.find("<") - 1);
std::string rc_template = rc_type.substr(
rc_type.find("<") + 1, rc_type.rfind(">") - rc_type.find("<") - 1);

std::vector<Cpp::TemplateArgInfo> arg_types;
if (auto l = Cppyy::GetType(lc_type, true))
arg_types.emplace_back(l);
else
return nullptr;

if (!rc_type.empty()) {
if (auto r = Cppyy::GetType(rc_type, true))
arg_types.emplace_back(r);
else
return nullptr;
}
Cppyy::TCppMethod_t cppmeth = Cpp::BestTemplateFunctionMatch(
unresolved_candidate_methods, {}, arg_types);
if (cppmeth)
return cppmeth;
}

return nullptr;
}

Expand Down
4 changes: 3 additions & 1 deletion clingwrapper/src/cpp_cppyy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,9 @@ namespace Cppyy {
RPY_EXPORTED
TCppMethod_t GetMethodTemplate(
TCppScope_t scope, const std::string& name, const std::string& proto);

RPY_EXPORTED
void GetClassOperators(Cppyy::TCppScope_t klass, const std::string& opname,
std::vector<TCppScope_t>& operators);
RPY_EXPORTED
TCppMethod_t GetGlobalOperator(
TCppType_t scope, const std::string& lc, const std::string& rc, const std::string& op);
Expand Down
Loading