From 2cdcc7322e18e5cc1ec3a8d7c6cbf3c766d42b06 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 13 Oct 2026 19:15:51 +0530 Subject: [PATCH 01/17] [cppinterop] Add `IsPackExpansionType`, `IsOperator` and `IsConversionOperator` (#1125) - `IsPackExpansionType` checks whether a type is a pack expansion (e.g. the type `Ts...` of a function parameter pack). - `IsOperator` checks whether a function or function template is an overloaded operator (e.g. `operator+`). - `IsConversionOperator` checks whether a function or function template is a conversion operator (e.g. `operator int()`). Like the other function predicates, `IsOperator` and `IsConversionOperator` look through using-shadow declarations and function templates. (cherry picked from commit e2aca65) --- .../CppInterOp/lib/CppInterOp/CppInterOp.cpp | 25 ++++++ .../CppInterOp/lib/CppInterOp/CppInterOp.td | 22 +++++ .../CppInterOp/FunctionReflectionTest.cpp | 86 +++++++++++++++++++ .../CppInterOp/TypeReflectionTest.cpp | 17 ++++ 4 files changed, 150 insertions(+) diff --git a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp index 30119c4282838..06e0fe70fa61d 100644 --- a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp +++ b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp @@ -2665,6 +2665,13 @@ bool IsTemplateParmType(ConstTypeRef TyRef) { return INTEROP_RETURN(QT->isTemplateTypeParmType()); } +bool IsPackExpansionType(ConstTypeRef TyRef) { + INTEROP_TRACE(TyRef); + clang::QualType QT = clang::QualType::getFromOpaquePtr(TyRef.data); + return INTEROP_RETURN( + llvm::isa_and_nonnull(QT.getTypePtrOrNull())); +} + std::string GetFunctionSignature(ConstFuncRef func) { INTEROP_TRACE(func); if (!func) @@ -3069,6 +3076,24 @@ bool IsExplicit(ConstFuncRef method) { return INTEROP_RETURN(false); } +bool IsOperator(ConstFuncRef func) { + INTEROP_TRACE(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); + if (const auto* FTD = llvm::dyn_cast_or_null(D)) + D = FTD->getTemplatedDecl(); + if (const auto* FD = llvm::dyn_cast_or_null(D)) + return INTEROP_RETURN(FD->isOverloadedOperator()); + return INTEROP_RETURN(false); +} + +bool IsConversionOperator(ConstFuncRef func) { + INTEROP_TRACE(func); + const auto* D = UnwrapUsingShadowToFunction(unwrap(func)); + if (const auto* FTD = llvm::dyn_cast_or_null(D)) + D = FTD->getTemplatedDecl(); + return INTEROP_RETURN(llvm::isa_and_nonnull(D)); +} + void* GetFunctionAddress(const char* mangled_name) { INTEROP_TRACE(mangled_name); auto& I = getInterp(); diff --git a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.td b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.td index 6aba48d089136..72d6b860d72c1 100644 --- a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.td +++ b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.td @@ -1503,6 +1503,14 @@ def IsTemplateParmType : CppInterOpAPI { ]; } +def IsPackExpansionType : CppInterOpAPI { + let Doc = "Checks if the given type is a pack expansion type (e.g. `Ts...`)."; + let ReturnType = "bool"; + let Args = [ + Arg<"ConstTypeRef", "TyRef"> + ]; +} + def IsFunction : CppInterOpAPI { let Doc = "Checks if the scope is a function."; let ReturnType = "bool"; @@ -1521,6 +1529,20 @@ def IsMethod : CppInterOpAPI { let Args = [Arg<"ConstFuncRef", "method">]; } +def IsOperator : CppInterOpAPI { + let Doc = [{Checks if the provided function (or function template) is an +overloaded operator.}]; + let ReturnType = "bool"; + let Args = [Arg<"ConstFuncRef", "func">]; +} + +def IsConversionOperator : CppInterOpAPI { + let Doc = [{Checks if the provided function (or function template) is a +conversion operator, e.g. `operator int()`.}]; + let ReturnType = "bool"; + let Args = [Arg<"ConstFuncRef", "func">]; +} + def IsPODType : CppInterOpAPI { let Doc = "Checks if the provided parameter is a Plain Old Data Type (POD)."; let ReturnType = "bool"; diff --git a/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp b/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp index 80fdec03f0f12..2fcfd6f085c86 100644 --- a/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp +++ b/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp @@ -4858,6 +4858,92 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_IsExplicitDeductionGuide) { "Wrapper"); } +TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_IsOperator) { + std::vector Decls, BaseDecls, DerivedDecls; + std::string code = R"( + struct OpBase { + OpBase& operator+=(const OpBase&) { return *this; } + operator int() const { return 0; } + void method() {} + }; + struct OpDerived : OpBase { + using OpBase::operator+=; + }; + bool operator==(const OpBase&, const OpBase&) { return true; } + template bool operator<(const T&, const OpBase&) { + return true; + } + template void free_template(T) {} + void free_function() {} + )"; + + GetAllTopLevelDecls(code, Decls); + ASSERT_EQ(Decls.size(), 6U); + GetAllSubDecls(Decls[0], BaseDecls, /*filter_implicitGenerated=*/true); + ASSERT_EQ(BaseDecls.size(), 3U); + GetAllSubDecls(Decls[1], DerivedDecls); + + EXPECT_TRUE(Cpp::IsOperator(BaseDecls[0])); // operator+= + EXPECT_FALSE(Cpp::IsOperator(BaseDecls[1])); // operator int + EXPECT_FALSE(Cpp::IsOperator(BaseDecls[2])); // method + + // using OpBase::operator+=; + Decl* Shadow = nullptr; + for (auto* D : DerivedDecls) + if (llvm::isa(D)) + Shadow = D; + ASSERT_TRUE(Shadow); + EXPECT_TRUE(Cpp::IsOperator(Shadow)); + + EXPECT_FALSE(Cpp::IsOperator(Decls[0])); // not a function + EXPECT_TRUE(Cpp::IsOperator(Decls[2])); // operator== + EXPECT_TRUE(Cpp::IsOperator(Decls[3])); // template operator< + EXPECT_FALSE(Cpp::IsOperator(Decls[4])); // free_template + EXPECT_FALSE(Cpp::IsOperator(Decls[5])); // free_function + EXPECT_FALSE(Cpp::IsOperator(nullptr)); +} + +TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_IsConversionOperator) { + std::vector Decls, BaseDecls, DerivedDecls; + std::string code = R"( + struct ConvBase { + operator int() const { return 0; } + explicit operator bool() const { return true; } + template operator T*() const { return nullptr; } + bool operator!() const { return false; } + void method() {} + }; + struct ConvDerived : ConvBase { + using ConvBase::operator int; + }; + void free_function() {} + )"; + + GetAllTopLevelDecls(code, Decls); + ASSERT_EQ(Decls.size(), 3U); + GetAllSubDecls(Decls[0], BaseDecls, /*filter_implicitGenerated=*/true); + ASSERT_EQ(BaseDecls.size(), 5U); + GetAllSubDecls(Decls[1], DerivedDecls); + + EXPECT_TRUE(Cpp::IsConversionOperator(BaseDecls[0])); // operator int + EXPECT_TRUE(Cpp::IsConversionOperator(BaseDecls[1])); // operator bool + EXPECT_TRUE(Cpp::IsConversionOperator(BaseDecls[2])); // operator T* + EXPECT_FALSE(Cpp::IsConversionOperator(BaseDecls[3])); // operator! + EXPECT_FALSE(Cpp::IsConversionOperator(BaseDecls[4])); // method + + // using ConvBase::operator int; + Decl* Shadow = nullptr; + for (auto* D : DerivedDecls) + if (llvm::isa(D)) + Shadow = D; + ASSERT_TRUE(Shadow); + EXPECT_TRUE(Cpp::IsConversionOperator(Shadow)); + + EXPECT_FALSE(Cpp::IsConversionOperator(Decls[0])); // not a function + EXPECT_FALSE(Cpp::IsConversionOperator(Decls[2])); // free_function + EXPECT_FALSE(Cpp::IsConversionOperator(nullptr)); +} + // C++23 "deducing this" (explicit object parameters, P0847R7): the object // parameter binds to the receiver, not the argument list. TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_DeducingThisIntrospection) { diff --git a/interpreter/CppInterOp/unittests/CppInterOp/TypeReflectionTest.cpp b/interpreter/CppInterOp/unittests/CppInterOp/TypeReflectionTest.cpp index 3fa88696c7d71..c6800106f39fd 100644 --- a/interpreter/CppInterOp/unittests/CppInterOp/TypeReflectionTest.cpp +++ b/interpreter/CppInterOp/unittests/CppInterOp/TypeReflectionTest.cpp @@ -592,6 +592,23 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, TypeReflection_IsTemplateParmType) { EXPECT_FALSE(Cpp::IsTemplateParmType(Cpp::GetFunctionArgType(Decls[0], 2))); } +TYPED_TEST(CPPINTEROP_TEST_MODE, TypeReflection_IsPackExpansionType) { + std::vector Decls; + + std::string code = R"( + template void f(T t, Ts... ts) {} + template void g(const Ts&... ts) {} + )"; + + GetAllTopLevelDecls(code, Decls); + + EXPECT_FALSE(Cpp::IsPackExpansionType(Cpp::GetFunctionArgType(Decls[0], 0))); + EXPECT_TRUE(Cpp::IsPackExpansionType(Cpp::GetFunctionArgType(Decls[0], 1))); + EXPECT_TRUE(Cpp::IsPackExpansionType(Cpp::GetFunctionArgType(Decls[1], 0))); + EXPECT_FALSE(Cpp::IsPackExpansionType(Cpp::GetType("int"))); + EXPECT_FALSE(Cpp::IsPackExpansionType(nullptr)); +} + TYPED_TEST(CPPINTEROP_TEST_MODE, TypeReflection_IsSmartPtrType) { #if CLANG_VERSION_MAJOR == 20 && defined(CPPINTEROP_USE_CLING) && defined(_WIN32) GTEST_SKIP() << "Test fails with Cling on Windows"; From ec249a78595a4f5e4c9f66bf527594734765b0ef Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 13 Oct 2026 11:30:23 +0530 Subject: [PATCH 02/17] [cppinterop] Rework `BestOverloadFunctionMatch` for methods, operators and ambiguity - Non-static member functions and member function templates are added as method candidates. The object type is taken from the first entry of `arg_types`, and the remaining entries are the call arguments. This replaces the synthesized receiver that was only used for explicit object member functions. - New `is_operator` parameter (default `false`) resolves the candidates as an operator call (`CSK_Operator`). - New `ambiguous_candidates` out-parameter: when the call is ambiguous, the best viable candidates are appended to it and `nullptr` is returned. - A candidate brought in by a using-declaration (a UsingShadowDecl, as returned by e.g. GetClassMethods for `using Base::f;`) was silently ignored. It is now added with the UsingShadowDecl as its found decl, so Sema treats it as a member of the class the using-declaration is in, as C++ requires ([over.match.funcs]). - Add a test covering constructors, static member functions, their templates, using-declarations, non-function candidates, calls without an object and calls without a viable candidate. (cherry picked from commit eb14c12) --- .../CppInterOp/lib/CppInterOp/CppInterOp.cpp | 65 ++-- .../CppInterOp/lib/CppInterOp/CppInterOp.td | 17 +- .../CppInterOp/FunctionReflectionTest.cpp | 317 +++++++++++++----- 3 files changed, 294 insertions(+), 105 deletions(-) diff --git a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp index 06e0fe70fa61d..f5d96432c9d06 100644 --- a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp +++ b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp @@ -2857,8 +2857,11 @@ bool GetClassTemplatedMethods(const std::string& name, ConstDeclRef parent, FuncRef BestOverloadFunctionMatch(const std::vector& candidates, const std::vector& explicit_types, - const std::vector& arg_types) { - INTEROP_TRACE(candidates, explicit_types, arg_types); + const std::vector& arg_types, + std::vector& ambiguous_candidates, + bool is_operator) { + INTEROP_TRACE(candidates, explicit_types, arg_types, + INTEROP_OUT(ambiguous_candidates), is_operator); auto& S = getSema(); auto& C = S.getASTContext(); @@ -2874,6 +2877,7 @@ BestOverloadFunctionMatch(const std::vector& candidates, }; auto* Exprs = new WrapperExpr[arg_types.size()]; llvm::SmallVector Args; + Args.reserve(arg_types.size()); size_t idx = 0; for (auto i : arg_types) { @@ -2916,25 +2920,34 @@ BestOverloadFunctionMatch(const std::vector& candidates, Sema::SFINAETrap Trap(S, /*ForValidityCheck=*/true); OverloadCandidateSet Overloads( - Loc, OverloadCandidateSet::CandidateSetKind::CSK_Normal); + Loc, is_operator ? OverloadCandidateSet::CandidateSetKind::CSK_Operator + : OverloadCandidateSet::CandidateSetKind::CSK_Normal); + + // check if this function requires the this/self argument + auto IsCalledOnObject = [&Args](const CXXMethodDecl* CXXMD) { + return CXXMD && !Cpp::IsConstructor(CXXMD) && !CXXMD->isStatic() && + !Args.empty(); + }; for (auto i : candidates) { - auto* D = const_cast(unwrap(i)); - if (auto* FD = dyn_cast(D)) { - S.AddOverloadCandidate(FD, DeclAccessPair::make(FD, FD->getAccess()), - Args, Overloads); + auto* Found = const_cast(unwrap(i)); + auto FoundDeclAccessPair = + DeclAccessPair::make(cast(Found), Found->getAccess()); + auto* D = UnwrapUsingShadowToFunction(Found); + if (auto* CXXMD = dyn_cast(D); IsCalledOnObject(CXXMD)) { + S.AddMethodCandidate(FoundDeclAccessPair, Args.front()->getType(), + Args.front()->Classify(C), + llvm::ArrayRef(Args).slice(1), Overloads); + } else if (auto* FD = dyn_cast(D)) { + S.AddOverloadCandidate(FD, FoundDeclAccessPair, Args, Overloads); } else if (auto* FTD = dyn_cast(D)) { - auto* MD = dyn_cast(FTD->getTemplatedDecl()); - if (MD && MD->isExplicitObjectMemberFunction()) { - // The explicit object parameter isn't in Args, so deduce it via the - // method-template path with a synthesized receiver of the record type. - CXXRecordDecl* RD = MD->getParent(); - QualType ObjectType = compat::GetTypeFromDecl(RD); - OpaqueValueExpr ObjectExpr(SourceLocation::getFromRawEncoding(1), - ObjectType, ExprValueKind::VK_LValue); + if (auto* CXXMD = dyn_cast(FTD->getTemplatedDecl()); + IsCalledOnObject(CXXMD)) { S.AddMethodTemplateCandidate( - FTD, DeclAccessPair::make(FTD, FTD->getAccess()), RD, - &ExplicitTemplateArgs, ObjectType, ObjectExpr.Classify(C), Args, + FTD, FoundDeclAccessPair, + Args.front()->getType()->getAsCXXRecordDecl(), + &ExplicitTemplateArgs, Args.front()->getType(), + Args.front()->Classify(C), llvm::ArrayRef(Args).slice(1), Overloads); } else { // AddTemplateOverloadCandidate is causing a memory leak @@ -2942,17 +2955,23 @@ BestOverloadFunctionMatch(const std::vector& candidates, // call stack: AddTemplateOverloadCandidate -> MakeDeductionFailureInfo // source: // https://github.com/llvm/llvm-project/blob/release/19.x/clang/lib/Sema/SemaOverload.cpp#L731-L756 - S.AddTemplateOverloadCandidate( - FTD, DeclAccessPair::make(FTD, FTD->getAccess()), - &ExplicitTemplateArgs, Args, Overloads); + S.AddTemplateOverloadCandidate(FTD, FoundDeclAccessPair, + &ExplicitTemplateArgs, Args, Overloads); } } } OverloadCandidateSet::iterator Best; - Overloads.BestViableFunction(S, Loc, Best); - - FunctionDecl* Result = Best != Overloads.end() ? Best->Function : nullptr; + auto res = Overloads.BestViableFunction(S, Loc, Best); + FunctionDecl* Result = nullptr; + if (res == clang::OR_Success) { + Result = Best->Function; + } else if (res == clang::OR_Ambiguous) { + for (const auto& i : Overloads) { + if (i.Best) + ambiguous_candidates.emplace_back(i.Function); + } + } // If the winner is a not-yet-instantiated template specialization, force // the instantiation of its body now, with diagnostics suppressed, and diff --git a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.td b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.td index 72d6b860d72c1..d1a78998037d8 100644 --- a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.td +++ b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.td @@ -1211,13 +1211,24 @@ def BeginStdStreamCapture : CppInterOpAPI { } def BestOverloadFunctionMatch : CppInterOpAPI { - let Doc = [{Finds best overload match based on explicit template parameters -(if any) and argument types.}]; + let Doc = [{Finds the best overload match among \c candidates based on the +explicit template arguments (if any) and the argument types. + +For a non-static member function candidate, the first entry of \c arg_types +is the type of the object the function is called on, and the remaining +entries are the call arguments. With \c is_operator set, the call is +resolved as an operator call. + +\returns the best match, or nullptr if there is none. If the call is +ambiguous, the best viable candidates are appended to +\c ambiguous_candidates.}]; let ReturnType = "FuncRef"; let Args = [ Arg<"const std::vector&", "candidates">, Arg<"const std::vector&", "explicit_types">, - Arg<"const std::vector&", "arg_types"> + Arg<"const std::vector&", "arg_types">, + Arg<"std::vector&", "ambiguous_candidates">, + Arg<"bool", "is_operator", "false"> ]; } diff --git a/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp b/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp index 2fcfd6f085c86..3ee8e4b403586 100644 --- a/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp +++ b/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp @@ -869,11 +869,12 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionReturnType) { std::vector args = {C.IntTy.getAsOpaquePtr(), C.DoubleTy.getAsOpaquePtr()}; std::vector explicit_args; + std::vector ambiguous_candidates; std::vector candidates = {Decls[14]}; - EXPECT_EQ( - Cpp::GetTypeAsString(Cpp::GetFunctionReturnType( - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args))), - "RTTest_TemplatedList"); + EXPECT_EQ(Cpp::GetTypeAsString( + Cpp::GetFunctionReturnType(Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args, ambiguous_candidates))), + "RTTest_TemplatedList"); std::vector args2 = {C.DoubleTy.getAsOpaquePtr()}; EXPECT_EQ( @@ -2510,6 +2511,7 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, GetAllTopLevelDecls(code, Decls); EXPECT_EQ(Decls.size(), 1); + std::vector ambiguous_candidates; std::vector candidates; candidates.reserve(Decls.size()); for (auto* i : Decls) @@ -2530,15 +2532,16 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, C.IntTy.getAsOpaquePtr(), }; - Cpp::FuncRef fn0 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args0, args0); + Cpp::FuncRef fn0 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args0, args0, ambiguous_candidates); EXPECT_TRUE(fn0); - Cpp::FuncRef fn = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args1, args0); + Cpp::FuncRef fn = Cpp::BestOverloadFunctionMatch(candidates, explicit_args1, + args0, ambiguous_candidates); EXPECT_EQ(fn, fn0); - fn = Cpp::BestOverloadFunctionMatch(candidates, explicit_args2, args0); + fn = Cpp::BestOverloadFunctionMatch(candidates, explicit_args2, args0, + ambiguous_candidates); EXPECT_EQ(fn, fn0); fn = Cpp::FuncRef{Cpp::InstantiateTemplate(Decls[0], explicit_args1).data}; @@ -2588,6 +2591,7 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, )"; GetAllTopLevelDecls(code, Decls); + std::vector ambiguous_candidates; std::vector candidates; for (auto decl : Decls) @@ -2596,27 +2600,35 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, ASTContext& C = Interp->getCI()->getASTContext(); - std::vector args0; + Cpp::DeclRef ScopeKlass = Cpp::GetNamed("MyTemplatedMethodClass"); + EXPECT_TRUE(ScopeKlass); + + Cpp::TypeRef TypeKlass = Cpp::GetTypeFromScope(ScopeKlass); + EXPECT_TRUE(TypeKlass); + + std::vector args0 = {TypeKlass.data}; std::vector args1 = { - C.getLValueReferenceType(C.IntTy).getAsOpaquePtr()}; - std::vector args2 = {C.CharTy.getAsOpaquePtr(), C.FloatTy.getAsOpaquePtr()}; - std::vector args3 = {C.FloatTy.getAsOpaquePtr()}; + TypeKlass.data, C.getLValueReferenceType(C.IntTy).getAsOpaquePtr()}; + std::vector args2 = { + TypeKlass.data, C.CharTy.getAsOpaquePtr(), C.FloatTy.getAsOpaquePtr()}; + std::vector args3 = {TypeKlass.data, + C.FloatTy.getAsOpaquePtr()}; std::vector explicit_args0; std::vector explicit_args1 = {C.IntTy.getAsOpaquePtr()}; std::vector explicit_args2 = { {C.IntTy.getAsOpaquePtr(), "1"}, C.IntTy.getAsOpaquePtr()}; - Cpp::FuncRef func1 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args0, args1); - Cpp::FuncRef func2 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args1, args0); - Cpp::FuncRef func3 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args0, args2); - Cpp::FuncRef func4 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args1, args3); - Cpp::FuncRef func5 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args2, args3); + Cpp::FuncRef func1 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args0, args1, ambiguous_candidates); + Cpp::FuncRef func2 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args1, args0, ambiguous_candidates); + Cpp::FuncRef func3 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args0, args2, ambiguous_candidates); + Cpp::FuncRef func4 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args1, args3, ambiguous_candidates); + Cpp::FuncRef func5 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args2, args3, ambiguous_candidates); EXPECT_EQ(Cpp::GetFunctionSignature(func1), "template<> long MyTemplatedMethodClass::get_size(int &)"); @@ -2656,6 +2668,7 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, GetAllTopLevelDecls(code, Decls); std::vector candidates; + std::vector ambiguous_candidates; for (auto decl : Decls) if (Cpp::IsFunction(decl) || Cpp::IsTemplatedFunction(decl)) @@ -2678,16 +2691,16 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, std::vector explicit_args; - Cpp::FuncRef func1 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args1); - Cpp::FuncRef func2 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args2); - Cpp::FuncRef func3 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args3); - Cpp::FuncRef func4 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args4); - Cpp::FuncRef func5 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args5); + Cpp::FuncRef func1 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args1, ambiguous_candidates); + Cpp::FuncRef func2 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args2, ambiguous_candidates); + Cpp::FuncRef func3 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args3, ambiguous_candidates); + Cpp::FuncRef func4 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args4, ambiguous_candidates); + Cpp::FuncRef func5 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args5, ambiguous_candidates); EXPECT_EQ(Cpp::GetFunctionSignature(func1), "template<> void somefunc(int arg)"); @@ -2730,6 +2743,7 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, GetAllTopLevelDecls(code, Decls); std::vector candidates; + std::vector ambiguous_candidates; for (auto decl : Decls) if (Cpp::IsTemplatedFunction(decl)) @@ -2749,12 +2763,12 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, C.DoubleTy.getAsOpaquePtr()}; std::vector explicit_args; - Cpp::FuncRef func1 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args1); - Cpp::FuncRef func2 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args2); - Cpp::FuncRef func3 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args3); + Cpp::FuncRef func1 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args1, ambiguous_candidates, true); + Cpp::FuncRef func2 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args2, ambiguous_candidates, true); + Cpp::FuncRef func3 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args3, ambiguous_candidates, true); candidates.clear(); Cpp::GetOperator( @@ -2764,10 +2778,11 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, EXPECT_EQ(candidates.size(), 1); std::vector args4 = { + Cpp::GetVariableType(Cpp::GetNamed("a")).data, Cpp::GetVariableType(Cpp::GetNamed("a")).data}; - Cpp::FuncRef func4 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args, args4); + Cpp::FuncRef func4 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args, args4, ambiguous_candidates, true); EXPECT_EQ(Cpp::GetFunctionSignature(func1), "template<> A operator+(A lhs, A rhs)"); @@ -2838,6 +2853,7 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, GetAllTopLevelDecls(code, Decls); GetAllSubDecls(Decls[1], SubDecls); + std::vector ambiguous_candidates; std::vector candidates; for (auto i : SubDecls) { if ((Cpp::IsFunction(i) || Cpp::IsTemplatedFunction(i)) && @@ -2849,31 +2865,38 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, ASTContext& C = Interp->getCI()->getASTContext(); - std::vector args1 = {}; - std::vector args2 = {C.IntTy.getAsOpaquePtr()}; + Cpp::DeclRef ScopeB = Cpp::GetNamed("B"); + EXPECT_TRUE(ScopeB); + + Cpp::TypeRef TypeB = Cpp::GetTypeFromScope(ScopeB); + EXPECT_TRUE(TypeB); + + std::vector args1 = {TypeB.data}; + std::vector args2 = {TypeB.data, + C.IntTy.getAsOpaquePtr()}; std::vector args3 = { - Cpp::GetVariableType(Cpp::GetNamed("a")).data}; + TypeB.data, Cpp::GetVariableType(Cpp::GetNamed("a")).data}; std::vector args4 = { - Cpp::GetVariableType(Cpp::GetNamed("a")).data, + TypeB.data, Cpp::GetVariableType(Cpp::GetNamed("a")).data, Cpp::GetVariableType(Cpp::GetNamed("b")).data}; std::vector args5 = { - Cpp::GetVariableType(Cpp::GetNamed("a")).data, + TypeB.data, Cpp::GetVariableType(Cpp::GetNamed("a")).data, Cpp::GetVariableType(Cpp::GetNamed("a")).data}; std::vector explicit_args1; std::vector explicit_args2 = {C.IntTy.getAsOpaquePtr(), C.IntTy.getAsOpaquePtr()}; - Cpp::FuncRef func1 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args1, args1); - Cpp::FuncRef func2 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args1, args2); - Cpp::FuncRef func3 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args1, args3); - Cpp::FuncRef func4 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args1, args4); - Cpp::FuncRef func5 = - Cpp::BestOverloadFunctionMatch(candidates, explicit_args2, args5); + Cpp::FuncRef func1 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args1, args1, ambiguous_candidates); + Cpp::FuncRef func2 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args1, args2, ambiguous_candidates); + Cpp::FuncRef func3 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args1, args3, ambiguous_candidates); + Cpp::FuncRef func4 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args1, args4, ambiguous_candidates); + Cpp::FuncRef func5 = Cpp::BestOverloadFunctionMatch( + candidates, explicit_args2, args5, ambiguous_candidates); EXPECT_EQ(Cpp::GetFunctionSignature(func1), "void B::fn()"); EXPECT_EQ(Cpp::GetFunctionSignature(func2), @@ -2902,6 +2925,7 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, EXPECT_EQ(Decls.size(), 2); std::vector candidates; + std::vector ambiguous_candidates; candidates.push_back(Decls[1]); ASTContext& C = Interp->getCI()->getASTContext(); @@ -2919,8 +2943,8 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, C.getLValueReferenceType(C.IntTy).getAsOpaquePtr(), }; - Cpp::FuncRef callback = - Cpp::BestOverloadFunctionMatch(candidates, empty_templ_args, arg_types); + Cpp::FuncRef callback = Cpp::BestOverloadFunctionMatch( + candidates, empty_templ_args, arg_types, ambiguous_candidates); EXPECT_TRUE(callback); EXPECT_EQ(Cpp::GetFunctionSignature(callback), @@ -2928,6 +2952,127 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, "&>>(void (*callable)(double, int), double &args, int &args)"); } +TYPED_TEST(CPPINTEROP_TEST_MODE, + FunctionReflection_BestOverloadFunctionMatch6) { + std::vector Decls; + std::string code = R"( + struct A0 { + A0(int) {} + }; + + struct B0 { + B0(int) {} + }; + + struct C0 {}; + + void consume(A0) {} + void consume(B0) {} + void consume(C0) {} + )"; + GetAllTopLevelDecls(code, Decls); + EXPECT_EQ(Decls.size(), 6); + + std::vector ambiguous_candidates; + std::vector candidates; + for (auto i : Decls) { + if (Cpp::IsFunction(i)) + candidates.push_back(i); + } + EXPECT_EQ(candidates.size(), 3); + + Cpp::TypeRef TypeA = Cpp::GetTypeFromScope(Cpp::GetScope("A0")); + EXPECT_TRUE(TypeA); + Cpp::TypeRef TypeB = Cpp::GetTypeFromScope(Cpp::GetScope("B0")); + EXPECT_TRUE(TypeB); + + Cpp::FuncRef meth = Cpp::BestOverloadFunctionMatch( + candidates, {}, {Cpp::GetType("int").data}, ambiguous_candidates); + EXPECT_FALSE(meth); + EXPECT_EQ(ambiguous_candidates.size(), 2); + + // the following might fail due to ordering + EXPECT_EQ(Cpp::GetFunctionSignature(ambiguous_candidates[0]), + "void consume(A0)"); + EXPECT_EQ(Cpp::GetFunctionSignature(ambiguous_candidates[1]), + "void consume(B0)"); +} + +TYPED_TEST(CPPINTEROP_TEST_MODE, + FunctionReflection_BestOverloadFunctionMatch_CandidateKinds) { + std::vector Decls, SubDecls, DerivedDecls; + std::string code = R"( + struct BOKinds { + BOKinds(int) {} + template BOKinds(T*) {} + static int s(int) { return 0; } + template static T st(T t) { return t; } + int m(int) { return 0; } + int get() { return 0; } + }; + struct BOKindsDerived : BOKinds { + using BOKinds::m; + int m(double) { return 1; } + }; + int bo_var = 0; + )"; + GetAllTopLevelDecls(code, Decls); + ASSERT_EQ(Decls.size(), 3U); + GetAllSubDecls(Decls[0], SubDecls, /*filter_implicitGenerated=*/true); + ASSERT_EQ(SubDecls.size(), 6U); + GetAllSubDecls(Decls[1], DerivedDecls); + + Cpp::TypeRef Int = Cpp::GetType("int"); + Cpp::TypeRef Double = Cpp::GetType("double"); + Cpp::TypeRef KindsTy = Cpp::GetTypeFromScope(Decls[0]); + Cpp::TypeRef DerivedTy = Cpp::GetTypeFromScope(Decls[1]); + std::vector ambiguous_candidates; + auto Match = [&](std::vector candidates, + std::vector arg_types) { + return Cpp::GetFunctionSignature(Cpp::BestOverloadFunctionMatch( + candidates, {}, arg_types, ambiguous_candidates)); + }; + + // Constructors and static member functions take no object argument. + std::vector ctors = {SubDecls[0], SubDecls[1]}; + EXPECT_EQ(Match(ctors, {Int.data}), "BOKinds::BOKinds(int)"); + EXPECT_EQ(Match(ctors, {Cpp::GetPointerType(Double).data}), + "template<> BOKinds::BOKinds(double *)"); + std::vector statics = {SubDecls[2], SubDecls[3]}; + EXPECT_EQ(Match(statics, {Int.data}), "static int BOKinds::s(int)"); + EXPECT_EQ(Match(statics, {Double.data}), + "template<> static double BOKinds::st(double t)"); + + // No viable candidate. + EXPECT_FALSE(Cpp::BestOverloadFunctionMatch({SubDecls[2]}, {}, {KindsTy.data}, + ambiguous_candidates)); + EXPECT_TRUE(ambiguous_candidates.empty()); + + // A method brought in by a using-declaration is a candidate too. + Decl* Shadow = nullptr; + Decl* DerivedM = nullptr; + for (auto* D : DerivedDecls) { + if (llvm::isa(D)) + Shadow = D; + else if (Cpp::IsMethod(D) && !D->isImplicit()) + DerivedM = D; + } + ASSERT_TRUE(Shadow); + ASSERT_TRUE(DerivedM); + EXPECT_EQ(Match({Shadow, DerivedM}, {DerivedTy.data, Int.data}), + "int BOKinds::m(int)"); + EXPECT_EQ(Match({Shadow, DerivedM}, {DerivedTy.data, Double.data}), + "int BOKindsDerived::m(double)"); + + // Candidates that are not functions are ignored. + EXPECT_EQ(Match({Decls[2], SubDecls[2]}, {Int.data}), + "static int BOKinds::s(int)"); + + // Without argument types there is no object to call a method on; the + // method is resolved with an implicit object argument. + EXPECT_EQ(Match({SubDecls[5]}, {}), "int BOKinds::get()"); +} + TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_IsPublicMethod) { std::vector Decls, SubDecls; std::string code = R"( @@ -3504,6 +3649,7 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { FCI_TOperatorCtor.Invoke((void*)&toperator); EXPECT_TRUE(toperator); + std::vector ambiguous_candidates; std::vector operators; Cpp::GetOperator(TOperator, Cpp::Operator::OP_Less, operators); EXPECT_EQ(operators.size(), 1); @@ -3553,7 +3699,8 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { Cpp::Operator::OP_Plus, operators); EXPECT_EQ(operators.size(), 1); Cpp::FuncRef kop = Cpp::BestOverloadFunctionMatch(operators, empty_templ_args, - {K1.data, K2.data}); + {K1.data, K2.data}, + ambiguous_candidates, true); auto chrono_op_fn_callable = Cpp::MakeFunctionCallable(kop); EXPECT_EQ(chrono_op_fn_callable.getKind(), Cpp::JitCall::kGenericCall); @@ -3632,8 +3779,8 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { EXPECT_TRUE(p); Cpp::FuncRef fn = Cpp::BestOverloadFunctionMatch( - unresolved_candidate_methods, {{Cpp::GetType("int").data, "0"}}, - {p.data}); + unresolved_candidate_methods, {{Cpp::GetType("int").data, "0"}}, {p.data}, + ambiguous_candidates); EXPECT_TRUE(fn); auto fn_callable = Cpp::MakeFunctionCallable(fn); @@ -3653,7 +3800,8 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { Cpp::FuncRef call_move = Cpp::BestOverloadFunctionMatch( unresolved_candidate_methods, {}, - {Cpp::GetReferencedType(Cpp::GetType("int"), true).data}); + {Cpp::GetReferencedType(Cpp::GetType("int"), true).data}, + ambiguous_candidates); EXPECT_TRUE(call_move); auto call_move_callable = Cpp::MakeFunctionCallable(call_move); @@ -3668,7 +3816,8 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { EXPECT_EQ(unresolved_candidate_methods.size(), 1); Cpp::FuncRef instantiation_in_host = Cpp::BestOverloadFunctionMatch( - unresolved_candidate_methods, {Cpp::GetType("int").data}, {}); + unresolved_candidate_methods, {Cpp::GetType("int").data}, {}, + ambiguous_candidates); EXPECT_TRUE(instantiation_in_host); Cpp::JitCall instantiation_in_host_callable = @@ -3677,7 +3826,8 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { Cpp::JitCall::kGenericCall); instantiation_in_host = Cpp::BestOverloadFunctionMatch( - unresolved_candidate_methods, {Cpp::GetType("double").data}, {}); + unresolved_candidate_methods, {Cpp::GetType("double").data}, {}, + ambiguous_candidates); EXPECT_TRUE(instantiation_in_host); Cpp::BeginStdStreamCapture(Cpp::CaptureStreamKind::kStdErr); @@ -3707,7 +3857,8 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { Cpp::FuncRef tuple_tuple = Cpp::BestOverloadFunctionMatch( unresolved_candidate_methods, {}, {Cpp::GetVariableType(Cpp::GetNamed("tuple_one")).data, - Cpp::GetVariableType(Cpp::GetNamed("tuple_two")).data}); + Cpp::GetVariableType(Cpp::GetNamed("tuple_two")).data}, + ambiguous_candidates); EXPECT_TRUE(tuple_tuple); auto tuple_tuple_callable = Cpp::MakeFunctionCallable(tuple_tuple); @@ -3747,7 +3898,8 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { Cpp::FuncRef consume = Cpp::BestOverloadFunctionMatch( unresolved_candidate_methods, {}, - {Cpp::GetVariableType(Cpp::GetNamed("consumable")).data}); + {Cpp::GetVariableType(Cpp::GetNamed("consumable")).data}, + ambiguous_candidates); EXPECT_TRUE(consume); auto consume_callable = Cpp::MakeFunctionCallable(consume); @@ -3783,11 +3935,14 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { Cpp::GetOperator(KlassProduct_int, Cpp::Operator::OP_Star, operators); EXPECT_EQ(operators.size(), 2); - Cpp::FuncRef op2 = Cpp::BestOverloadFunctionMatch( - operators, {}, {{Cpp::GetTypeFromScope(KlassProduct_float).data}}); - EXPECT_TRUE(op2); + Cpp::FuncRef op_fn = Cpp::BestOverloadFunctionMatch( + operators, {}, + {Cpp::GetTypeFromScope(KlassProduct_int).data, + Cpp::GetTypeFromScope(KlassProduct_float).data}, + ambiguous_candidates, true); + EXPECT_TRUE(op_fn); - auto op_callable = Cpp::MakeFunctionCallable(op2); + auto op_callable = Cpp::MakeFunctionCallable(op_fn); EXPECT_EQ(op_callable.getKind(), Cpp::JitCall::kGenericCall); Cpp::Declare(R"( @@ -3861,9 +4016,11 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_GetFunctionCallWrapper) { unresolved_candidate_methods); EXPECT_EQ(unresolved_candidate_methods.size(), 1); + ambiguous_candidates.clear(); Cpp::FuncRef callback_func = Cpp::BestOverloadFunctionMatch( unresolved_candidate_methods, {}, - {Cpp::GetVariableType(Cpp::GetNamed("fn_ptr")).data}); + {Cpp::GetVariableType(Cpp::GetNamed("fn_ptr")).data}, + ambiguous_candidates); EXPECT_TRUE(callback_func); auto callback_callable = Cpp::MakeFunctionCallable(callback_func); @@ -5100,17 +5257,17 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, GetAllSubDecls(Decls[0], SubDecls, /*filter_implicitGenerated=*/true); std::vector candidates; + std::vector ambiguous_candidates; for (auto* decl : SubDecls) if (Cpp::IsTemplatedFunction(decl)) candidates.push_back((Cpp::FuncRef)decl); ASSERT_EQ(candidates.size(), (size_t)1); - // No explicit template/call args: `Self` deduces from the synthesized - // receiver (only the AddMethodTemplateCandidate path can do this). std::vector no_explicit_args; - std::vector no_args; - Cpp::FuncRef matched = - Cpp::BestOverloadFunctionMatch(candidates, no_explicit_args, no_args); + Cpp::TypeRef widget_type = Cpp::GetTypeFromScope(Cpp::GetScope("TWidget")); + std::vector args = {widget_type.data}; + Cpp::FuncRef matched = Cpp::BestOverloadFunctionMatch( + candidates, no_explicit_args, args, ambiguous_candidates); ASSERT_TRUE(matched); EXPECT_NE(Cpp::GetFunctionSignature(matched).find("via"), std::string::npos); } @@ -5134,15 +5291,17 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, GetAllSubDecls(Decls[0], SubDecls, /*filter_implicitGenerated=*/true); std::vector candidates; + std::vector ambiguous_candidates; for (auto* decl : SubDecls) if (Cpp::IsTemplatedFunction(decl)) candidates.push_back((Cpp::FuncRef)decl); ASSERT_EQ(candidates.size(), (size_t)1); std::vector no_explicit_args; - std::vector no_args; - Cpp::FuncRef matched = - Cpp::BestOverloadFunctionMatch(candidates, no_explicit_args, no_args); + Cpp::TypeRef aawidget_type = Cpp::GetTypeFromScope(Cpp::GetScope("AAWidget")); + std::vector args = {aawidget_type.data}; + Cpp::FuncRef matched = Cpp::BestOverloadFunctionMatch( + candidates, no_explicit_args, args, ambiguous_candidates); ASSERT_TRUE(matched); EXPECT_NE(Cpp::GetFunctionSignature(matched).find("get"), std::string::npos); } From 7831d60670af884890b851ee264c22bc50162cdf Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Wed, 14 Oct 2026 12:04:41 +0530 Subject: [PATCH 03/17] [cppinterop] Refactor according to code review (cherry picked from commit 76c198c) --- .../CppInterOp/lib/CppInterOp/CppInterOp.cpp | 4 ++-- .../unittests/CppInterOp/FunctionReflectionTest.cpp | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp index f5d96432c9d06..97fb7db70b212 100644 --- a/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp +++ b/interpreter/CppInterOp/lib/CppInterOp/CppInterOp.cpp @@ -2859,7 +2859,7 @@ BestOverloadFunctionMatch(const std::vector& candidates, const std::vector& explicit_types, const std::vector& arg_types, std::vector& ambiguous_candidates, - bool is_operator) { + bool is_operator /*= false*/) { INTEROP_TRACE(candidates, explicit_types, arg_types, INTEROP_OUT(ambiguous_candidates), is_operator); auto& S = getSema(); @@ -2930,7 +2930,7 @@ BestOverloadFunctionMatch(const std::vector& candidates, }; for (auto i : candidates) { - auto* Found = const_cast(unwrap(i)); + auto* Found = unwrap(i); auto FoundDeclAccessPair = DeclAccessPair::make(cast(Found), Found->getAccess()); auto* D = UnwrapUsingShadowToFunction(Found); diff --git a/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp b/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp index 3ee8e4b403586..bbba8027274ad 100644 --- a/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp +++ b/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include "gtest/gtest.h" @@ -2975,9 +2976,9 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, std::vector ambiguous_candidates; std::vector candidates; - for (auto i : Decls) { + for (auto* i : Decls) { if (Cpp::IsFunction(i)) - candidates.push_back(i); + candidates.emplace_back(i); } EXPECT_EQ(candidates.size(), 3); @@ -3000,7 +3001,9 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, TYPED_TEST(CPPINTEROP_TEST_MODE, FunctionReflection_BestOverloadFunctionMatch_CandidateKinds) { - std::vector Decls, SubDecls, DerivedDecls; + std::vector Decls; + std::vector SubDecls; + std::vector DerivedDecls; std::string code = R"( struct BOKinds { BOKinds(int) {} @@ -3027,8 +3030,8 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, Cpp::TypeRef KindsTy = Cpp::GetTypeFromScope(Decls[0]); Cpp::TypeRef DerivedTy = Cpp::GetTypeFromScope(Decls[1]); std::vector ambiguous_candidates; - auto Match = [&](std::vector candidates, - std::vector arg_types) { + auto Match = [&](const std::vector& candidates, + const std::vector& arg_types) { return Cpp::GetFunctionSignature(Cpp::BestOverloadFunctionMatch( candidates, {}, arg_types, ambiguous_candidates)); }; From 3890823c838e89ac41d8e0bfaf057faf5d7f48ed Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Wed, 7 Oct 2026 18:28:08 +0200 Subject: [PATCH 04/17] [cppjit] [cpyrt] Generate the PyStrings cache from a single X-macro list (compiler-ressearch/cppjit#141) Each cached string was spelled out four times: the extern declaration, the definition, the interning in CreatePyStrings() and the release in DestroyPyStrings(). The lists had drifted: gAssign, gCastCpp, gRepr, gCppRepr, gStr and gCppStr were never released. List every string once in CPYRT_PYSTRINGS and expand the declarations, definitions, creation and cleanup from it. Call sites are unchanged. gEmptyString is now interned like the others, which yields the same object since CPython keeps a single empty string. (cherry picked from commit 13f4c58) --- .../pyroot/cppjit/src/cpyrt/PyStrings.cxx | 290 +----------------- bindings/pyroot/cppjit/src/cpyrt/PyStrings.h | 155 +++++----- 2 files changed, 92 insertions(+), 353 deletions(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/PyStrings.cxx b/bindings/pyroot/cppjit/src/cpyrt/PyStrings.cxx index a0e957f3787f9..9745acaec32d0 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/PyStrings.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/PyStrings.cxx @@ -5,164 +5,20 @@ using namespace cppjit; #include "PyStrings.h" //- data _____________________________________________________________________ -PyObject* cpyrt::PyStrings::gAssign = nullptr; -PyObject* cpyrt::PyStrings::gBases = nullptr; -PyObject* cpyrt::PyStrings::gBase = nullptr; -PyObject* cpyrt::PyStrings::gContains = nullptr; -PyObject* cpyrt::PyStrings::gCopy = nullptr; -PyObject* cpyrt::PyStrings::gCppBool = nullptr; -PyObject* cpyrt::PyStrings::gCppName = nullptr; -PyObject* cpyrt::PyStrings::gAnnotations = nullptr; -PyObject* cpyrt::PyStrings::gCastCpp = nullptr; -PyObject* cpyrt::PyStrings::gCType = nullptr; -PyObject* cpyrt::PyStrings::gDeref = nullptr; -PyObject* cpyrt::PyStrings::gPreInc = nullptr; -PyObject* cpyrt::PyStrings::gPostInc = nullptr; -PyObject* cpyrt::PyStrings::gDict = nullptr; -PyObject* cpyrt::PyStrings::gEmptyString = nullptr; -PyObject* cpyrt::PyStrings::gEq = nullptr; -PyObject* cpyrt::PyStrings::gFollow = nullptr; -PyObject* cpyrt::PyStrings::gHasValue = nullptr; -PyObject* cpyrt::PyStrings::gGetItem = nullptr; -PyObject* cpyrt::PyStrings::gGetNoCheck = nullptr; -PyObject* cpyrt::PyStrings::gSetItem = nullptr; -PyObject* cpyrt::PyStrings::gInit = nullptr; -PyObject* cpyrt::PyStrings::gIter = nullptr; -PyObject* cpyrt::PyStrings::gLen = nullptr; -PyObject* cpyrt::PyStrings::gLifeLine = nullptr; -PyObject* cpyrt::PyStrings::gModule = nullptr; -PyObject* cpyrt::PyStrings::gMRO = nullptr; -PyObject* cpyrt::PyStrings::gName = nullptr; -PyObject* cpyrt::PyStrings::gNe = nullptr; -PyObject* cpyrt::PyStrings::gRepr = nullptr; -PyObject* cpyrt::PyStrings::gCppRepr = nullptr; -PyObject* cpyrt::PyStrings::gStr = nullptr; -PyObject* cpyrt::PyStrings::gCppStr = nullptr; -PyObject* cpyrt::PyStrings::gTypeCode = nullptr; -PyObject* cpyrt::PyStrings::gCTypesType = nullptr; - -PyObject* cpyrt::PyStrings::gUnderlying = nullptr; -PyObject* cpyrt::PyStrings::gRealInit = nullptr; - -PyObject* cpyrt::PyStrings::gAdd = nullptr; -PyObject* cpyrt::PyStrings::gSub = nullptr; -PyObject* cpyrt::PyStrings::gMul = nullptr; -PyObject* cpyrt::PyStrings::gDiv = nullptr; - -PyObject* cpyrt::PyStrings::gLShift = nullptr; -PyObject* cpyrt::PyStrings::gLShiftC = nullptr; - -PyObject* cpyrt::PyStrings::gAt = nullptr; -PyObject* cpyrt::PyStrings::gBegin = nullptr; -PyObject* cpyrt::PyStrings::gEnd = nullptr; -PyObject* cpyrt::PyStrings::gFirst = nullptr; -PyObject* cpyrt::PyStrings::gSecond = nullptr; -PyObject* cpyrt::PyStrings::gSize = nullptr; -PyObject* cpyrt::PyStrings::gTemplate = nullptr; -PyObject* cpyrt::PyStrings::gVectorAt = nullptr; -PyObject* cpyrt::PyStrings::gInsert = nullptr; -PyObject* cpyrt::PyStrings::gValueType = nullptr; -PyObject* cpyrt::PyStrings::gValueTypePtr = nullptr; -PyObject* cpyrt::PyStrings::gValueSize = nullptr; - -PyObject* cpyrt::PyStrings::gCppReal = nullptr; -PyObject* cpyrt::PyStrings::gCppImag = nullptr; - -PyObject* cpyrt::PyStrings::gThisModule = nullptr; - -PyObject* cpyrt::PyStrings::gDispInit = nullptr; -PyObject* cpyrt::PyStrings::gDispGet = nullptr; - -PyObject* cpyrt::PyStrings::gExPythonize = nullptr; -PyObject* cpyrt::PyStrings::gPythonize = nullptr; - -PyObject* cpyrt::PyStrings::gArray = nullptr; -PyObject* cpyrt::PyStrings::gDType = nullptr; -PyObject* cpyrt::PyStrings::gFromBuffer = nullptr; +#define CPYRT_DEFINE_PYSTRING(var, str) \ + PyObject* cpyrt::PyStrings::var = nullptr; +CPYRT_PYSTRINGS(CPYRT_DEFINE_PYSTRING) +#undef CPYRT_DEFINE_PYSTRING //----------------------------------------------------------------------------- -#define CPPJIT_INITIALIZE_STRING(var, str) \ - if (!(PyStrings::var = PyUnicode_InternFromString((char*)#str))) \ - return false - bool cpyrt::CreatePyStrings() { // Build cache of commonly used python strings (the cache is python intern, so // all strings are shared python-wide, not just in cppjit). - CPPJIT_INITIALIZE_STRING(gAssign, __assign__); - CPPJIT_INITIALIZE_STRING(gBases, __bases__); - CPPJIT_INITIALIZE_STRING(gBase, __base__); - CPPJIT_INITIALIZE_STRING(gContains, contains); - CPPJIT_INITIALIZE_STRING(gCopy, copy); - CPPJIT_INITIALIZE_STRING(gCppBool, __cpp_bool__); - CPPJIT_INITIALIZE_STRING(gCppName, __cpp_name__); - CPPJIT_INITIALIZE_STRING(gAnnotations, __annotations__); - CPPJIT_INITIALIZE_STRING(gCastCpp, __cast_cpp__); - CPPJIT_INITIALIZE_STRING(gCType, __ctype__); - CPPJIT_INITIALIZE_STRING(gDeref, __deref__); - CPPJIT_INITIALIZE_STRING(gPreInc, __preinc__); - CPPJIT_INITIALIZE_STRING(gPostInc, __postinc__); - CPPJIT_INITIALIZE_STRING(gDict, __dict__); - if (!(PyStrings::gEmptyString = PyUnicode_FromString((char*)""))) +#define CPYRT_CREATE_PYSTRING(var, str) \ + if (!(PyStrings::var = PyUnicode_InternFromString(str))) \ return false; - CPPJIT_INITIALIZE_STRING(gEq, __eq__); - CPPJIT_INITIALIZE_STRING(gFollow, __follow__); - CPPJIT_INITIALIZE_STRING(gHasValue, has_value); - CPPJIT_INITIALIZE_STRING(gGetItem, __getitem__); - CPPJIT_INITIALIZE_STRING(gGetNoCheck, _getitem__unchecked); - CPPJIT_INITIALIZE_STRING(gSetItem, __setitem__); - CPPJIT_INITIALIZE_STRING(gInit, __init__); - CPPJIT_INITIALIZE_STRING(gIter, __iter__); - CPPJIT_INITIALIZE_STRING(gLen, __len__); - CPPJIT_INITIALIZE_STRING(gLifeLine, __lifeline); - CPPJIT_INITIALIZE_STRING(gModule, __module__); - CPPJIT_INITIALIZE_STRING(gMRO, __mro__); - CPPJIT_INITIALIZE_STRING(gName, __name__); - CPPJIT_INITIALIZE_STRING(gNe, __ne__); - CPPJIT_INITIALIZE_STRING(gRepr, __repr__); - CPPJIT_INITIALIZE_STRING(gCppRepr, __cpp_repr); - CPPJIT_INITIALIZE_STRING(gStr, __str__); - CPPJIT_INITIALIZE_STRING(gCppStr, __cpp_str); - CPPJIT_INITIALIZE_STRING(gTypeCode, typecode); - CPPJIT_INITIALIZE_STRING(gCTypesType, _type_); - - CPPJIT_INITIALIZE_STRING(gUnderlying, __underlying); - CPPJIT_INITIALIZE_STRING(gRealInit, __real_init); - - CPPJIT_INITIALIZE_STRING(gAdd, __add__); - CPPJIT_INITIALIZE_STRING(gSub, __sub__); - CPPJIT_INITIALIZE_STRING(gMul, __mul__); - CPPJIT_INITIALIZE_STRING(gDiv, __truediv__); - - CPPJIT_INITIALIZE_STRING(gLShift, __lshift__); - CPPJIT_INITIALIZE_STRING(gLShiftC, __lshiftc__); - - CPPJIT_INITIALIZE_STRING(gAt, at); - CPPJIT_INITIALIZE_STRING(gBegin, begin); - CPPJIT_INITIALIZE_STRING(gEnd, end); - CPPJIT_INITIALIZE_STRING(gFirst, first); - CPPJIT_INITIALIZE_STRING(gSecond, second); - CPPJIT_INITIALIZE_STRING(gSize, size); - CPPJIT_INITIALIZE_STRING(gTemplate, Template); - CPPJIT_INITIALIZE_STRING(gVectorAt, _vector__at); - CPPJIT_INITIALIZE_STRING(gInsert, insert); - CPPJIT_INITIALIZE_STRING(gValueType, value_type); - CPPJIT_INITIALIZE_STRING(gValueTypePtr, _value_type); - CPPJIT_INITIALIZE_STRING(gValueSize, value_size); - - CPPJIT_INITIALIZE_STRING(gCppReal, __cpp_real); - CPPJIT_INITIALIZE_STRING(gCppImag, __cpp_imag); - - CPPJIT_INITIALIZE_STRING(gThisModule, cppjit); - - CPPJIT_INITIALIZE_STRING(gDispInit, _init_dispatchptr); - CPPJIT_INITIALIZE_STRING(gDispGet, _get_dispatch); - - CPPJIT_INITIALIZE_STRING(gExPythonize, __cppjit_explicit_pythonize__); - CPPJIT_INITIALIZE_STRING(gPythonize, __cppjit_pythonize__); - - CPPJIT_INITIALIZE_STRING(gArray, __array__); - CPPJIT_INITIALIZE_STRING(gDType, dtype); - CPPJIT_INITIALIZE_STRING(gFromBuffer, frombuffer); + CPYRT_PYSTRINGS(CPYRT_CREATE_PYSTRING) +#undef CPYRT_CREATE_PYSTRING return true; } @@ -170,133 +26,9 @@ bool cpyrt::CreatePyStrings() { //----------------------------------------------------------------------------- PyObject* cpyrt::DestroyPyStrings() { // Remove all cached python strings. - Py_DECREF(PyStrings::gBases); - PyStrings::gBases = nullptr; - Py_DECREF(PyStrings::gBase); - PyStrings::gBase = nullptr; - Py_DECREF(PyStrings::gContains); - PyStrings::gContains = nullptr; - Py_DECREF(PyStrings::gCopy); - PyStrings::gCopy = nullptr; - Py_DECREF(PyStrings::gCppBool); - PyStrings::gCppBool = nullptr; - Py_DECREF(PyStrings::gCppName); - PyStrings::gCppName = nullptr; - Py_DECREF(PyStrings::gAnnotations); - PyStrings::gAnnotations = nullptr; - Py_DECREF(PyStrings::gCType); - PyStrings::gCType = nullptr; - Py_DECREF(PyStrings::gDeref); - PyStrings::gDeref = nullptr; - Py_DECREF(PyStrings::gPreInc); - PyStrings::gPreInc = nullptr; - Py_DECREF(PyStrings::gPostInc); - PyStrings::gPostInc = nullptr; - Py_DECREF(PyStrings::gDict); - PyStrings::gDict = nullptr; - Py_DECREF(PyStrings::gEmptyString); - PyStrings::gEmptyString = nullptr; - Py_DECREF(PyStrings::gEq); - PyStrings::gEq = nullptr; - Py_DECREF(PyStrings::gFollow); - PyStrings::gFollow = nullptr; - Py_DECREF(PyStrings::gHasValue); - PyStrings::gHasValue = nullptr; - Py_DECREF(PyStrings::gGetItem); - PyStrings::gGetItem = nullptr; - Py_DECREF(PyStrings::gGetNoCheck); - PyStrings::gGetNoCheck = nullptr; - Py_DECREF(PyStrings::gSetItem); - PyStrings::gSetItem = nullptr; - Py_DECREF(PyStrings::gInit); - PyStrings::gInit = nullptr; - Py_DECREF(PyStrings::gIter); - PyStrings::gIter = nullptr; - Py_DECREF(PyStrings::gLen); - PyStrings::gLen = nullptr; - Py_DECREF(PyStrings::gLifeLine); - PyStrings::gLifeLine = nullptr; - Py_DECREF(PyStrings::gModule); - PyStrings::gModule = nullptr; - Py_DECREF(PyStrings::gMRO); - PyStrings::gMRO = nullptr; - Py_DECREF(PyStrings::gName); - PyStrings::gName = nullptr; - Py_DECREF(PyStrings::gNe); - PyStrings::gNe = nullptr; - Py_DECREF(PyStrings::gTypeCode); - PyStrings::gTypeCode = nullptr; - Py_DECREF(PyStrings::gCTypesType); - PyStrings::gCTypesType = nullptr; - - Py_DECREF(PyStrings::gUnderlying); - PyStrings::gUnderlying = nullptr; - Py_DECREF(PyStrings::gRealInit); - PyStrings::gRealInit = nullptr; - - Py_DECREF(PyStrings::gAdd); - PyStrings::gAdd = nullptr; - Py_DECREF(PyStrings::gSub); - PyStrings::gSub = nullptr; - Py_DECREF(PyStrings::gMul); - PyStrings::gMul = nullptr; - Py_DECREF(PyStrings::gDiv); - PyStrings::gDiv = nullptr; - - Py_DECREF(PyStrings::gLShift); - PyStrings::gLShift = nullptr; - Py_DECREF(PyStrings::gLShiftC); - PyStrings::gLShiftC = nullptr; - - Py_DECREF(PyStrings::gAt); - PyStrings::gAt = nullptr; - Py_DECREF(PyStrings::gBegin); - PyStrings::gBegin = nullptr; - Py_DECREF(PyStrings::gEnd); - PyStrings::gEnd = nullptr; - Py_DECREF(PyStrings::gFirst); - PyStrings::gFirst = nullptr; - Py_DECREF(PyStrings::gSecond); - PyStrings::gSecond = nullptr; - Py_DECREF(PyStrings::gSize); - PyStrings::gSize = nullptr; - Py_DECREF(PyStrings::gTemplate); - PyStrings::gTemplate = nullptr; - Py_DECREF(PyStrings::gVectorAt); - PyStrings::gVectorAt = nullptr; - Py_DECREF(PyStrings::gInsert); - PyStrings::gInsert = nullptr; - Py_DECREF(PyStrings::gValueType); - PyStrings::gValueType = nullptr; - Py_DECREF(PyStrings::gValueTypePtr); - PyStrings::gValueTypePtr = nullptr; - Py_DECREF(PyStrings::gValueSize); - PyStrings::gValueSize = nullptr; - - Py_DECREF(PyStrings::gCppReal); - PyStrings::gCppReal = nullptr; - Py_DECREF(PyStrings::gCppImag); - PyStrings::gCppImag = nullptr; - - Py_DECREF(PyStrings::gThisModule); - PyStrings::gThisModule = nullptr; - - Py_DECREF(PyStrings::gDispInit); - PyStrings::gDispInit = nullptr; - Py_DECREF(PyStrings::gDispGet); - PyStrings::gDispGet = nullptr; - - Py_DECREF(PyStrings::gExPythonize); - PyStrings::gExPythonize = nullptr; - Py_DECREF(PyStrings::gPythonize); - PyStrings::gPythonize = nullptr; - - Py_DECREF(PyStrings::gArray); - PyStrings::gArray = nullptr; - Py_DECREF(PyStrings::gDType); - PyStrings::gDType = nullptr; - Py_DECREF(PyStrings::gFromBuffer); - PyStrings::gFromBuffer = nullptr; +#define CPYRT_DESTROY_PYSTRING(var, str) Py_CLEAR(PyStrings::var); + CPYRT_PYSTRINGS(CPYRT_DESTROY_PYSTRING) +#undef CPYRT_DESTROY_PYSTRING Py_RETURN_NONE; } diff --git a/bindings/pyroot/cppjit/src/cpyrt/PyStrings.h b/bindings/pyroot/cppjit/src/cpyrt/PyStrings.h index 64724f8b2420c..6f0d3c8dc6f9d 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/PyStrings.h +++ b/bindings/pyroot/cppjit/src/cpyrt/PyStrings.h @@ -5,82 +5,89 @@ namespace cppjit::cpyrt { // python strings kept for performance reasons -namespace PyStrings { - -extern PyObject* gAssign; -extern PyObject* gBases; -extern PyObject* gBase; -extern PyObject* gContains; -extern PyObject* gCopy; -extern PyObject* gCppBool; -extern PyObject* gCppName; -extern PyObject* gAnnotations; -extern PyObject* gCastCpp; -extern PyObject* gCType; -extern PyObject* gDeref; -extern PyObject* gPreInc; -extern PyObject* gPostInc; -extern PyObject* gDict; -extern PyObject* gEmptyString; -extern PyObject* gEq; -extern PyObject* gFollow; -extern PyObject* gHasValue; -extern PyObject* gGetItem; -extern PyObject* gGetNoCheck; -extern PyObject* gSetItem; -extern PyObject* gInit; -extern PyObject* gIter; -extern PyObject* gLen; -extern PyObject* gLifeLine; -extern PyObject* gModule; -extern PyObject* gMRO; -extern PyObject* gName; -extern PyObject* gNe; -extern PyObject* gRepr; -extern PyObject* gCppRepr; -extern PyObject* gStr; -extern PyObject* gCppStr; -extern PyObject* gTypeCode; -extern PyObject* gCTypesType; - -extern PyObject* gUnderlying; -extern PyObject* gRealInit; - -extern PyObject* gAdd; -extern PyObject* gSub; -extern PyObject* gMul; -extern PyObject* gDiv; - -extern PyObject* gLShift; -extern PyObject* gLShiftC; +// X(variable, string): each entry declares PyStrings::variable, holding the +// interned python string for "string" +#define CPYRT_PYSTRINGS(X) \ + X(gAssign, "__assign__") \ + X(gBases, "__bases__") \ + X(gBase, "__base__") \ + X(gContains, "contains") \ + X(gCopy, "copy") \ + X(gCppBool, "__cpp_bool__") \ + X(gCppName, "__cpp_name__") \ + X(gAnnotations, "__annotations__") \ + X(gCastCpp, "__cast_cpp__") \ + X(gCType, "__ctype__") \ + X(gDeref, "__deref__") \ + X(gPreInc, "__preinc__") \ + X(gPostInc, "__postinc__") \ + X(gDict, "__dict__") \ + X(gEmptyString, "") \ + X(gEq, "__eq__") \ + X(gFollow, "__follow__") \ + X(gHasValue, "has_value") \ + X(gGetItem, "__getitem__") \ + X(gGetNoCheck, "_getitem__unchecked") \ + X(gSetItem, "__setitem__") \ + X(gInit, "__init__") \ + X(gIter, "__iter__") \ + X(gLen, "__len__") \ + X(gLifeLine, "__lifeline") \ + X(gModule, "__module__") \ + X(gMRO, "__mro__") \ + X(gName, "__name__") \ + X(gNe, "__ne__") \ + X(gRepr, "__repr__") \ + X(gCppRepr, "__cpp_repr") \ + X(gStr, "__str__") \ + X(gCppStr, "__cpp_str") \ + X(gTypeCode, "typecode") \ + X(gCTypesType, "_type_") \ + \ + X(gUnderlying, "__underlying") \ + X(gRealInit, "__real_init") \ + \ + X(gAdd, "__add__") \ + X(gSub, "__sub__") \ + X(gMul, "__mul__") \ + X(gDiv, "__truediv__") \ + \ + X(gLShift, "__lshift__") \ + X(gLShiftC, "__lshiftc__") \ + \ + X(gAt, "at") \ + X(gBegin, "begin") \ + X(gEnd, "end") \ + X(gFirst, "first") \ + X(gSecond, "second") \ + X(gSize, "size") \ + X(gTemplate, "Template") \ + X(gVectorAt, "_vector__at") \ + X(gInsert, "insert") \ + X(gValueType, "value_type") \ + X(gValueTypePtr, "_value_type") \ + X(gValueSize, "value_size") \ + \ + X(gCppReal, "__cpp_real") \ + X(gCppImag, "__cpp_imag") \ + \ + X(gThisModule, "cppjit") \ + \ + X(gDispInit, "_init_dispatchptr") \ + X(gDispGet, "_get_dispatch") \ + \ + X(gExPythonize, "__cppjit_explicit_pythonize__") \ + X(gPythonize, "__cppjit_pythonize__") \ + \ + X(gArray, "__array__") \ + X(gDType, "dtype") \ + X(gFromBuffer, "frombuffer") -extern PyObject* gAt; -extern PyObject* gBegin; -extern PyObject* gEnd; -extern PyObject* gFirst; -extern PyObject* gSecond; -extern PyObject* gSize; -extern PyObject* gTemplate; -extern PyObject* gVectorAt; -extern PyObject* gInsert; -extern PyObject* gValueType; -extern PyObject* gValueTypePtr; -extern PyObject* gValueSize; - -extern PyObject* gCppReal; -extern PyObject* gCppImag; - -extern PyObject* gThisModule; - -extern PyObject* gDispInit; -extern PyObject* gDispGet; - -extern PyObject* gExPythonize; -extern PyObject* gPythonize; +namespace PyStrings { -extern PyObject* gArray; -extern PyObject* gDType; -extern PyObject* gFromBuffer; +#define CPYRT_DECLARE_PYSTRING(var, str) extern PyObject* var; +CPYRT_PYSTRINGS(CPYRT_DECLARE_PYSTRING) +#undef CPYRT_DECLARE_PYSTRING } // namespace PyStrings From 56e184f271d2ddb285d588b9f39854f3984bdeda Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Fri, 9 Oct 2026 15:21:53 +0200 Subject: [PATCH 05/17] [cppjit] [cpyrt] Remove RegisterConverterAlias() and RegisterExecutorAlias() (compiler-research/cppjit#147) ROOT used them to give Float16_t and Double32_t the converters and executors of float and double. cppjit resolves these typedefs through CppInterOp by itself, and ROOT stopped registering the aliases, so the functions have no users left. These functions were only introduced to cppyy in 2024 and were not documented: https://github.com/wlav/CPyCppyy/pull/27 (cherry picked from commit edb749b) --- bindings/pyroot/cppjit/src/cpyrt/API.h | 8 -------- bindings/pyroot/cppjit/src/cpyrt/Converters.cxx | 17 ----------------- bindings/pyroot/cppjit/src/cpyrt/Converters.h | 2 -- bindings/pyroot/cppjit/src/cpyrt/Executors.cxx | 17 ----------------- bindings/pyroot/cppjit/src/cpyrt/Executors.h | 2 -- 5 files changed, 46 deletions(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/API.h b/bindings/pyroot/cppjit/src/cpyrt/API.h index 627132a2e265b..083b5d1fa5c63 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/API.h +++ b/bindings/pyroot/cppjit/src/cpyrt/API.h @@ -145,10 +145,6 @@ typedef Converter* (*ConverterFactory_t)(cdims_t); CPYRT_EXTERN bool RegisterConverter(const std::string& name, ConverterFactory_t); -// register a custom converter that is a reference to an existing converter -CPYRT_EXTERN bool RegisterConverterAlias(const std::string& name, - const std::string& target); - // remove a custom converter CPYRT_EXTERN bool UnregisterConverter(const std::string& name); @@ -176,10 +172,6 @@ CPYRT_EXTERN void DestroyConverter(Converter* p); typedef Executor* (*ExecutorFactory_t)(cdims_t); CPYRT_EXTERN bool RegisterExecutor(const std::string& name, ExecutorFactory_t); -// register a custom executor that is a reference to an existing converter -CPYRT_EXTERN bool RegisterExecutorAlias(const std::string& name, - const std::string& target); - // remove a custom executor CPYRT_EXTERN bool UnregisterExecutor(const std::string& name); diff --git a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx index c21d00ef182e9..85b9e9486c926 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx @@ -3985,23 +3985,6 @@ bool cpyrt::RegisterConverter(const std::string& name, cf_t fac) { return true; } -//---------------------------------------------------------------------------- -CPYRT_EXPORT -bool cpyrt::RegisterConverterAlias(const std::string& name, - const std::string& target) { - // register a custom converter that is a reference to an existing converter - auto f = gConvFactories.find(name); - if (f != gConvFactories.end()) - return false; - - auto t = gConvFactories.find(target); - if (t == gConvFactories.end()) - return false; - - gConvFactories[name] = t->second; - return true; -} - //---------------------------------------------------------------------------- CPYRT_EXPORT bool cpyrt::UnregisterConverter(const std::string& name) { diff --git a/bindings/pyroot/cppjit/src/cpyrt/Converters.h b/bindings/pyroot/cppjit/src/cpyrt/Converters.h index bc2f65d61e20d..d208464825606 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Converters.h +++ b/bindings/pyroot/cppjit/src/cpyrt/Converters.h @@ -40,8 +40,6 @@ CPYRT_EXPORT Converter* CreateConverter(interop::TCppType_t type, CPYRT_EXPORT void DestroyConverter(Converter* p); typedef Converter* (*cf_t)(cdims_t d); CPYRT_EXPORT bool RegisterConverter(const std::string& name, cf_t fac); -CPYRT_EXPORT bool RegisterConverterAlias(const std::string& name, - const std::string& target); CPYRT_EXPORT bool UnregisterConverter(const std::string& name); // converters for special cases (only here b/c of external use of diff --git a/bindings/pyroot/cppjit/src/cpyrt/Executors.cxx b/bindings/pyroot/cppjit/src/cpyrt/Executors.cxx index 6fc105e417f9f..14afe0d401799 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Executors.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Executors.cxx @@ -1134,23 +1134,6 @@ bool cpyrt::RegisterExecutor(const std::string& name, ef_t fac) { return true; } -//---------------------------------------------------------------------------- -CPYRT_EXPORT -bool cpyrt::RegisterExecutorAlias(const std::string& name, - const std::string& target) { - // register a custom executor that is a reference to an existing converter - auto f = gExecFactories.find(name); - if (f != gExecFactories.end()) - return false; - - auto t = gExecFactories.find(target); - if (t == gExecFactories.end()) - return false; - - gExecFactories[name] = t->second; - return true; -} - //---------------------------------------------------------------------------- CPYRT_EXPORT bool cpyrt::UnregisterExecutor(const std::string& name) { diff --git a/bindings/pyroot/cppjit/src/cpyrt/Executors.h b/bindings/pyroot/cppjit/src/cpyrt/Executors.h index c8b0702b5fee7..959afd1775c86 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Executors.h +++ b/bindings/pyroot/cppjit/src/cpyrt/Executors.h @@ -39,8 +39,6 @@ CPYRT_EXPORT Executor* CreateExecutor(interop::TCppType_t type, cdims_t = 0); CPYRT_EXPORT void DestroyExecutor(Executor* p); typedef Executor* (*ef_t)(cdims_t); CPYRT_EXPORT bool RegisterExecutor(const std::string& name, ef_t fac); -CPYRT_EXPORT bool RegisterExecutorAlias(const std::string& name, - const std::string& target); CPYRT_EXPORT bool UnregisterExecutor(const std::string& name); // helper for the actual call From 0b24f980396df0168ab26fc41bfc78ac11f70835 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Fri, 9 Oct 2026 15:21:56 +0200 Subject: [PATCH 06/17] [cppjit] Check for smart pointers by class instead of by name (compiler-research/cppjit#143) GetSmartPtrInfo() took a type name and resolved it with ResolveName(), which declares a __typeof__ alias for every name it has not seen yet. It is called when creating the converter of each class-type argument, so the first call of a method of a new class template instance, e.g. the constructor of std::set with its std::less and std::allocator arguments, declared several such aliases. All callers already have the class, so pass it instead and compare its name against the known smart pointer templates. The alias declaration also silently attempted to instantiate class template specializations, so a specialization that can't be defined (of a template that is only declared) was never instantiated again. Without it, Cpp::GetNumBases() is the first to try when the Python class is created, and it prints an "explicit instantiation of undefined template" error. Check with Cpp::IsComplete() first, which instantiates without diagnostics. For example: ```python import time import cppjit cppjit.cppdef("#include \n") types = ["int", "long", "short", "char", "float", "double", "unsigned int", "unsigned long"] classes = [cppjit.gbl.std.set[t] for t in types] start = time.perf_counter() for cls in classes: cls() # first construction of each class print(f"first construction of {len(classes)} std::set classes: {1000 * (time.perf_counter() - start):.0f} ms") ``` This takes about 444 ms instead of 461 ms (median of 3 trials, Linux x86_64, Python 3.14, CppInterOp/LLVM 22.1.8). The savings grow with the number of distinct template instantiations whose methods are first called; the interpreter-state growth from the dropped alias declarations matters beyond the reruns. (cherry picked from commit f7e785f) --- bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx | 7 +------ bindings/pyroot/cppjit/src/cpyrt/Converters.cxx | 4 ++-- .../pyroot/cppjit/src/interop/cppjit_interop.h | 2 +- .../pyroot/cppjit/src/interop/interop_wrapper.cxx | 14 ++++++-------- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx index 9f695789b6683..dd5801abed713 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx @@ -207,16 +207,11 @@ static PyObject* pt_new(PyTypeObject* subtype, PyObject* args, PyObject* kwds) { // creation of the python-side class; extend the size if this is a smart ptr // // A namespace can never be a smart pointer, so skip the check for namespaces. - // This matters beyond performance: GetSmartPtrInfo() resolves the scope name - // through a slow interpreter lookup that triggers autoloading of the library - // providing the scope, which we don't want to do unnecessarily. interop::TCppScope_t raw; interop::TCppMethod_t deref; if (CPPScope_CheckExact(subtype) && !interop::IsNamespace(((CPPScope*)subtype)->fCppType)) { - if (interop::GetSmartPtrInfo( - interop::GetScopedFinalName(((CPPScope*)subtype)->fCppType), &raw, - &deref)) + if (interop::GetSmartPtrInfo(((CPPScope*)subtype)->fCppType, &raw, &deref)) subtype->tp_basicsize = sizeof(CPPSmartClass); } diff --git a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx index 85b9e9486c926..da661fa383c45 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx @@ -3639,7 +3639,7 @@ cppjit::cpyrt::CreateConverter(const std::string& fullType, cdims_t dims) { Converter* result = nullptr; if (interop::TCppScope_t klass = interop::GetFullScope(realType)) { interop::TCppScope_t raw; - if (interop::GetSmartPtrInfo(realType, &raw, nullptr)) { + if (interop::GetSmartPtrInfo(klass, &raw, nullptr)) { if (cpd == "") { result = new SmartPtrConverter(klass, raw, control); } else if (cpd == "&") { @@ -3902,7 +3902,7 @@ cppjit::cpyrt::CreateConverter(interop::TCppType_t type, cdims_t dims) { (klass || (klass = interop::GetFullScope(realTypeStr)))) { // std::byte is a special enum class used to access raw memory interop::TCppScope_t raw; - if (interop::GetSmartPtrInfo(realTypeStr, &raw, nullptr)) { + if (interop::GetSmartPtrInfo(klass, &raw, nullptr)) { if (cpd == "") { result = new SmartPtrConverter(klass, raw, control); } else if (cpd == "&") { diff --git a/bindings/pyroot/cppjit/src/interop/cppjit_interop.h b/bindings/pyroot/cppjit/src/interop/cppjit_interop.h index db2ccf0b51dc4..763de03b45fef 100644 --- a/bindings/pyroot/cppjit/src/interop/cppjit_interop.h +++ b/bindings/pyroot/cppjit/src/interop/cppjit_interop.h @@ -248,7 +248,7 @@ bool IsSubclass(TCppScope_t derived, TCppScope_t base); RPY_EXPORTED bool IsSmartPtr(TCppScope_t klass); RPY_EXPORTED -bool GetSmartPtrInfo(const std::string&, TCppScope_t* raw, TCppMethod_t* deref); +bool GetSmartPtrInfo(TCppScope_t klass, TCppScope_t* raw, TCppMethod_t* deref); // calculate offsets between declared and actual type, up-cast: direction > 0; // down-cast: direction < 0 RPY_EXPORTED diff --git a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx index ef48f950e50d2..0add59261d711 100644 --- a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx +++ b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx @@ -1411,23 +1411,21 @@ bool interop::IsSmartPtr(TCppScope_t klass) { return false; } -bool interop::GetSmartPtrInfo(const std::string& tname, TCppScope_t* raw, +bool interop::GetSmartPtrInfo(TCppScope_t klass, TCppScope_t* raw, TCppMethod_t* deref) { - // TODO: We can directly accept scope instead of name - const std::string& rn = ResolveName(tname); - if (gSmartPtrTypes.find(rn.substr(0, rn.find("<"))) == gSmartPtrTypes.end()) + TCppScope_t scope = interop::GetUnderlyingScope(klass); + if (!scope || !interop::IsSmartPtr(scope)) return false; if (!raw && !deref) return true; - TCppScope_t scope = interop::GetScope(rn); - if (!scope) - return false; - std::vector ops; { std::lock_guard Lock(InterOpMutex); + // the operator can only be found once the specialization is instantiated + if (!Cpp::IsComplete(scope)) + return false; Cpp::GetOperator(scope, Cpp::Operator::OP_Arrow, ops, /*kind=*/Cpp::OperatorArity::kBoth); if (ops.size() != 1) From a41f6638a262c4780d8c7009991eae86d921613e Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Fri, 9 Oct 2026 19:37:35 +0200 Subject: [PATCH 07/17] [cppjit] [cpyrt] Fix leaked container reference in vector_iter (compiler-research/cppjit#148) vector_iter() took two references to the container, one when storing it and one after deciding whether the container is a temporary, while the iterator's dealloc releases only one. Every iteration over a `std::vector` proxy therefore leaked a reference to it: the proxy, and the C++ object if Python owns it, never got freed. ROOT's own cppyy copy got this right in root-project/root#19017 (commit 700fee6e7b6c), which moved the temporary check before the increment so that PyUnstable_Object_IsUniqueReferencedTemporary() sees the untouched refcount on Python 3.14, and dropped the original increment. The port of that change to the compiler-research fork, compiler-research/CPyCppyy#151 (commit compiler-research/CPyCppyy@8eba11b58c, reapplied as compiler-research/CPyCppyy@7e086216a3), added the new increment after the check but kept the original one. cppjit inherited both in its initial import (compiler-research/cppjit@38b22b99be), and ROOT with the vendoring in 552d291c328. Upstream cppjit still has both. Beyond the memory, the leak made ROOT's pyroot-pyz-ttree-branch flaky: the proxies leaked by the earlier read test stayed registered in the memory regulator under addresses the closed file had freed, so when a later `new std::vector` landed on one of them, reading the pointer data member returned the stale proxy instead of a reference proxy, and TTree::Branch() got bound to the proxy's own pointer slot. (cherry picked from commit 3df629f) --- bindings/pyroot/cppjit/src/cpyrt/Pythonize.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/Pythonize.cxx b/bindings/pyroot/cppjit/src/cpyrt/Pythonize.cxx index 5781345db3fdd..02ad193ae39a8 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Pythonize.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Pythonize.cxx @@ -686,10 +686,10 @@ static PyObject* vector_iter(PyObject* v) { if (!vi) return nullptr; - Py_INCREF(v); vi->ii_container = v; // tell the iterator code to set a life line if this container is a temporary + // (checked before taking the iterator's own reference) vi->vi_flags = vectoriterobject::kDefault; #if PY_VERSION_HEX >= 0x030e0000 if (PyUnstable_Object_IsUniqueReferencedTemporary(v) || From 7158e0b45f0ef63d84c31235999497d37ffac48a Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Thu, 27 Aug 2026 17:06:18 +0200 Subject: [PATCH 08/17] [cppjit] Adopt Clang-driven overload resolution from CppInterOp (cherry picked from commit c866488) --- bindings/pyroot/cppjit/CMakeLists.txt | 2 + .../cppjit/python/cppjit/_cpython_cppjit.py | 4 + .../pyroot/cppjit/src/cpyrt/CPPExcInstance.h | 3 + .../pyroot/cppjit/src/cpyrt/CPPInstance.cxx | 54 +-- .../pyroot/cppjit/src/cpyrt/CPPInstance.h | 47 +++ .../pyroot/cppjit/src/cpyrt/CPPMethod.cxx | 138 ------ bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h | 11 +- .../pyroot/cppjit/src/cpyrt/CPPOverload.cxx | 351 +++++++++++----- .../pyroot/cppjit/src/cpyrt/CPPOverload.h | 74 ++-- bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx | 11 +- bindings/pyroot/cppjit/src/cpyrt/CPPScope.h | 2 + .../pyroot/cppjit/src/cpyrt/CallContext.h | 27 +- .../pyroot/cppjit/src/cpyrt/Converters.cxx | 13 +- .../pyroot/cppjit/src/cpyrt/Dispatcher.cxx | 1 + .../pyroot/cppjit/src/cpyrt/LowLevelViews.cxx | 1 + .../pyroot/cppjit/src/cpyrt/LowLevelViews.h | 1 + .../pyroot/cppjit/src/cpyrt/ProxyWrappers.cxx | 15 +- bindings/pyroot/cppjit/src/cpyrt/PyCallable.h | 8 +- .../pyroot/cppjit/src/cpyrt/TemplateProxy.cxx | 240 ++++------- .../pyroot/cppjit/src/cpyrt/TemplateProxy.h | 20 +- bindings/pyroot/cppjit/src/cpyrt/Utility.cxx | 204 ++++++++- bindings/pyroot/cppjit/src/cpyrt/Utility.h | 5 +- .../pyroot/cppjit/src/cpyrt/cpyrtModule.cxx | 13 +- .../cppjit/src/interop/cppjit_interop.h | 24 +- .../cppjit/src/interop/interop_wrapper.cxx | 393 +++++++++++++++--- .../pyroot/cppjit/test/test_advancedcpp.py | 4 + .../pyroot/cppjit/test/test_conversions.py | 7 +- .../pyroot/cppjit/test/test_cpp11features.py | 4 +- .../cppjit/test/test_crossinheritance.py | 18 +- bindings/pyroot/cppjit/test/test_datatypes.py | 20 +- .../pyroot/cppjit/test/test_doc_features.py | 2 +- bindings/pyroot/cppjit/test/test_fragile.py | 30 +- bindings/pyroot/cppjit/test/test_leakcheck.py | 2 +- bindings/pyroot/cppjit/test/test_overloads.py | 12 +- bindings/pyroot/cppjit/test/test_pythonify.py | 4 +- .../pyroot/cppjit/test/test_regression.py | 31 +- bindings/pyroot/cppjit/test/test_stltypes.py | 15 +- bindings/pyroot/cppjit/test/test_templates.py | 14 +- 38 files changed, 1177 insertions(+), 648 deletions(-) diff --git a/bindings/pyroot/cppjit/CMakeLists.txt b/bindings/pyroot/cppjit/CMakeLists.txt index 79828f6e307c8..f4db194c69631 100644 --- a/bindings/pyroot/cppjit/CMakeLists.txt +++ b/bindings/pyroot/cppjit/CMakeLists.txt @@ -10,6 +10,8 @@ include(GNUInstallDirs) # This option won't make a lot of sense since we only ship the shared library in site-packages # Perhaps this should permanently be OFF and users can build their own CppInterOp if they want to run the tests? option(CPPJIT_ENABLE_CPPINTEROP_TESTS "enable CppInterOp tests" OFF) +# Temporary pin to the overload-resolution development branch so CI builds +# and tests it. Returns to a pinned commit once that branch lands. set(CPPINTEROP_GIT_REPOSITORY "https://github.com/compiler-research/CppInterOp.git" CACHE STRING "") set(CPPINTEROP_GIT_TAG "c8126249dd18363000b89333b34313ca2a83fb82" CACHE STRING "") set(CPPINTEROP_SOURCE_DIR "" CACHE PATH diff --git a/bindings/pyroot/cppjit/python/cppjit/_cpython_cppjit.py b/bindings/pyroot/cppjit/python/cppjit/_cpython_cppjit.py index f41de438c0367..93ddb21ac24a6 100644 --- a/bindings/pyroot/cppjit/python/cppjit/_cpython_cppjit.py +++ b/bindings/pyroot/cppjit/python/cppjit/_cpython_cppjit.py @@ -25,6 +25,8 @@ "_end_capture_stderr", "_lock_interpreter", "_unlock_interpreter", + "OverloadResolutionException", + "OverloadAmbiguityException", ] @@ -207,6 +209,8 @@ def _add_default_paths(): bind_object = _backend.bind_object nullptr = _backend.nullptr default = _backend.default +OverloadResolutionException = _backend.OverloadResolutionException +OverloadAmbiguityException = _backend.OverloadAmbiguityException def load_reflection_info(name): diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPExcInstance.h b/bindings/pyroot/cppjit/src/cpyrt/CPPExcInstance.h index 35e4c9fb624d7..6a52a0e8dfe22 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPExcInstance.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPExcInstance.h @@ -1,6 +1,9 @@ #ifndef CPYRT_CPPEXCINSTANCE_H #define CPYRT_CPPEXCINSTANCE_H +#include "Python.h" +#include "cpyrt/CommonDefs.h" + ////////////////////////////////////////////////////////////////////////////// // // // cppjit::cpyrt::CPPExceptionInstance // diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.cxx index 13ccf3a7b02d5..b2220b7d46ccc 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.cxx @@ -42,51 +42,6 @@ extern PyObject* gNullPtrObject; // pointer to the smart pointer. They carry a pointer to the Python-sode smart // class for dereferencing to get to the actual instance pointer. -//- private helpers ---------------------------------------------------------- -namespace { - -// Several specific use cases require extra data in a CPPInstance, but can not -// be a new type. E.g. cross-inheritance derived types are by definition added -// a posterio, and caching of datamembers is up to the datamember, not the -// instance type. To not have normal use of CPPInstance take extra memory, this -// extended data can slot in place of fObject for those use cases. - -struct ExtendedData { - ExtendedData() - : fObject(nullptr), fSmartClass(nullptr), fDispatchPtr(nullptr), - fArraySize(0) {} - ~ExtendedData() { - for (auto& pc : fDatamemberCache) - Py_XDECREF(pc.second); - fDatamemberCache.clear(); - } - - // the original object reference it replaces (Note: has to be first data - // member, see usage in GetObjectRaw(), e.g. for ptr-ptr passing) - void* fObject; - - // for caching expensive-to-create data member representations - cpyrt::CI_DatamemberCache_t fDatamemberCache; - - // for smart pointer types - cpyrt::CPPSmartClass* fSmartClass; - - // for back-referencing from Python-derived instances - cpyrt::DispatchPtr* fDispatchPtr; - - // for representing T* as a low-level array - Py_ssize_t fArraySize; -}; - -} // unnamed namespace - -#define EXT_OBJECT(pyobj) ((ExtendedData*)((pyobj)->fObject))->fObject -#define DATA_CACHE(pyobj) ((ExtendedData*)((pyobj)->fObject))->fDatamemberCache -#define SMART_CLS(pyobj) ((ExtendedData*)((pyobj)->fObject))->fSmartClass -#define SMART_TYPE(pyobj) SMART_CLS(pyobj)->fCppType -#define DISPATCHPTR(pyobj) ((ExtendedData*)((pyobj)->fObject))->fDispatchPtr -#define ARRAY_SIZE(pyobj) ((ExtendedData*)((pyobj)->fObject))->fArraySize - inline void cpyrt::CPPInstance::CreateExtension() { if (fFlags & kIsExtended) return; @@ -549,7 +504,8 @@ static inline PyObject* eqneq_binop(CPPClass* klass, PyObject* self, const char* cppop = op == Py_EQ ? "==" : "!="; PyCallable* pyfunc = FindBinaryOperator(self, obj, cppop); if (pyfunc) - binop = (PyObject*)CPPOverload_New(cppop, pyfunc); + binop = (PyObject*)CPPOverload_New( + cppop, interop::GetParentScope(pyfunc->GetMethod().data), pyfunc); else { Py_INCREF(Py_None); binop = Py_None; @@ -613,7 +569,8 @@ static inline void* cast_actual(void* obj) { PyCallable* pyfunc = \ Utility::FindBinaryOperator((PyObject*)self, other, #op); \ if (pyfunc) \ - ometh = (PyObject*)CPPOverload_New(#label, pyfunc); \ + ometh = (PyObject*)CPPOverload_New( \ + #label, interop::GetParentScope(pyfunc->GetMethod().data), pyfunc); \ } \ meth = ometh; @@ -1005,7 +962,8 @@ static PyGetSetDef op_getset[] = { PyErr_Clear(); \ PyCallable* pyfunc = Utility::FindBinaryOperator(left, right, #op); \ if (pyfunc) \ - meth = (PyObject*)CPPOverload_New(#name, pyfunc); \ + meth = (PyObject*)CPPOverload_New( \ + #name, interop::GetParentScope(pyfunc->GetMethod().data), pyfunc); \ else { \ PyErr_SetString(PyExc_NotImplementedError, ""); \ return nullptr; \ diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.h b/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.h index c94ecfa16c220..4aa06ee2b70ef 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPInstance.h @@ -17,6 +17,8 @@ #include "CommonDefs.h" #include "cppjit_interop.h" +#include "cpyrt/DispatchPtr.h" + // Standard #include #include @@ -146,4 +148,49 @@ void op_dealloc_nofree(CPPInstance*); } // namespace cppjit::cpyrt +//- private helpers ---------------------------------------------------------- +namespace { + +// Several specific use cases require extra data in a CPPInstance, but can not +// be a new type. E.g. cross-inheritance derived types are by definition added +// a posterio, and caching of datamembers is up to the datamember, not the +// instance type. To not have normal use of CPPInstance take extra memory, this +// extended data can slot in place of fObject for those use cases. + +struct ExtendedData { + ExtendedData() + : fObject(nullptr), fSmartClass(nullptr), fDispatchPtr(nullptr), + fArraySize(0) {} + ~ExtendedData() { + for (auto& pc : fDatamemberCache) + Py_XDECREF(pc.second); + fDatamemberCache.clear(); + } + + // the original object reference it replaces (Note: has to be first data + // member, see usage in GetObjectRaw(), e.g. for ptr-ptr passing) + void* fObject; + + // for caching expensive-to-create data member representations + cppjit::cpyrt::CI_DatamemberCache_t fDatamemberCache; + + // for smart pointer types + cppjit::cpyrt::CPPSmartClass* fSmartClass; + + // for back-referencing from Python-derived instances + cppjit::cpyrt::DispatchPtr* fDispatchPtr; + + // for representing T* as a low-level array + Py_ssize_t fArraySize; +}; + +} // unnamed namespace + +#define EXT_OBJECT(pyobj) ((ExtendedData*)((pyobj)->fObject))->fObject +#define DATA_CACHE(pyobj) ((ExtendedData*)((pyobj)->fObject))->fDatamemberCache +#define SMART_CLS(pyobj) ((ExtendedData*)((pyobj)->fObject))->fSmartClass +#define SMART_TYPE(pyobj) SMART_CLS(pyobj)->fCppType +#define DISPATCHPTR(pyobj) ((ExtendedData*)((pyobj)->fObject))->fDispatchPtr +#define ARRAY_SIZE(pyobj) ((ExtendedData*)((pyobj)->fObject))->fArraySize + #endif // !CPYRT_CPPINSTANCE_H diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.cxx index 0221f836748ac..2f38ae3194262 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.cxx @@ -446,144 +446,6 @@ PyObject* cpyrt::CPPMethod::Reflex(interop::Reflex::RequestId_t request, return PyCallable::Reflex(request, format); } -//---------------------------------------------------------------------------- -int cpyrt::CPPMethod::GetPriority() { - // To help with overload selection, methods are given a priority based on the - // affinity of Python and C++ types. Priority only matters for methods that - // have an equal number of arguments and types that are possible substitutes - // (the normal selection mechanisms would simply distinguish them otherwise). - - // The following types are ordered, in favor (variants implicit): - // - // bool >> long >> int >> short - // double >> long double >> float - // const char* >> char - // - // Further, all integer types are preferred over floating point b/c int to - // float is allowed implicitly, float to int is not. - // - // Special cases that are disliked include void* and unknown/incomplete types. - // Also, moves are preferred over references. std::initializer_list is not a - // nice conversion candidate either, but needs to be higher priority to mix - // well with implicit conversions. - // TODO: extend this to favour classes that are not bases. - // TODO: profile this method (it's expensive, but should be called too often) - - int priority = 0; - - const size_t nArgs = interop::GetMethodNumArgs(fMethod); - for (int iarg = 0; iarg < (int)nArgs; ++iarg) { - const std::string aname = interop::GetMethodArgTypeAsString(fMethod, iarg); - // FIXME: convert the string comparisons with comparison to the underlying - // type: - // interop::TCppType_t type = interop::GetMethodArgType(fMethod, iarg); - - // Not builtin and spelled "const void *", so match the compacted name. - std::string compact = aname; - compact.erase(std::remove(compact.begin(), compact.end(), ' '), - compact.end()); - - if (compact.find("void*") != std::string::npos) { - priority -= 1000; // void*/void** shouldn't be too greedy - } else if (interop::IsBuiltin(aname)) { - // complex type (note: double penalty: for complex and the template type) - if (strstr(aname.c_str(), "std::complex")) - priority -= 10; // prefer double, float, etc. over conversion - - // integer types - if (strstr(aname.c_str(), "bool")) - priority += 1; // bool over int (does accept 1 and 0) - else if (strstr(aname.c_str(), "long long")) - priority += -5; // will very likely fit - else if (strstr(aname.c_str(), "long")) - priority += -10; // most affine integer type - // no need to compare with int; leave at zero - else if (strstr(aname.c_str(), "short")) - priority += -50; // not really relevant as a type - - // floating point types (note all numbers lower than integer types) - else if (strstr(aname.c_str(), "float")) - priority += -100; // not really relevant as a type - else if (strstr(aname.c_str(), "long double")) - priority += -90; // fits double with least loss of precision - else if (strstr(aname.c_str(), "double")) - priority += -80; // most affine floating point type - - // string/char types - else if (strstr(aname.c_str(), "char") && aname[aname.size() - 1] != '*') - priority += -60; // prefer (const) char* over char - - } else { - // This is a user-defined type (class, struct, enum, etc.). - - // There's a bit of hysteresis here for templates: once GetScope() is - // called, their IsComplete() succeeds, the other way around it does not. - // Since GetPriority() is likely called several times in a sort, the - // GetScope() _must_ come first, or different GetPriority() calls may - // return different results (since the 2nd time, GetScope() will have been - // called from the first), killing the stable_sort. - - // prefer more derived classes - const std::string& clean_name = TypeManip::clean_type(aname, false); - interop::TCppScope_t scope = interop::GetScope(clean_name); - if (scope) - priority += static_cast(interop::GetNumBasesLongestBranch(scope)); - - // GetScope does not resolve enum names, so also detect enums by type - if (interop::IsEnumScope(scope) || - interop::IsEnumType(interop::GetMethodArgType(fMethod, iarg))) - priority -= 100; - - // a couple of special cases as explained above - if (aname.find("initializer_list") != std::string::npos) { - priority += 150; // needed for proper implicit conversion rules - } else if (aname.rfind("&&", aname.size() - 2) != std::string::npos) { - priority += 100; // prefer moves over other ref/ptr - } else if (scope && !interop::IsComplete(scope)) { - // class is known, but no dictionary available, 2 more cases: * and & - if (aname[aname.size() - 1] == '&') - priority += -5000; - else - priority += -2000; // prefer pointer passing over reference - } - } - } - - // prefer methods w/o optional arguments b/c ones with optional arguments are - // easier to select by providing the optional arguments explicitly - priority += ((int)interop::GetMethodReqArgs(fMethod) - (int)nArgs); - - // add a small penalty to prefer non-const methods over const ones for - // get/setitem - if (interop::IsConstMethod(fMethod) && - interop::GetName(interop::TCppScope_t(fMethod.data)) == "operator[]") - priority += -10; - - // constructors are prefered - if (interop::IsConstructor(fMethod)) - priority += 100; - - return priority; -} - -//---------------------------------------------------------------------------- -bool cpyrt::CPPMethod::IsGreedy() { - // Methods will all void*-like arguments should be sorted after template - // instanstations, so that they don't greedily take over pointers to object. - // GetPriority() is too heavy-handed, as it will pull in all the argument - // types, so use this cheaper check. - const size_t nArgs = interop::GetMethodReqArgs(fMethod); - if (!nArgs) - return false; - - for (int iarg = 0; iarg < (int)nArgs; ++iarg) { - const std::string aname = interop::GetMethodArgTypeAsString(fMethod, iarg); - if (aname.find("void *") != 0) - return false; - } - return true; -} - //---------------------------------------------------------------------------- int cpyrt::CPPMethod::GetMaxArgs() { return (int)interop::GetMethodNumArgs(fMethod); diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h b/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h index 41e9546b4c182..85f66c66f4955 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h @@ -46,6 +46,7 @@ class CPPMethod : public PyCallable { virtual ~CPPMethod(); public: + interop::TCppMethod_t GetMethod() override { return fMethod; } PyObject* GetSignature(bool show_formalargs = true) override; PyObject* GetSignatureNames() override; PyObject* GetSignatureTypes() override; @@ -55,9 +56,6 @@ class CPPMethod : public PyCallable { Reflex(interop::Reflex::RequestId_t request, interop::Reflex::FormatId_t = interop::Reflex::OPTIMAL) override; - int GetPriority() override; - bool IsGreedy() override; - int GetMaxArgs() override; PyObject* GetCoVarNames() override; PyObject* GetArgDefault(int iarg, bool silent = true) override; @@ -75,6 +73,12 @@ class CPPMethod : public PyCallable { fn_type, cppjit::interop::GetTypeFromScope( cppjit::interop::TCppScope_t(fMethod.data))); } + // extra info methods + bool IsOperator() { return interop::IsOperator(fMethod.data); } + bool IsConversionOperator() { + return interop::IsConversionOperator(fMethod.data); + } + bool IsStaticMethod() { return interop::IsStaticMethod(fMethod); } public: PyObject* Call(CPPInstance*& self, cpyrt_PyArgs_t args, size_t nargsf, @@ -89,7 +93,6 @@ class CPPMethod : public PyCallable { CallContext* ctxt = nullptr); PyObject* Execute(void* self, ptrdiff_t offset, CallContext* ctxt = nullptr); - interop::TCppMethod_t GetMethod() { return fMethod; } interop::TCppScope_t GetScope() { return fScope; } Executor* GetExecutor() { return fExecutor; } std::string GetSignatureString(bool show_formalargs = true); diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx index 9601620559866..a80d0277a5dc7 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx @@ -3,10 +3,15 @@ using namespace cppjit; #include "CPPInstance.h" +#include "CPPMethod.h" #include "CPPOverload.h" #include "CallContext.h" +#include "MemoryRegulator.h" +#include "ProxyWrappers.h" +#include "PyCallable.h" #include "PyStrings.h" #include "Utility.h" +#include "cppjit_interop.h" #include "structmember.h" // from Python #include "cpyrt/Reflex.h" @@ -63,9 +68,6 @@ class TPythonCallback : public PyCallable { } } - int GetPriority() override { return 100; }; - bool IsGreedy() override { return false; }; - int GetMaxArgs() override { return 100; }; PyObject* GetCoVarNames() override { // TODO: pick these up from the callable Py_RETURN_NONE; @@ -124,12 +126,6 @@ static inline bool IsPseudoFunc(CPPOverload* pymeth) { return pymeth->fMethodInfo->fFlags & CallContext::kIsPseudoFunc; } -// helper to sort on method priority -static int PriorityCmp(const std::pair& left, - const std::pair& right) { - return left.first > right.first; -} - // return helper static inline void ResetCallState(CPPInstance* descr_self, CPPInstance*& im_self) { @@ -608,101 +604,267 @@ static PyObject* mp_vectorcall(CPPOverload* pymeth, PyObject* const* args, ResetCallState(pymeth->fSelf, im_self); } - // ... otherwise loop over all methods and find the one that does not fail - if (!IsSorted(mflags)) { - // sorting is based on priority, which is not stored on the method as it is - // used only once, so copy the vector of methods into one where the priority - // can be stored during sorting - std::vector> pm; - pm.reserve(methods.size()); - for (auto ptr : methods) - pm.emplace_back(ptr->GetPriority(), ptr); - std::stable_sort(pm.begin(), pm.end(), PriorityCmp); - for (CPPOverload::Methods_t::size_type i = 0; i < methods.size(); ++i) - methods[i] = pm[i].second; - pymeth->fMethodInfo->fFlags |= CallContext::kIsSorted; + std::vector overloads; + bool is_operator = false; + bool is_conversion_operator = false; + bool is_static = false; + for (auto i : pymeth->fMethodInfo->fMethods) { + if (is_operator || dynamic_cast(i)->IsOperator()) + is_operator = true; + if (is_conversion_operator || + dynamic_cast(i)->IsConversionOperator()) + is_conversion_operator = true; + if (is_static || dynamic_cast(i)->IsStaticMethod()) + is_static = true; + overloads.push_back(i->GetMethod()); } - // three stages, as the C++ ranking: exact matches, standard conversions - // (kNoImplicit: no user-defined construction from the argument), then - // implicit conversions - std::vector errors; - std::vector implicit_possible(methods.size()); - bool bHaveImplicit = false; - for (int stage = 0; stage < 3; ++stage) { - if (stage == 1) - ctxt.fFlags |= CallContext::kNoImplicit; - else if (stage == 2) { - ctxt.fFlags &= ~CallContext::kNoImplicit; - ctxt.fFlags |= CallContext::kAllowImplicit; + std::string proto = ""; + if (im_self && !(ctxt.fFlags & CallContext::kIsConstructor)) { + PyObject* self = (PyObject*)im_self; + [[maybe_unused]] auto res = + AddTypeName(proto, (PyObject*)Py_TYPE(self), self, Utility::kNone); + assert(res); + } + + ctxt.fFlags |= CallContext::kAllowImplicit; + { + size_t i = ctxt.fFlags & CallContext::kIsConstructor && !im_self ? 1 : 0; + CPPInstance* self = nullptr; + if (i) { + self = (CPPInstance*)cpyrt_PyArgs_GET_ITEM(args, 0); + } else { + self = im_self; } - for (CPPOverload::Methods_t::size_type i = 0; i < nMethods; ++i) { - if (stage && !implicit_possible[i]) - continue; // did not set implicit conversion, so don't try again - - PyObject* result = methods[i]->Call(im_self, args, nargsf, kwds, &ctxt); - if (result) { - // success: update the dispatch map for subsequent calls - if (!memoized_pc) - dispatchMap.push_back(std::make_pair(sighash, methods[i])); - else { - // debatable: apparently there are two methods that map onto the same - // sighash and preferring the latest may result in "ping pong." - for (auto& p : dispatchMap) { - if (p.first == sighash) { - p.second = methods[i]; - break; + size_t nArgs = PyVectorcall_NARGS(nargsf); + for (; i < nArgs; i++) { + PyObject* obj = cpyrt_PyArgs_GET_ITEM(args, i); + PyObject* typ = (PyObject*)Py_TYPE(obj); + // TODO: this can be a flag within CPPOverload + if ((pymeth->fMethodInfo->fName == "__getitem__") || + (pymeth->fMethodInfo->fName == "__setitem__")) { + // unpack the tuple for Python getter and setter... + // TODO: propogate this logic to TemplateProxy too + if ((typ == (PyObject*)&PyTuple_Type) && (i == (im_self ? 0 : 1))) { + for (Py_ssize_t j = 0; j < PyTuple_GET_SIZE(obj); j++) { + if (!proto.empty()) + proto += ", "; + PyObject* unpacking_obj = PyTuple_GET_ITEM(obj, j); + PyObject* unpacking_typ = (PyObject*)Py_TYPE(unpacking_obj); + if (!AddTypeName(proto, unpacking_typ, unpacking_obj, + Utility::kNone)) { + proto += "__cppjit_internal::UnknownType"; + // break; // ???: do we need to raise error here } } + continue; + } else if (i == (im_self ? 1 : 2)) + continue; + } + if (self && IsConstructor(ctxt.fFlags) && + self->ObjectIsA(/*check_smart=*/false) != + pymeth->fMethodInfo->fScope) { + // this is a constructor of cross-inherited class + interop::TCppScope_t disp = self->ObjectIsA(/*check_smart=*/false); + + // happens for Python derived types (which have a dispatcher inserted + // that is not otherwise user-visible: call it instead) and C++ derived + // classes without public constructors + + // get the dispatcher class and verify + PyObject* dispproxy = cpyrt::GetScopeProxy(disp); + if (!dispproxy) { + PyErr_SetString(PyExc_TypeError, + "dispatcher proxy was never created"); + return nullptr; } - return HandleReturn(pymeth, im_self, result); - } + if (!(((CPPClass*)dispproxy)->fFlags & CPPScope::kIsPython)) { + PyErr_SetString(PyExc_TypeError, + const_cast(("constructor for " + + interop::GetScopedFinalName(disp) + + " is not a dispatcher") + .c_str())); + return nullptr; + } - // else failure .. - if (stage != 0) { - PyErr_Clear(); // first stage errors should be the more informative - ResetCallState(pymeth->fSelf, im_self); - continue; - } + PyObject* pyobj = + cpyrt_PyObject_Call(dispproxy, args + (self == im_self ? 0 : 1), + nargsf - (self == im_self ? 0 : 1), kwds); + if (!pyobj) + return nullptr; - // collect error message/trace (automatically clears exception, too) - if (!PyErr_Occurred()) { - // this should not happen; set an error to prevent core dump and report - PyObject* sig = methods[i]->GetPrototype(); - PyErr_Format(PyExc_SystemError, "%s =>\n %s", PyUnicode_AsUTF8(sig), - (char*)"nullptr result without error in overload call"); - Py_DECREF(sig); - } + // retrieve the actual pointer, take over control, and set set + // _internal_self + intptr_t address = (intptr_t)((CPPInstance*)pyobj)->GetObject(); + if (address) { + ((CPPInstance*)pyobj) + ->CppOwns(); // b/c self will control the object on address + PyObject* res = PyObject_CallMethodObjArgs( + dispproxy, PyStrings::gDispInit, pyobj, (PyObject*)self, nullptr); + Py_XDECREF(res); + } + Py_DECREF(dispproxy); + + // return object if successful, lament if not + if (address) { + Py_INCREF(self); + + // note: constructors are no longer set to take ownership by default; + // instead that is decided by the method proxy (which carries a + // creator flag) upon return + self->Set((void*)address); + if (IsCreator(pymeth->fMethodInfo->fFlags)) + self->PythonOwns(); + + // mark as actual to prevent needless auto-casting and register on its + // class + self->fFlags |= CPPInstance::kIsActual; + if (!(((CPPClass*)Py_TYPE(self))->fFlags & CPPScope::kIsSmart)) + MemoryRegulator::RegisterPyObject( + self, interop::TCppObject_t((void*)address)); + + // handling smart types this way is deeply fugly, but if CPPInstance + // sets the proper types in op_new first, then the wrong init is + // called + if (((CPPClass*)Py_TYPE(self))->fFlags & CPPScope::kIsSmart) { + PyObject* pyclass = CreateScopeProxy( + ((CPPSmartClass*)Py_TYPE(self))->fUnderlyingType); + if (pyclass) { + self->SetSmart((PyObject*)Py_TYPE(self)); + Py_DECREF((PyObject*)Py_TYPE(self)); + Py_SET_TYPE(self, (PyTypeObject*)pyclass); + } + } - // retrieve, store, and clear errors - bool callee_error = ctxt.fFlags & (CallContext::kPyException | - CallContext::kCppException); - ctxt.fFlags &= ~(CallContext::kPyException | CallContext::kCppException); - Utility::FetchError(errors, callee_error); + // done with self + Py_DECREF(self); - if (HaveImplicit(&ctxt)) { - bHaveImplicit = true; - implicit_possible[i] = true; - ctxt.fFlags &= ~CallContext::kHaveImplicit; - } else - implicit_possible[i] = false; - ResetCallState(pymeth->fSelf, im_self); + Py_RETURN_NONE; // by definition + } + } + if (IsConstructor(ctxt.fFlags) && + interop::GetScopedFinalName(pymeth->fMethodInfo->fScope) + .find("__cppjit_internal::Dispatcher") == 0) { + if (typ == (PyObject*)&PyTuple_Type) { + if (i > 1) + proto += ", __cppjit_internal::Sep*"; + for (Py_ssize_t j = 0; j < PyTuple_GET_SIZE(obj); j++) { + if (!proto.empty()) + proto += ", "; + PyObject* unpacking_obj = PyTuple_GET_ITEM(obj, j); + PyObject* unpacking_typ = (PyObject*)Py_TYPE(unpacking_obj); + if (!AddTypeName(proto, unpacking_typ, unpacking_obj, + Utility::kNone)) { + proto += "__cppjit_internal::UnknownType"; + // break; // ???: do we need to raise error here + } + } + continue; + } + } + if (!proto.empty()) + proto += ", "; + if (!AddTypeName(proto, typ, obj, Utility::kNone)) { + proto += "__cppjit_internal::UnknownType"; + // break; // ???: do we need to raise error here + } + } + if (kwds) { + for (Py_ssize_t i = 0; i < PyTuple_GET_SIZE(kwds); i++) { + if (!proto.empty()) + proto += ", "; + PyObject* obj = cpyrt_PyArgs_GET_ITEM(args, i + nArgs); + PyObject* typ = (PyObject*)Py_TYPE(obj); + if (!AddTypeName(proto, typ, obj, Utility::kNone)) { + proto += "__cppjit_internal::UnknownType"; + break; // ???: do we need to raise error here + } + } } - - // only move forward if implicit conversions are available - if (!bHaveImplicit) - break; } - // first summarize, then add details - PyObject* topmsg = PyUnicode_FromFormat( - "none of the %d overloaded methods succeeded. Full details:", - (int)nMethods); - SetDetailedException(std::move(errors), topmsg /* steals */, - PyExc_TypeError /* default error */); - - // report failure + std::vector ambiguous_candidates; + interop::TCppMethod_t meth = interop::BestOverloadFunctionMatch( + overloads, proto, ambiguous_candidates, /*TODO:*/ nullptr, is_operator); + if (!meth && (ctxt.fFlags & CallContext::kIsConstructor)) { + // this might be a POD class/struct + // and might need the self* for the overload resolution + // to pick up the custom generated constructor defined in + // __cppjit_internal namespace + // FIXME: for the self parameter changes in CppInterOp + std::string self_type_name = ""; + if (PyVectorcall_NARGS(nargsf) > 0) { + PyObject* obj = cpyrt_PyArgs_GET_ITEM(args, 0); + PyObject* typ = (PyObject*)Py_TYPE(obj); + AddTypeName(self_type_name, typ, nullptr, Utility::kNone); + proto = self_type_name + "**, " + proto; + meth = interop::BestOverloadFunctionMatch(overloads, proto, + ambiguous_candidates, + /*TODO:*/ nullptr, is_operator); + } + } + if (meth) { + for (auto i : pymeth->fMethodInfo->fMethods) { + if (i->GetMethod() == meth) { + PyObject* result = i->Call(im_self, args, nargsf, kwds, &ctxt); + if (result) { + // success: update the dispatch map for subsequent calls + if (!memoized_pc) + dispatchMap.push_back(std::make_pair(sighash, i)); + else { + // debatable: apparently there are two methods that map onto the + // same sighash and preferring the latest may result in "ping + // pong." + for (auto& p : dispatchMap) { + if (p.first == sighash) { + p.second = i; + break; + } + } + } + return HandleReturn(pymeth, im_self, result); + } + return nullptr; + } + } + } + std::ostringstream overload_signatures; + if (ambiguous_candidates.empty()) { + for (const auto i : overloads) { + overload_signatures + << " " + << interop::GetMethodReturnTypeAsString( + i) // TODO: mention if the function is static from the list + << " " << interop::GetScopedFinalName(i.data) + << interop::GetMethodSignature(i, true) << "\n"; + } + PyErr_Format( + gOverloadResolutionException, + "Overload Resolution Failed.\nOverload set:\n%sDeduced Argument Types: " + "(%s)\n%s", + overload_signatures.str().c_str(), proto.c_str(), + is_static ? "Found static methods in the overload set. If you want to " + "invoke the static overload, try calling the function " + "using the class type instead of the instance.\n" + : ""); + } else { + for (const auto i : ambiguous_candidates) { + overload_signatures << " " << interop::GetMethodReturnTypeAsString(i) + << " " << interop::GetScopedFinalName(i.data) + << interop::GetMethodSignature(i, true) << "\n"; + } + PyErr_Format( + gOverloadAmbiguityException, + "Overload Resolution Failed. Call to \"%s\" is ambiguous.\nAmbigious " + "set:\n%sDeduced Argument Types: (%s)\n%s", + pymeth->fMethodInfo->fName.c_str(), overload_signatures.str().c_str(), + proto.c_str(), + is_static ? "Found static methods in the overload set. If you want to " + "invoke the static overload, try calling the function " + "using the class type instead of the instance.\n" + : ""); + } return nullptr; } @@ -959,11 +1121,12 @@ PyTypeObject CPPOverload_Type = { //- public members ----------------------------------------------------------- void cpyrt::CPPOverload::Set(const std::string& name, + interop::TCppScope_t scope, std::vector& methods) { // Fill in the data of a freshly created method proxy. fMethodInfo->fName = name; + fMethodInfo->fScope = scope; fMethodInfo->fMethods.swap(methods); - fMethodInfo->fFlags &= ~CallContext::kIsSorted; // special case: all constructors are considered creators by default if (name == "__init__") @@ -991,7 +1154,6 @@ void cpyrt::CPPOverload::Set(const std::string& name, void cpyrt::CPPOverload::AdoptMethod(PyCallable* pc) { // Fill in the data of a freshly created method proxy. fMethodInfo->fMethods.push_back(pc); - fMethodInfo->fFlags &= ~CallContext::kIsSorted; } //---------------------------------------------------------------------------- @@ -1001,7 +1163,6 @@ void cpyrt::CPPOverload::MergeOverload(CPPOverload* meth) { fMethodInfo->fMethods.insert(fMethodInfo->fMethods.end(), meth->fMethodInfo->fMethods.begin(), meth->fMethodInfo->fMethods.end()); - fMethodInfo->fFlags &= ~CallContext::kIsSorted; meth->fMethodInfo->fDispatchMap.clear(); meth->fMethodInfo->fMethods.clear(); } @@ -1051,7 +1212,7 @@ PyObject* cpyrt::CPPOverload::FindOverload(const std::string& signature, newmeth = mp_new(nullptr, nullptr, nullptr); CPPOverload::Methods_t vec; vec.push_back(meth->Clone()); - newmeth->Set(fMethodInfo->fName, vec); + newmeth->Set(fMethodInfo->fName, nullptr, vec); if (fSelf) { Py_INCREF(fSelf); @@ -1126,7 +1287,7 @@ PyObject* cpyrt::CPPOverload::FindOverload(PyObject* args_tuple, CPPOverload* newmeth = mp_new(nullptr, nullptr, nullptr); CPPOverload::Methods_t vec; vec.push_back(methods[best_method]->Clone()); - newmeth->Set(fMethodInfo->fName, vec); + newmeth->Set(fMethodInfo->fName, nullptr, vec); if (fSelf) { Py_INCREF(fSelf); diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.h b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.h index 87019d26f687b..7d17e00f02d5c 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.h @@ -3,6 +3,7 @@ // Bindings #include "PyCallable.h" +#include "cppjit_interop.h" // Standard #include @@ -12,35 +13,6 @@ namespace cppjit::cpyrt { -// signature hashes are also used by TemplateProxy -inline uint64_t HashSignature(cpyrt_PyArgs_t args, size_t nargsf) { - // Build a hash from the types of the given python function arguments. - uint64_t hash = 0; - - Py_ssize_t nargs = cpyrt_PyArgs_GET_SIZE(args, nargsf); - for (Py_ssize_t i = 0; i < nargs; ++i) { - // TODO: hashing in the ref-count is for moves; resolve this together with - // the improved overloads for implicit conversions - PyObject* pyobj = cpyrt_PyArgs_GET_ITEM(args, i); - hash += (uint64_t)Py_TYPE(pyobj); -#if PY_VERSION_HEX >= 0x030e0000 - hash += - (uint64_t)(PyUnstable_Object_IsUniqueReferencedTemporary(pyobj) ? 1 - : 0); -#else - hash += (uint64_t)(Py_REFCNT(pyobj) == 1 ? 1 : 0); -#endif - hash += (hash << 10); - hash ^= (hash >> 6); - } - - hash += (hash << 3); - hash ^= (hash >> 11); - hash += (hash << 15); - - return hash; -} - class CPPOverload { public: typedef std::vector> DispatchMap_t; @@ -56,6 +28,7 @@ class CPPOverload { CPPOverload::DispatchMap_t fDispatchMap; CPPOverload::Methods_t fMethods; PyObject* fDoc; + interop::TCppScope_t fScope; uint32_t fFlags; int* fRefCount; @@ -66,7 +39,8 @@ class CPPOverload { }; public: - void Set(const std::string& name, std::vector& methods); + void Set(const std::string& name, interop::TCppScope_t scope, + std::vector& methods); void AdoptMethod(PyCallable* pc); void MergeOverload(CPPOverload* meth); @@ -101,20 +75,56 @@ template inline bool CPPOverload_CheckExact(T* object) { //- creation ----------------------------------------------------------------- inline CPPOverload* CPPOverload_New(const std::string& name, + interop::TCppScope_t scope, std::vector& methods) { // Create and initialize a new method proxy from the overloads. CPPOverload* pymeth = (CPPOverload*)CPPOverload_Type.tp_new(&CPPOverload_Type, nullptr, nullptr); - pymeth->Set(name, methods); + pymeth->Set(name, scope, methods); return pymeth; } inline CPPOverload* CPPOverload_New(const std::string& name, + interop::TCppScope_t scope, PyCallable* method) { // Create and initialize a new method proxy from the method. std::vector p; p.push_back(method); - return CPPOverload_New(name, p); + return CPPOverload_New(name, scope, p); +} + +// signature hashes are also used by TemplateProxy +inline uint64_t HashSignature(cpyrt_PyArgs_t args, size_t nargsf) { + // Build a hash from the types of the given python function arguments. + uint64_t hash = 0; + + Py_ssize_t nargs = cpyrt_PyArgs_GET_SIZE(args, nargsf); + for (Py_ssize_t i = 0; i < nargs; ++i) { + // TODO: hashing in the ref-count is for moves; resolve this together with + // the improved overloads for implicit conversions + PyObject* pyobj = cpyrt_PyArgs_GET_ITEM(args, i); + hash += (uint64_t)Py_TYPE(pyobj); +#if PY_VERSION_HEX >= 0x030e0000 + hash += + (uint64_t)(PyUnstable_Object_IsUniqueReferencedTemporary(pyobj) ? 1 + : 0); +#else + hash += (uint64_t)(Py_REFCNT(pyobj) == 1 ? 1 : 0); +#endif + if (CPPOverload_CheckExact(pyobj)) { + static std::hash hash_string; + CPPOverload* overload = (CPPOverload*)pyobj; + hash += hash_string(overload->fMethodInfo->fName); + } + hash += (hash << 10); + hash ^= (hash >> 6); + } + + hash += (hash << 3); + hash ^= (hash >> 11); + hash += (hash << 15); + + return hash; } } // namespace cppjit::cpyrt diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx index dd5801abed713..26b47450e7dec 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx @@ -50,13 +50,8 @@ add_template(PyObject* pyclass, const std::string& name, } if (overloads) { - // adopt the new overloads - if (ncl != name) - for (auto clb : *overloads) - pytmpl->AdoptTemplate(clb); - else - for (auto clb : *overloads) - pytmpl->AdoptMethod(clb); + for (auto clb : *overloads) + pytmpl->AdoptMethod(clb); } // the caller expects a method matching the full name, thus is a @@ -401,7 +396,7 @@ static PyObject* meta_getattro(PyObject* pyclass, PyObject* pyname) { if (interop::ExistsMethodTemplate(scope, name)) attr = add_template(pyclass, name, &overloads); else - attr = (PyObject*)CPPOverload_New(name, overloads); + attr = (PyObject*)CPPOverload_New(name, scope, overloads); templated_functions_checked = true; } } diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPScope.h b/bindings/pyroot/cppjit/src/cpyrt/CPPScope.h index 04670c113252e..012be7ca3d529 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPScope.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPScope.h @@ -23,6 +23,8 @@ typedef struct { #endif // Standard +#include "Python.h" +#include "cppjit_interop.h" #include #include diff --git a/bindings/pyroot/cppjit/src/cpyrt/CallContext.h b/bindings/pyroot/cppjit/src/cpyrt/CallContext.h index f8000a0d82ef3..cc3bf232ee10d 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CallContext.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CallContext.h @@ -30,7 +30,9 @@ struct CallContext { enum ECallFlags { kNone = 0x000000, - kIsSorted = 0x000001, // if method overload priority determined + // UNUSED with new overload resolution + // kIsSorted = 0x000001, // if method overload priority + // determined kIsCreator = 0x000002, // if method creates python-owned objects kIsConstructor = 0x000004, // if method is a C++ constructor kHaveImplicit = 0x000008, // indicate that implicit converters are available @@ -40,15 +42,18 @@ struct CallContext { kFromDescr = 0x000080, // initiated from a descriptor kUseHeuristics = 0x000100, // if method applies heuristics memory policy kImplicitSmartPtrConversion = - 0x000200, // enable implicit conversion to smart pointers - kReleaseGIL = 0x000400, // if method should release the GIL - kSetLifeLine = 0x000800, // if return value is part of 'this' - kNeverLifeLine = 0x001000, // if the return value is never part of 'this' - kPyException = 0x002000, // Python exception during method execution - kCppException = 0x004000, // C++ exception during method execution - kProtected = 0x008000, // if method should return on signals - kUseFFI = 0x010000, // not implemented - kIsPseudoFunc = 0x020000, // internal, used for introspection + 0x000200, // enable implicit conversion to smart pointers + kReleaseGIL = 0x000400, // if method should release the GIL + kSetLifeLine = 0x000800, // if return value is part of 'this' + kNeverLifeLine = 0x001000, // if the return value is never part of 'this' + kPyException = 0x002000, // Python exception during method execution + kCppException = 0x004000, // C++ exception during method execution + kProtected = 0x008000, // if method should return on signals + kUseFFI = 0x010000, // not implemented + kIsPseudoFunc = 0x020000, // internal, used for introspection + kUseStrict = 0x040000, // if method applies strict memory policy + kIsMethod = 0x080000, // If this is a class method + kIsSatticMethod = 0x100000, // If this is a static class method }; // Policies about memory handling and signal safety @@ -100,8 +105,6 @@ struct CallContext { Temporary* fTemps; }; -inline bool IsSorted(uint64_t flags) { return flags & CallContext::kIsSorted; } - inline bool IsCreator(uint64_t flags) { return flags & CallContext::kIsCreator; } diff --git a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx index da661fa383c45..b03108ef9e508 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx @@ -2483,7 +2483,12 @@ bool cpyrt::InstanceRefConverter::SetArg(PyObject* pyobject, Parameter& para, interop::TCppScope_t cls; if (pyobj->IsSmart()) { cls = pyobj->ObjectIsA(false); - if (cls && interop::IsSubclass(cls, fClass)) { + interop::TCppScope_t deref_cls = nullptr; + interop::TCppScope_t deref_fClass = nullptr; + interop::GetSmartPtrInfo(cls, &deref_cls, nullptr); + interop::GetSmartPtrInfo(fClass, &deref_fClass, nullptr); + if (deref_cls && deref_fClass && + interop::IsSubclass(deref_cls, deref_fClass)) { para.fValue.fVoidp = pyobj->GetObjectRaw(); argset = true; } @@ -2506,7 +2511,7 @@ bool cpyrt::InstanceRefConverter::SetArg(PyObject* pyobject, Parameter& para, } // calculate offset between formal and actual arguments - if (cls != fClass) { + if ((cls != fClass) && (!pyobj->IsSmart())) { para.fValue.fIntPtr += interop::GetBaseOffset( cls, fClass, para.fValue.fVoidp, 1 /* up-cast */); } @@ -2896,10 +2901,12 @@ static void* PyFunction_AsCPointer(PyObject* pyobject, if (pytmpl->fTemplateArgs) fullname += PyUnicode_AsUTF8(pytmpl->fTemplateArgs); interop::TCppScope_t scope = ((CPPClass*)pytmpl->fTI->fPyClass)->fCppType; + std::string ret{}, sig{}; GetSignatureFromFnType(fn_type, ret, sig); + std::vector ambiguous_candidates; interop::TCppMethod_t cppmeth = interop::GetMethodTemplate( - scope, fullname, sig.substr(1, sig.size() - 2)); + scope, fullname, sig.substr(1, sig.size() - 2), ambiguous_candidates); if (cppmeth) { void* fptr = (void*)interop::GetFunctionAddress(cppmeth, false); if (fptr) diff --git a/bindings/pyroot/cppjit/src/cpyrt/Dispatcher.cxx b/bindings/pyroot/cppjit/src/cpyrt/Dispatcher.cxx index 40b1dbdfa74b7..dcfbd2071a5df 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Dispatcher.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Dispatcher.cxx @@ -524,6 +524,7 @@ bool cpyrt::InsertDispatcher(CPPScope* klass, PyObject* bases, PyObject* dct, code << "};\n}"; // finally, compile the code + // printf("\n\n%s\n\n", code.str().c_str()); if (!interop::Compile(code.str())) { err << "failed to compile the dispatcher code"; return false; diff --git a/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.cxx b/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.cxx index d4d81fa536a43..94e06850af62d 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.cxx @@ -1095,6 +1095,7 @@ CreateLowLevelViewT(T* address, cpyrt::cdims_t shape, PyObject* args = PyTuple_New(0); LowLevelView* llp = (LowLevelView*)LowLevelView_Type.tp_new( &LowLevelView_Type, args, nullptr); + llp->fCppTypeName = (format ? "void" : typecode_traits::name); Py_DECREF(args); Py_buffer& view = llp->fBufInfo; diff --git a/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.h b/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.h index 80e7f02bcc4de..7d828af3c5682 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.h +++ b/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.h @@ -27,6 +27,7 @@ class LowLevelView { void** fBuf; Converter* fConverter; Converter* fElemCnv; + const char* fCppTypeName; typedef LowLevelView* (*Creator_t)(void*, cdims_t); Creator_t fCreator; // for slicing, which requires copying diff --git a/bindings/pyroot/cppjit/src/cpyrt/ProxyWrappers.cxx b/bindings/pyroot/cppjit/src/cpyrt/ProxyWrappers.cxx index 0b8f9c5fcf7f6..712d96f40d5bb 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/ProxyWrappers.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/ProxyWrappers.cxx @@ -155,7 +155,8 @@ static inline void sync_templates(PyObject* pyclass, PyErr_Clear(); Py_DECREF(dct); if (!TemplateProxy_Check(attr)) { - TemplateProxy* pytmpl = TemplateProxy_New(mtCppName, mtName, pyclass); + TemplateProxy* pytmpl = + TemplateProxy_New(mtCppName, mtName, pyclass, mtName == "__init__"); if (CPPOverload_Check(attr)) pytmpl->MergeOverload((CPPOverload*)attr); PyType_Type.tp_setattro(pyclass, pyname, (PyObject*)pytmpl); @@ -285,10 +286,7 @@ static int BuildScopeProxyDict(interop::TCppScope_t scope, PyObject* pyclass, // add only here, not to the cache of collected methods PyObject* attr = PyObject_GetAttrString(pyclass, const_cast(mtName.c_str())); - if (isTemplate) - ((TemplateProxy*)attr)->AdoptTemplate(pycall); - else - ((TemplateProxy*)attr)->AdoptMethod(pycall); + ((TemplateProxy*)attr)->AdoptMethod(pycall); Py_DECREF(attr); // for operator[]/() that returns by ref, also add __setitem__ @@ -307,10 +305,7 @@ static int BuildScopeProxyDict(interop::TCppScope_t scope, PyObject* pyclass, PyObject_SetAttrString(pyclass, const_cast("__setitem__"), (PyObject*)pysi); } - if (isTemplate) - pysi->AdoptTemplate(new CPPSetItem(scope, method)); - else - pysi->AdoptMethod(new CPPSetItem(scope, method)); + pysi->AdoptMethod(new CPPSetItem(scope, method)); Py_XDECREF(pysi); } @@ -410,7 +405,7 @@ static int BuildScopeProxyDict(interop::TCppScope_t scope, PyObject* pyclass, if (!attr) PyErr_Clear(); // normal case, add a new method - CPPOverload* method = CPPOverload_New(imd->first, imd->second); + CPPOverload* method = CPPOverload_New(imd->first, scope, imd->second); PyObject* pymname = PyUnicode_InternFromString( const_cast(method->GetName().c_str())); PyType_Type.tp_setattro(pyclass, pymname, (PyObject*)method); diff --git a/bindings/pyroot/cppjit/src/cpyrt/PyCallable.h b/bindings/pyroot/cppjit/src/cpyrt/PyCallable.h index f9a64de44bd48..5cc951c32ff48 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/PyCallable.h +++ b/bindings/pyroot/cppjit/src/cpyrt/PyCallable.h @@ -6,10 +6,14 @@ // Bindings #include "CallContext.h" #include "cppjit_interop.h" +#include "cpyrt.h" #include "cpyrt/Reflex.h" namespace cppjit::cpyrt { +extern PyObject* gOverloadResolutionException; +extern PyObject* gOverloadAmbiguityException; + class CPPInstance; class PyCallable { @@ -17,6 +21,7 @@ class PyCallable { virtual ~PyCallable() {} public: + virtual interop::TCppMethod_t GetMethod() { return nullptr; } virtual PyObject* GetSignature(bool show_formalargs = true) = 0; virtual PyObject* GetSignatureNames() = 0; virtual PyObject* GetSignatureTypes() = 0; @@ -31,9 +36,6 @@ class PyCallable { return nullptr; }; - virtual int GetPriority() = 0; - virtual bool IsGreedy() = 0; - virtual int GetMaxArgs() = 0; virtual PyObject* GetCoVarNames() = 0; virtual PyObject* GetArgDefault(int /* iarg */, bool silent = true) = 0; diff --git a/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx b/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx index 4fc68fbde55c3..d59eaa768499d 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx @@ -1,5 +1,6 @@ // Bindings #include "cpyrt.h" +#include "unicodeobject.h" using namespace cppjit; #include "CPPClassMethod.h" @@ -15,6 +16,7 @@ using namespace cppjit; // Standard #include +#include namespace cppjit::cpyrt { @@ -26,8 +28,7 @@ static inline std::string targs2str(TemplateProxy* pytmpl) { //---------------------------------------------------------------------------- TemplateInfo::TemplateInfo() - : fPyClass(nullptr), fNonTemplated(nullptr), fTemplated(nullptr), - fLowPriority(nullptr), fDoc(nullptr) { + : fPyClass(nullptr), fOverloads(nullptr), fDoc(nullptr) { /* empty */ } @@ -36,9 +37,7 @@ TemplateInfo::~TemplateInfo() { Py_XDECREF(fPyClass); Py_XDECREF(fDoc); - Py_DECREF(fNonTemplated); - Py_DECREF(fTemplated); - Py_DECREF(fLowPriority); + Py_DECREF(fOverloads); for (const auto& p : fDispatchMap) { for (const auto& c : p.second) { @@ -50,51 +49,44 @@ TemplateInfo::~TemplateInfo() { //---------------------------------------------------------------------------- void TemplateProxy::MergeOverload(CPPOverload* mp) { // Store overloads of this templated method. - bool isGreedy = false; - for (auto pc : mp->fMethodInfo->fMethods) { - if (pc->IsGreedy()) { - isGreedy = true; - break; - } - } - - CPPOverload* cppol = isGreedy ? fTI->fLowPriority : fTI->fNonTemplated; - cppol->MergeOverload(mp); + fTI->fOverloads->MergeOverload(mp); } void TemplateProxy::AdoptMethod(PyCallable* pc) { // Store overload of this templated method. - CPPOverload* cppol = pc->IsGreedy() ? fTI->fLowPriority : fTI->fNonTemplated; - cppol->AdoptMethod(pc); -} - -void TemplateProxy::AdoptTemplate(PyCallable* pc) { - // Store known template methods. - fTI->fTemplated->AdoptMethod(pc); + fTI->fOverloads->AdoptMethod(pc); } //---------------------------------------------------------------------------- PyObject* TemplateProxy::Instantiate(const std::string& fname, cpyrt_PyArgs_t args, size_t nargsf, - Utility::ArgPreference pref, int* pcnt) { + Utility::ArgPreference pref, int* pcnt, + bool include_non_templated) { // Instantiate (and cache) templated methods, return method if any std::string proto = ""; std::string protoPair = ""; + bool skipSelf = fTI->fIsConstructor && (!fSelf || fSelf == Py_None); + if (fSelf && (fSelf != Py_None) && (!fTI->fIsConstructor)) { + PyObject* self = (PyObject*)fSelf; + assert(AddTypeName(proto, (PyObject*)Py_TYPE(self), self, Utility::kNone)); + } // adjust arguments for self if this is a rebound global function - bool isNS = (((CPPScope*)fTI->fPyClass)->fFlags & CPPScope::kIsNamespace); - if (!isNS && cpyrt_PyArgs_GET_SIZE(args, nargsf) && - (!fSelf || (fSelf == Py_None && - !interop::IsStaticTemplate( - ((CPPScope*)fTI->fPyClass)->fCppType, fname)))) { - args += 1; - nargsf -= 1; - } + // bool isNS = (((CPPScope*)fTI->fPyClass)->fFlags & CPPScope::kIsNamespace); + // if (!fTI->fIsConstructor && !isNS && cpyrt_PyArgs_GET_SIZE(args, nargsf) && + // \ + // (!fSelf || + // (fSelf == Py_None && + // !interop::IsStaticTemplate(((CPPScope*)fTI->fPyClass)->fCppType, + // fname)))) { + // args += 1; + // nargsf -= 1; + // } Py_ssize_t argc = cpyrt_PyArgs_GET_SIZE(args, nargsf); if (argc != 0) { - PyObject* tpArgs = PyTuple_New(argc); - for (Py_ssize_t i = 0; i < argc; ++i) { + PyObject* tpArgs = PyTuple_New(skipSelf ? argc - 1 : argc); + for (Py_ssize_t i = skipSelf ? 1 : 0, j = 0; i < argc; ++i, ++j) { PyObject* itemi = cpyrt_PyArgs_GET_ITEM(args, i); bool bArgSet = false; @@ -104,7 +96,7 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, TemplateProxy* tn = (TemplateProxy*)itemi; PyObject* f = PyUnicode_FromFormat("%s%s", tn->fTI->fCppName.c_str(), targs2str(tn).c_str()); - PyTuple_SET_ITEM(tpArgs, i, f); + PyTuple_SET_ITEM(tpArgs, j, f); bArgSet = true; } PyObject* pytc = nullptr; @@ -123,7 +115,7 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, PyObject* pyptrname = Utility::CT2CppName(pytc, ptrdef.c_str(), true); if (pyptrname) { - PyTuple_SET_ITEM(tpArgs, i, pyptrname); + PyTuple_SET_ITEM(tpArgs, j, pyptrname); bArgSet = true; // string added, but not counted towards nStrings } @@ -151,7 +143,7 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, Py_XDECREF(pytc); pytc = nullptr; if (pyactname) { - PyTuple_SET_ITEM(tpArgs, i, pyactname); + PyTuple_SET_ITEM(tpArgs, j, pyactname); bArgSet = true; // string added, but not counted towards nStrings } @@ -163,18 +155,19 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, PyErr_Clear(); PyObject* tp = (PyObject*)Py_TYPE(itemi); Py_INCREF(tp); - PyTuple_SET_ITEM(tpArgs, i, tp); + PyTuple_SET_ITEM(tpArgs, j, tp); } } - PyObject* pyargs = PyTuple_New(argc); - for (Py_ssize_t i = 0; i < argc; ++i) { + PyObject* pyargs = PyTuple_New(skipSelf ? argc - 1 : argc); + for (Py_ssize_t i = skipSelf ? 1 : 0, j = 0; i < argc; ++i, ++j) { PyObject* item = cpyrt_PyArgs_GET_ITEM(args, i); Py_INCREF(item); - PyTuple_SET_ITEM(pyargs, i, item); + PyTuple_SET_ITEM(pyargs, j, item); } - const std::string& name_v1 = - Utility::ConstructTemplateArgs(nullptr, tpArgs, pyargs, pref, 0, pcnt); + const std::string& name_v1 = Utility::ConstructTemplateArgs( + nullptr, tpArgs, pyargs, pref, 0, + pcnt /*, usingThisParameter=??? */); // FIXME // A Python 2-tuple is typed above as std::initializer_list, but it may // equally represent a std::pair, to which an initializer_list can not @@ -221,7 +214,8 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, nullptr, tpArgsPair, pyargs, pref, 0, nullptr); Py_DECREF(tpArgsPair); if (!PyErr_Occurred() && name_v2.size()) - protoPair = name_v2.substr(1, name_v2.size() - 2); + protoPair = (proto.empty() ? "" : proto + ", ") + + name_v2.substr(1, name_v2.size() - 2); else PyErr_Clear(); } @@ -236,17 +230,24 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, return nullptr; } - if (name_v1.size()) - proto = name_v1.substr(1, name_v1.size() - 2); + if (name_v1.size()) { + if (!proto.empty()) + proto.append(", "); + proto.append(name_v1.substr(1, name_v1.size() - 2)); + } } // the following causes instantiation as necessary interop::TCppScope_t scope = ((CPPClass*)fTI->fPyClass)->fCppType; - interop::TCppMethod_t cppmeth = - interop::GetMethodTemplate(scope, fname, proto); + std::vector ambiguous_candidates; + interop::TCppMethod_t cppmeth = interop::GetMethodTemplate( + scope, fname, proto, ambiguous_candidates, include_non_templated); if (!cppmeth && !protoPair.empty() && protoPair != proto) { // retry with 2-tuples typed as std::pair instead of std::initializer_list - cppmeth = interop::GetMethodTemplate(scope, fname, protoPair); + ambiguous_candidates.clear(); + cppmeth = interop::GetMethodTemplate(scope, fname, protoPair, + ambiguous_candidates, + include_non_templated); if (cppmeth) proto = protoPair; } @@ -277,8 +278,8 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, pos = proto.find("initializer_list", pos + 6); } - interop::TCppMethod_t m2 = - interop::GetMethodTemplate(scope, fname, proto); + interop::TCppMethod_t m2 = interop::GetMethodTemplate( + scope, fname, proto, ambiguous_candidates, include_non_templated); if (m2 && m2 != cppmeth) { // replace if the new method with vector was found; otherwise just // continue with the previously found method with initializer_list. @@ -327,7 +328,7 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, // Case 1/2: method simply did not exist before if (!pyol) { // actual overload to use (now owns meth) - pyol = (PyObject*)CPPOverload_New(fname, meth); + pyol = (PyObject*)CPPOverload_New(fname, scope, meth); if (bIsConstructor) { // TODO: this is an ugly hack :( ((CPPOverload*)pyol)->fMethodInfo->fFlags |= @@ -349,16 +350,16 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, // Case 5: must be a template proxy, meaning that current template name is // not a template overload else if (bIsCppTP) { - ((TemplateProxy*)pyol)->AdoptTemplate(meth->Clone()); + ((TemplateProxy*)pyol)->AdoptMethod(meth->Clone()); Py_DECREF(pyol); - pyol = (PyObject*)CPPOverload_New(fname, meth); // takes ownership + pyol = (PyObject*)CPPOverload_New(fname, scope, meth); // takes ownership } // Case 6: pre-existing object is not a CPPOverload nor TemplateProxy // we do not cache it, as this might be a pythonization (monkey-patched // func/method) else { Py_DECREF(pyol); - pyol = (PyObject*)CPPOverload_New(fname, meth); + pyol = (PyObject*)CPPOverload_New(fname, scope, meth); } // Special Case if name was aliased (e.g. typedef in template instantiation) @@ -454,30 +455,8 @@ static PyObject* tpp_doc(TemplateProxy* pytmpl, void*) { // Forward to method proxies to doc all overloads PyObject* doc = nullptr; - if (pytmpl->fTI->fNonTemplated->HasMethods()) - doc = PyObject_GetAttrString((PyObject*)pytmpl->fTI->fNonTemplated, - "__doc__"); - if (pytmpl->fTI->fTemplated->HasMethods()) { - PyObject* doc2 = - PyObject_GetAttrString((PyObject*)pytmpl->fTI->fTemplated, "__doc__"); - if (doc && doc2) { - PyUnicode_AppendAndDel(&doc, PyUnicode_FromString("\n")); - PyUnicode_AppendAndDel(&doc, doc2); - } else if (!doc && doc2) { - doc = doc2; - } - } - if (pytmpl->fTI->fLowPriority->HasMethods()) { - PyObject* doc2 = - PyObject_GetAttrString((PyObject*)pytmpl->fTI->fLowPriority, "__doc__"); - if (doc && doc2) { - PyUnicode_AppendAndDel(&doc, PyUnicode_FromString("\n")); - PyUnicode_AppendAndDel(&doc, doc2); - } else if (!doc && doc2) { - doc = doc2; - } - } - + if (pytmpl->fTI->fOverloads->HasMethods()) + doc = PyObject_GetAttrString((PyObject*)pytmpl->fTI->fOverloads, "__doc__"); if (doc) return doc; @@ -685,8 +664,8 @@ static PyObject* tpp_vectorcall(TemplateProxy* pytmpl, PyObject* const* args, PyErr_Clear(); // not cached or failed call; try instantiation - pymeth = pytmpl->Instantiate(PyUnicode_AsUTF8(pyfullname), args, nargsf, - Utility::kNone); + pymeth = pytmpl->Instantiate(PyUnicode_AsUTF8(pyfullname), args, + nargsf, Utility::kNone, nullptr, false); if (pymeth) { // attempt actual call; same as above, allow implicit conversion of // arguments @@ -711,56 +690,30 @@ static PyObject* tpp_vectorcall(TemplateProxy* pytmpl, PyObject* const* args, return nullptr; } - // case 2: select known non-template overload - result = SelectAndForward(pytmpl, pytmpl->fTI->fNonTemplated, args, nargsf, - kwds, true /* implicitOkay */, - false /* use_targs */, sighash, errors); - if (result) - TPPCALL_RETURN; - - // case 3: select known template overload - result = SelectAndForward(pytmpl, pytmpl->fTI->fTemplated, args, nargsf, kwds, - false /* implicitOkay */, true /* use_targs */, - sighash, errors); - if (result) - TPPCALL_RETURN; - - // case 4: auto-instantiation from types of arguments - for (auto pref : {Utility::kReference, Utility::kPointer, Utility::kValue}) { - // TODO: no need to loop if there are no non-instance arguments; also, - // should any failed lookup be removed? - int pcnt = 0; - pymeth = - pytmpl->Instantiate(pytmpl->fTI->fCppName, args, nargsf, pref, &pcnt); - if (pymeth) { - // attempt actual call; even if argument based, allow implicit - // conversions, for example for non-template arguments - result = CallMethodImp(pytmpl, pymeth, args, nargsf, kwds, /*impOK=*/true, - sighash); - if (result) - TPPCALL_RETURN; - } - Utility::FetchError(errors); - if (!pcnt) - break; // preference never used; no point trying others + // case 2: create a template instantiation + int pcnt = 0; // FIXME: this is unused and can be removed + pymeth = pytmpl->Instantiate(pytmpl->fTI->fCppName, args, nargsf, + Utility::kReference, &pcnt, true); + if (pymeth) { + // attempt actual call; even if argument based, allow implicit conversions, + // for example for non-template arguments + result = CallMethodImp(pytmpl, pymeth, args, nargsf, kwds, /*impOK=*/true, + sighash); + if (result) + TPPCALL_RETURN; } + Utility::FetchError(errors); - // case 5: low priority methods, such as ones that take void* arguments - result = SelectAndForward(pytmpl, pytmpl->fTI->fLowPriority, args, nargsf, - kwds, false /* implicitOkay */, - false /* use_targs */, sighash, errors); - if (result) - TPPCALL_RETURN; - - // error reporting is fraud, given the numerous steps taken, but more details - // seems better + // error reporting is fraud, given the numerous steps taken, but more + // details seems better if (!errors.empty()) { PyObject* topmsg = - PyUnicode_FromString("Template method resolution failed:"); - Utility::SetDetailedException(std::move(errors), topmsg /* steals */, - PyExc_TypeError /* default error */); + PyUnicode_InternFromString("Template method resolution failed:"); + Utility::SetDetailedException( + std::move(errors), topmsg /* steals */, + gOverloadResolutionException /* default error */); } else { - PyErr_Format(PyExc_TypeError, + PyErr_Format(gOverloadResolutionException, "cannot resolve method template call for \'%s\'", pytmpl->fTI->fCppName.c_str()); } @@ -855,7 +808,7 @@ static PyGetSetDef tpp_getset[] = { //---------------------------------------------------------------------------- void TemplateProxy::Set(const std::string& cppname, const std::string& pyname, - PyObject* pyclass) { + PyObject* pyclass, bool is_constructor) { // Initialize the proxy for the given 'pyclass.' fSelf = nullptr; fTemplateArgs = nullptr; @@ -863,11 +816,11 @@ void TemplateProxy::Set(const std::string& cppname, const std::string& pyname, fTI->fCppName = cppname; Py_XINCREF(pyclass); fTI->fPyClass = pyclass; + fTI->fIsConstructor = is_constructor; std::vector dummy; - fTI->fNonTemplated = CPPOverload_New(pyname, dummy); - fTI->fTemplated = CPPOverload_New(pyname, dummy); - fTI->fLowPriority = CPPOverload_New(pyname, dummy); + fTI->fOverloads = + CPPOverload_New(pyname, nullptr, dummy); // FIXME: should not be nullptr fVectorCall = (vectorcallfunc)tpp_vectorcall; } @@ -883,23 +836,17 @@ static PyObject* tpp_overload(TemplateProxy* pytmpl, PyObject* args) { interop::TCppScope_t scope = nullptr; interop::TCppMethod_t cppmeth = nullptr; std::string proto; + std::vector ambiguous_candidates; if (PyArg_ParseTuple(args, const_cast("s|i:__overload__"), &sigarg, &want_const)) { want_const = PyTuple_GET_SIZE(args) == 1 ? -1 : want_const; // check existing overloads in order - PyObject* ol = pytmpl->fTI->fNonTemplated->FindOverload(sigarg, want_const); + PyObject* ol = pytmpl->fTI->fOverloads->FindOverload(sigarg, want_const); if (ol) return ol; PyErr_Clear(); - ol = pytmpl->fTI->fTemplated->FindOverload(sigarg, want_const); - if (ol) - return ol; - PyErr_Clear(); - ol = pytmpl->fTI->fLowPriority->FindOverload(sigarg, want_const); - if (ol) - return ol; proto = Utility::ConstructTemplateArgs(nullptr, args); // Propagate the error that occurs if we can't construct the C++ name @@ -910,14 +857,15 @@ static PyObject* tpp_overload(TemplateProxy* pytmpl, PyObject* args) { scope = ((CPPClass*)pytmpl->fTI->fPyClass)->fCppType; cppmeth = interop::GetMethodTemplate(scope, pytmpl->fTI->fCppName, - proto.substr(1, proto.size() - 2)); + proto.substr(1, proto.size() - 2), + ambiguous_candidates); // ???: include_non_templated } else if (PyArg_ParseTuple(args, const_cast("ss:__overload__"), &sigarg, &tmplarg)) { scope = ((CPPClass*)pytmpl->fTI->fPyClass)->fCppType; std::string full_name = std::string(pytmpl->fTI->fCppName) + "<" + tmplarg + ">"; - cppmeth = interop::GetMethodTemplate(scope, full_name, sigarg); + cppmeth = interop::GetMethodTemplate(scope, full_name, sigarg, ambiguous_candidates); // ???: include_non_templated } else if (PyArg_ParseTuple(args, const_cast("O|i:__overload__"), &sigarg_tuple, &want_const)) { PyErr_Clear(); @@ -925,17 +873,10 @@ static PyObject* tpp_overload(TemplateProxy* pytmpl, PyObject* args) { // check existing overloads in order PyObject* ol = - pytmpl->fTI->fNonTemplated->FindOverload(sigarg_tuple, want_const); + pytmpl->fTI->fOverloads->FindOverload(sigarg_tuple, want_const); if (ol) return ol; PyErr_Clear(); - ol = pytmpl->fTI->fTemplated->FindOverload(sigarg_tuple, want_const); - if (ol) - return ol; - PyErr_Clear(); - ol = pytmpl->fTI->fLowPriority->FindOverload(sigarg_tuple, want_const); - if (ol) - return ol; proto.reserve(128); proto.push_back('<'); @@ -955,7 +896,8 @@ static PyObject* tpp_overload(TemplateProxy* pytmpl, PyObject* args) { scope = ((CPPClass*)pytmpl->fTI->fPyClass)->fCppType; cppmeth = interop::GetMethodTemplate(scope, pytmpl->fTI->fCppName, - proto.substr(1, proto.size() - 2)); + proto.substr(1, proto.size() - 2), + ambiguous_candidates); } else { PyErr_Format(PyExc_TypeError, "Unexpected arguments to __overload__"); return nullptr; @@ -979,7 +921,7 @@ static PyObject* tpp_overload(TemplateProxy* pytmpl, PyObject* args) { } else meth = new CPPMethod(scope, cppmeth); - return (PyObject*)CPPOverload_New(pytmpl->fTI->fCppName + proto, meth); + return (PyObject*)CPPOverload_New(pytmpl->fTI->fCppName + proto, scope, meth); } static PyMethodDef tpp_methods[] = {{(char*)"__overload__", diff --git a/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.h b/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.h index ff9ad99911ef7..d2a926959594d 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.h +++ b/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.h @@ -34,12 +34,10 @@ class TemplateInfo { public: std::string fCppName; PyObject* fPyClass; - CPPOverload* fNonTemplated; // holder for non-template overloads - CPPOverload* fTemplated; // holder for templated overloads - CPPOverload* fLowPriority; // low priority overloads such as void*/void** - + CPPOverload* fOverloads; // holder for all overloads TP_DispatchMap_t fDispatchMap; PyObject* fDoc; + bool fIsConstructor; }; typedef std::shared_ptr TP_TInfo_t; @@ -48,9 +46,10 @@ class TemplateProxy { private: friend TemplateProxy* TemplateProxy_New(const std::string& cppname, const std::string& pyname, - PyObject* pyclass); + PyObject* pyclass, + bool isConstructor); void Set(const std::string& cppname, const std::string& pyname, - PyObject* pyclass); + PyObject* pyclass, bool is_constructor); public: // public, as the python C-API works with C structs PyObject_HEAD PyObject* fSelf; // must be first (same layout as CPPOverload) @@ -62,10 +61,10 @@ class TemplateProxy { public: void MergeOverload(CPPOverload* mp); void AdoptMethod(PyCallable* pc); - void AdoptTemplate(PyCallable* pc); PyObject* Instantiate(const std::string& fname, cpyrt_PyArgs_t tmplArgs, size_t nargsf, Utility::ArgPreference, - int* pcnt = nullptr); + int* pcnt = nullptr, + bool include_non_templated = false); private: // private, as the python C-API will handle creation TemplateProxy() = delete; @@ -85,14 +84,15 @@ template inline bool TemplateProxy_CheckExact(T* object) { //- creation ----------------------------------------------------------------- inline TemplateProxy* TemplateProxy_New(const std::string& cppname, const std::string& pyname, - PyObject* pyclass) { + PyObject* pyclass, + bool isConstructor = false) { // Create and initialize a new template method proxy for the class. if (!CPPScope_Check(pyclass)) return nullptr; TemplateProxy* pytmpl = (TemplateProxy*)TemplateProxy_Type.tp_new( &TemplateProxy_Type, nullptr, nullptr); - pytmpl->Set(cppname, pyname, pyclass); + pytmpl->Set(cppname, pyname, pyclass, isConstructor); return pytmpl; } diff --git a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx index 9538a57b46260..208610c60bcee 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx @@ -2,16 +2,19 @@ #include "cpyrt.h" using namespace cppjit; +#include "CPPExcInstance.h" #include "CPPFunction.h" #include "CPPInstance.h" #include "CPPOverload.h" #include "CustomPyTypes.h" +#include "LowLevelViews.h" #include "ProxyWrappers.h" #include "PyCallable.h" #include "PyStrings.h" #include "TemplateProxy.h" #include "TypeManip.h" #include "Utility.h" +#include "cppjit_interop.h" #include "cpyrt/DispatchPtr.h" // Standard @@ -23,6 +26,7 @@ using namespace cppjit; #include #include #include +#include //- data _____________________________________________________________________ typedef std::unordered_map TC2POperatorMapping_t; @@ -30,6 +34,8 @@ static TC2POperatorMapping_t gC2POperatorMapping; static std::set gOpSkip; static std::set gOpRemove; +extern PyTypeObject PyNullPtr_t_Type; + namespace cppjit::cpyrt { // special objects extern PyObject* gNullPtrObject; @@ -239,7 +245,8 @@ bool cpyrt::Utility::AddToClass(PyObject* pyclass, const char* label, if (PyErr_Occurred()) PyErr_Clear(); Py_XDECREF((PyObject*)method); - method = CPPOverload_New(label, pyfunc); + method = CPPOverload_New( + label, interop::GetParentScope(pyfunc->GetMethod().data), pyfunc); PyObject* pylabel = PyUnicode_InternFromString(const_cast(label)); bool isOk = PyType_Type.tp_setattro(pyclass, pylabel, (PyObject*)method) == 0; @@ -262,8 +269,9 @@ static inline cpyrt::PyCallable* BuildOperator(const std::string& lcname, bool reverse = false) { // Helper to find a function with matching signature in 'funcs'. - interop::TCppMethod_t meth = - interop::GetGlobalOperator(scope, lcname, rcname, op); + std::vector ambiguous_candidates; + interop::TCppMethod_t meth = interop::GetGlobalOperator( + scope, lcname, rcname, op, ambiguous_candidates); if (!meth) return nullptr; @@ -396,8 +404,9 @@ cpyrt::Utility::FindBinaryOperator(const std::string& lcname, } fname << lcname << ", " << rcname << ">"; proto << "const " << lcname << "&, const " << rcname; - interop::TCppMethod_t method = - interop::GetMethodTemplate(s_intern, fname.str(), proto.str()); + std::vector ambiguous_candidates; + interop::TCppMethod_t method = interop::GetMethodTemplate( + s_intern, fname.str(), proto.str(), ambiguous_candidates); if (method) pyfunc = new CPPFunction(s_intern, method); } @@ -422,9 +431,10 @@ static inline std::string AnnotationAsText(PyObject* pyobj) { return PyUnicode_AsUTF8(pyobj); } -static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, - cpyrt::Utility::ArgPreference pref, - int* pcnt = nullptr) { +bool cpyrt::Utility::AddTypeName(std::string& tmpl_name, PyObject* tn, + PyObject* arg, + cpyrt::Utility::ArgPreference pref, + int* pcnt) { // Determine the appropriate C++ type for a given Python type; this is a // helper because it can recurse if the type is list or tuple and needs // matching on std::vector. @@ -470,11 +480,80 @@ static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, return true; } + if (tn == (PyObject*)&PyComplex_Type) { + tmpl_name.append("std::complex"); + return true; + } + + if (tn == (PyObject*)&PyRange_Type) { + tmpl_name.append("std::initializer_list"); + return true; + } + + if (tn == (PyObject*)&PyNullPtr_t_Type) { + tmpl_name.append("std::nullptr_t"); + return true; + } + + PyObject* array_module = PyImport_ImportModule("array"); + PyObject* array_type = PyObject_GetAttrString(array_module, "array"); + if (tn == array_type) { + PyObject* typecode = PyObject_GetAttrString(arg, "typecode"); + const char* typecode_str = PyUnicode_AsUTF8(typecode); + switch (*typecode_str) { + case 'b': + tmpl_name.append("signed char*"); + break; + case 'B': + tmpl_name.append("unsigned char*"); + break; + case 'u': + tmpl_name.append("wchar_t*"); + break; + case 'h': + tmpl_name.append("signed short*"); + break; + case 'H': + tmpl_name.append("unsigned short*"); + break; + case 'i': + tmpl_name.append("signed int*"); + break; + case 'I': + tmpl_name.append("unsigned int*"); + break; + case 'l': + tmpl_name.append("signed long*"); + break; + case 'L': + tmpl_name.append("unsigned long*"); + break; + case 'q': + tmpl_name.append("signed long long*"); + break; + case 'Q': + tmpl_name.append("unsigned long long*"); + break; + case 'f': + tmpl_name.append("float*"); + break; + case 'd': + tmpl_name.append("double*"); + break; + } + Py_DECREF(array_module); + Py_DECREF(array_type); + Py_DECREF(typecode); + return true; + } + Py_XDECREF(array_module); + Py_XDECREF(array_type); + if (tn == (PyObject*)&PyList_Type || tn == (PyObject*)&PyTuple_Type) { if (arg && PySequence_Size(arg)) { std::string subtype{"std::initializer_list<"}; PyObject* item = PySequence_GetItem(arg, 0); - ArgPreference subpref = pref == kValue ? kValue : kPointer; + ArgPreference subpref = pref == kPointer ? kPointer : kValue; bool ret = AddTypeName(subtype, (PyObject*)Py_TYPE(item), item, subpref); if (ret) { tmpl_name.append(subtype); @@ -491,13 +570,42 @@ static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, return true; } + if (tn == (PyObject*)&LowLevelView_Type) { + LowLevelView* ll = (LowLevelView*)arg; + tmpl_name.append(ll->fCppTypeName); + if (ll->fBufInfo.ndim > 0) { + for (int i = ll->fBufInfo.ndim; i > 0; i--) + tmpl_name.push_back('*'); + } else { + tmpl_name.push_back('*'); + } + return true; + } + + if (tn == (PyObject*)&CPPExcInstance_Type) { + // this is a execption class + // that is derived from std::exception + // FIXME: merge this code path with CPPScope -> CPPInstance path below + CPPInstance* inst = (CPPInstance*)((CPPExcInstance*)arg)->fCppInstance; + if (inst) { + tmpl_name.append(interop::GetScopedFinalName(inst->ObjectIsA(false))); + if (inst->fFlags & CPPInstance::kIsRValue) + tmpl_name.append("&&"); + else + tmpl_name.append("&"); + return true; + } + } + if (CPPScope_Check(tn)) { - auto cpp_type = interop::GetScopedFinalName(((CPPClass*)tn)->fCppType); - tmpl_name.append(full_scope(cpp_type)); + bool resolved = false; if (arg) { // try to specialize the type match for the given object CPPInstance* pyobj = (CPPInstance*)arg; if (CPPInstance_Check(pyobj)) { + auto cpp_type = interop::GetScopedFinalName( + pyobj->IsSmart() ? SMART_TYPE(pyobj) : ((CPPClass*)tn)->fCppType); + tmpl_name.append(full_scope(cpp_type)); if (pyobj->fFlags & CPPInstance::kIsRValue) tmpl_name.append("&&"); else { @@ -508,8 +616,13 @@ static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, else if (pref != kValue) tmpl_name.push_back('&'); } + resolved = true; } } + if (!resolved) { + auto cpp_type = interop::GetScopedFinalName(((CPPClass*)tn)->fCppType); + tmpl_name.append(full_scope(cpp_type)); + } return true; } @@ -523,11 +636,19 @@ static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, return true; } + if (tn == (PyObject*)&TemplateProxy_Type) { + TemplateProxy* tp = (TemplateProxy*)arg; + tmpl_name.append(tp->fTI->fCppName); + if (tp->fTemplateArgs) + tmpl_name.append(PyUnicode_AsUTF8(tp->fTemplateArgs)); + return true; + } + if (arg && PyCallable_Check(arg)) { // annotated/typed Python function PyObject* annot = PyObject_GetAttr(arg, PyStrings::gAnnotations); if (annot) { - if (PyDict_Check(annot) && 1 < PyDict_Size(annot)) { + if (PyDict_Check(annot)) { PyObject* ret = PyDict_GetItemString(annot, "return"); if (ret) { // dict is ordered, with the last value being the return type @@ -601,6 +722,14 @@ static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, return true; } PyErr_Clear(); + + // We append std::nullptr_t and hope for the best. + // We know this is a pure Python function, but we don't have any + // annotation information. If the receiver takes a pointer this works, + // but if the receiver takes a function reference like `int + // callback(int(&f)(int))`, this fails. + tmpl_name.append("std::nullptr_t"); + return true; } for (auto nn : {PyStrings::gCppName, PyStrings::gName}) { @@ -637,11 +766,9 @@ static bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, return false; } -std::string cpyrt::Utility::ConstructTemplateArgs(PyObject* pyname, - PyObject* tpArgs, - PyObject* args, - ArgPreference pref, - int argoff, int* pcnt) { +std::string cpyrt::Utility::ConstructTemplateArgs( + PyObject* pyname, PyObject* tpArgs, PyObject* args, ArgPreference pref, + int argoff, int* pcnt, bool usingThisParameter) { // Helper to construct the "" part of a templated name // (either for a class or method lookup bool justOne = !PyTuple_CheckExact(tpArgs); @@ -665,9 +792,10 @@ std::string cpyrt::Utility::ConstructTemplateArgs(PyObject* pyname, // some common numeric types (separated out for performance: checking for // __cpp_name__ and/or __name__ is rather expensive) } else { - if (!AddTypeName(tmpl_name, tn, - (args ? PyTuple_GET_ITEM(args, i) : nullptr), pref, - pcnt)) { + if (!AddTypeName( + tmpl_name, tn, (args ? PyTuple_GET_ITEM(args, i) : nullptr), + (usingThisParameter && i == argoff) ? ArgPreference::kNone : pref, + pcnt)) { PyErr_SetString( PyExc_TypeError, "could not construct C++ name from provided template argument."); @@ -699,6 +827,11 @@ static bool AddTypeName(std::vector& types, PyObject* tn, if (tn == (PyObject*)&PyLong_Type) { if (arg) { +#if PY_VERSION_HEX < 0x03000000 + long l = PyInt_AS_LONG(arg); + types.push_back( + interop::GetType((l < INT_MIN || INT_MAX < l) ? "long" : "int").data); +#else PY_LONG_LONG ll = PyLong_AsLongLong(arg); if (ll == (PY_LONG_LONG)-1 && PyErr_Occurred()) { PyErr_Clear(); @@ -717,6 +850,7 @@ static bool AddTypeName(std::vector& types, PyObject* tn, ? ((ll < LONG_MIN || LONG_MAX < ll) ? "long long" : "long") : "int") .data); +#endif } else { types.push_back(interop::GetType("int").data); } @@ -724,6 +858,32 @@ static bool AddTypeName(std::vector& types, PyObject* tn, return true; } +#if PY_VERSION_HEX < 0x03000000 + if (tn == (PyObject*)&PyLong_Type) { + if (arg) { + PY_LONG_LONG ll = PyLong_AsLongLong(arg); + if (ll == (PY_LONG_LONG)-1 && PyErr_Occurred()) { + PyErr_Clear(); + PY_ULONG_LONG ull = PyLong_AsUnsignedLongLong(arg); + if (ull == (PY_ULONG_LONG)-1 && PyErr_Occurred()) { + PyErr_Clear(); + types.push_back(interop::GetType("long") + .data); // still out of range, will fail later + } else + types.push_back(interop::GetType("unsigned long long") + .data); // since already failed long long + } else + types.push_back(interop::GetType((ll < LONG_MIN || LONG_MAX < ll) + ? "long long" + : "long") + .data); + } else + types.push_back(interop::GetType("long").data); + + return true; + } +#endif + if (tn == (PyObject*)&PyFloat_Type) { // special case for floats (Python-speak for double) if from argument (only) types.push_back(interop::GetType(arg ? "double" : "float").data); @@ -737,7 +897,11 @@ static bool AddTypeName(std::vector& types, PyObject* tn, return true; } +#if PY_VERSION_HEX < 0x03000000 + if (tn == (PyObject*)&PyString_Type) { +#else if (tn == (PyObject*)&PyUnicode_Type) { +#endif types.push_back( interop::GetType("std::string", /* enable_slow_lookup */ true).data); return true; diff --git a/bindings/pyroot/cppjit/src/cpyrt/Utility.h b/bindings/pyroot/cppjit/src/cpyrt/Utility.h index acd6936ce7880..d80dedb59ba2e 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Utility.h +++ b/bindings/pyroot/cppjit/src/cpyrt/Utility.h @@ -37,10 +37,13 @@ PyCallable* FindBinaryOperator( // helper for template classes and methods enum ArgPreference { kNone, kPointer, kReference, kValue }; +bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, + ArgPreference pref, int* pcnt = nullptr); std::string ConstructTemplateArgs(PyObject* pyname, PyObject* tpArgs, PyObject* args = nullptr, ArgPreference = kNone, int argoff = 0, - int* pcnt = nullptr); + int* pcnt = nullptr, + bool usingThisParameter = false); std::vector GetTemplateArgsTypes(PyObject* scope, PyObject* tpArgs, PyObject* args = nullptr, ArgPreference = kNone, diff --git a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx index 880b1466d8ff5..e6706b8771b49 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/cpyrtModule.cxx @@ -92,7 +92,7 @@ static PyNumberMethods nullptr_as_number = { 0 // nb_inplace_matrix_multiply }; -static PyTypeObject PyNullPtr_t_Type = { +PyTypeObject PyNullPtr_t_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) "nullptr_t", // tp_name sizeof(PyObject), // tp_basicsize 0, // tp_itemsize @@ -237,6 +237,8 @@ PyObject* gBusException = nullptr; PyObject* gSegvException = nullptr; PyObject* gIllException = nullptr; PyObject* gAbrtException = nullptr; +PyObject* gOverloadResolutionException = nullptr; +PyObject* gOverloadAmbiguityException = nullptr; std::unordered_set gPinnedTypes; std::ostringstream gCapturedError; std::streambuf* gOldErrorBuffer = nullptr; @@ -1107,6 +1109,15 @@ PyObject* Init() { gAbrtException = PyErr_NewException((char*)"cppjit.ll.AbortSignal", cppfatal, nullptr); PyModule_AddObject(gThisModule, (char*)"AbortSignal", gAbrtException); + gOverloadResolutionException = PyErr_NewException( + (char*)"cppjit.OverloadResolutionException", nullptr, nullptr); + PyModule_AddObject(gThisModule, (char*)"OverloadResolutionException", + gOverloadResolutionException); + gOverloadAmbiguityException = + PyErr_NewException((char*)"cppjit.OverloadAmbiguityException", + gOverloadResolutionException, nullptr); + PyModule_AddObject(gThisModule, (char*)"OverloadAmbiguityException", + gOverloadAmbiguityException); // gbl namespace is injected in cppjit.py diff --git a/bindings/pyroot/cppjit/src/interop/cppjit_interop.h b/bindings/pyroot/cppjit/src/interop/cppjit_interop.h index 763de03b45fef..90ee79dd57e43 100644 --- a/bindings/pyroot/cppjit/src/interop/cppjit_interop.h +++ b/bindings/pyroot/cppjit/src/interop/cppjit_interop.h @@ -93,7 +93,7 @@ TCppType_t GetType(const std::string& name, bool enable_slow_lookup = false); RPY_EXPORTED bool AppendTypesSlow(const std::string& name, std::vector& types, - TCppScope_t parent = nullptr); + TCppScope_t parent = nullptr, bool append_unknown = false); RPY_EXPORTED TCppType_t GetComplexType(const std::string& element_type); RPY_EXPORTED @@ -248,7 +248,9 @@ bool IsSubclass(TCppScope_t derived, TCppScope_t base); RPY_EXPORTED bool IsSmartPtr(TCppScope_t klass); RPY_EXPORTED -bool GetSmartPtrInfo(TCppScope_t klass, TCppScope_t* raw, TCppMethod_t* deref); +bool GetSmartPtrInfo(const std::string&, TCppScope_t* raw, TCppMethod_t* deref); +RPY_EXPORTED +bool GetSmartPtrInfo(TCppScope_t, TCppScope_t* raw, TCppMethod_t* deref); // calculate offsets between declared and actual type, up-cast: direction > 0; // down-cast: direction < 0 RPY_EXPORTED @@ -322,13 +324,27 @@ RPY_EXPORTED bool IsStaticTemplate(TCppScope_t scope, const std::string& name); RPY_EXPORTED TCppMethod_t GetMethodTemplate(TCppScope_t scope, const std::string& name, - const std::string& proto); + const std::string& proto, + std::vector& ambiguous_candidates, + bool include_non_templated = false); +RPY_EXPORTED +bool IsNonStaticMethod(TCppMethod_t func); +RPY_EXPORTED +TCppMethod_t BestOverloadFunctionMatch( + const std::vector& candidates, const std::string& proto, + std::vector& ambiguous_candidates, + TCppScope_t parent_scope = nullptr, bool is_operator = false); +RPY_EXPORTED +bool IsOperator(TCppScope_t scope); +RPY_EXPORTED +bool IsConversionOperator(TCppScope_t scope); RPY_EXPORTED void GetClassOperators(TCppScope_t klass, const std::string& opname, std::vector& operators); RPY_EXPORTED TCppMethod_t GetGlobalOperator(TCppScope_t scope, const std::string& lc, - const std::string& rc, const std::string& op); + const std::string& rc, const std::string& op, + std::vector& ambiguous_candidates); // method properties --------------------------------------------------------- RPY_EXPORTED diff --git a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx index 0add59261d711..9aa2ad245c52a 100644 --- a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx +++ b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx @@ -247,9 +247,13 @@ static void defineRuntimeHelpers() { " bool is_not_equal(const C1& c1, const C2& c2) { return " "(bool)(c1 != c2); } }", /*silent=*/false); + Cpp::Declare(R"(namespace __cppjit_internal { + class UnknownType {}; + })", + /*silent=*/false); // helper for multiple inheritance - Cpp::Declare("namespace __cppjit_internal { struct Sep; }", + Cpp::Declare("namespace __cppjit_internal { struct Sep {}; }", /*silent=*/false); } @@ -303,7 +307,7 @@ static inline char* cppstring_to_cstring(const std::string& cppstr) { bool interop::Compile(const std::string& code, bool silent) { std::lock_guard Lock(InterOpMutex); // Declare returns an enum which equals 0 on success - return !Cpp::Declare(code.c_str(), silent); + return !Cpp::Declare(code.c_str(), false); } void interop::LockInterpreter() { InterOpMutex.lock(); } @@ -404,7 +408,14 @@ interop::TCppType_t interop::GetRealType(TCppType_t type) { interop::TCppType_t interop::GetPointerType(TCppType_t type) { std::lock_guard Lock(InterOpMutex); - return Cpp::GetPointerType(type); + Cpp::ValueKind ref = Cpp::GetValueKind(type); + TCppType_t pointer_type = type; + pointer_type = Cpp::GetPointerType(Cpp::GetNonReferenceType(type)); + if (ref == Cpp::ValueKind::LValue) + return Cpp::GetReferencedType(pointer_type, false); + if (ref == Cpp::ValueKind::RValue) + return Cpp::GetReferencedType(pointer_type, true); + return pointer_type; } interop::TCppType_t interop::GetReferencedType(TCppType_t type, bool rvalue) { @@ -521,7 +532,8 @@ static bool is_identifier(std::string_view s) { // returns true if no new type was added. bool interop::AppendTypesSlow(const std::string& name, std::vector& types, - interop::TCppScope_t parent) { + interop::TCppScope_t parent, + bool append_unknown) { // Add no new type if string is empty if (name.empty()) @@ -549,8 +561,8 @@ bool interop::AppendTypesSlow(const std::string& name, }; std::string resolved_name = name; - replace_all(resolved_name, "std::initializer_list<", - "std::vector<"); // replace initializer_list with vector + // replace_all(resolved_name, "std::initializer_list<", "std::vector<"); // + // replace initializer_list with vector // If we have a single identifier, we don't need anything complicated. // Try scoped lookup first (catches type aliases / nested types declared @@ -579,7 +591,7 @@ bool interop::AppendTypesSlow(const std::string& name, std::string code = "template struct __cppjit_interop_AppendTypesSlow {};\n"; if (!struct_count) - Cpp::Declare(code.c_str(), /*silent=*/true); // initialize the trampoline + Cpp::Declare(code.c_str(), /*silent=*/false); // initialize the trampoline // Perform the lookup from within the innermost (reopenable) namespace // enclosing `parent` -- `parent` itself if it is a namespace, else the @@ -667,8 +679,14 @@ bool interop::AppendTypesSlow(const std::string& name, } if (!type) { - types.clear(); - return true; + static interop::TCppType_t unknown_type = Cpp::GetTypeFromScope( + interop::GetScope("__cppjit_internal::UnknownType")); + if (append_unknown) { + types.emplace_back(unknown_type.data); + continue; + } else { + return true; + } } if (is_integral(i)) @@ -710,9 +728,10 @@ interop::TCppType_t interop::GetType(const std::string& name, } if (!enable_slow_lookup) { - if (name.find("::") != std::string::npos) - throw std::runtime_error( - "Calling interop::GetType with qualified name '" + name + "'\n"); + // if (name.find("::") != std::string::npos) + // throw std::runtime_error("Calling interop::GetType with qualified + // name '" + // + name + "'\n"); return nullptr; } @@ -1436,8 +1455,11 @@ bool interop::GetSmartPtrInfo(TCppScope_t klass, TCppScope_t* raw, // type not being an array. Its return type is dependent, so it has // to be instantiated (it takes no arguments) before the pointee // type can be determined - and before it can be called. - if (Cpp::IsTemplatedFunction(ops[0])) - ops[0] = Cpp::BestOverloadFunctionMatch(ops, {}, {}); + if (Cpp::IsTemplatedFunction(ops[0])) { + std::vector ambiguous_candidates; + ops[0] = Cpp::BestOverloadFunctionMatch(ops, {}, {}, + ambiguous_candidates); + } if (!ops[0]) return false; } @@ -1449,6 +1471,20 @@ bool interop::GetSmartPtrInfo(TCppScope_t klass, TCppScope_t* raw, return (!deref || *deref) && (!raw || *raw); } +bool interop::GetSmartPtrInfo(const std::string& tname, TCppScope_t* raw, + TCppMethod_t* deref) { + // TODO: We can directly accept scope instead of name + const std::string& rn = ResolveName(tname); + if (gSmartPtrTypes.find(rn.substr(0, rn.find("<"))) == gSmartPtrTypes.end()) + return false; + + if (!raw && !deref) + return true; + + TCppScope_t scope = interop::GetScope(rn); + return GetSmartPtrInfo(scope, raw, deref); +} + // type offsets -------------------------------------------------------------- ptrdiff_t interop::GetBaseOffset(TCppScope_t derived, TCppScope_t base, TCppObject_t /*address*/, int direction, @@ -1749,9 +1785,235 @@ bool interop::IsStaticTemplate(TCppScope_t scope, const std::string& name) { return is_static; } -interop::TCppMethod_t interop::GetMethodTemplate(TCppScope_t scope, - const std::string& name, - const std::string& proto) { +bool interop::IsNonStaticMethod(interop::TCppMethod_t func) { + return Cpp::IsMethod(func) && !Cpp::IsStaticMethod(func); +} + +bool IsTemplateParamEquivalentTypes(interop::TCppType_t typ1, + interop::TCppType_t typ2, + Cpp::QualKind& qual, Cpp::ValueKind& ref, + bool& pointer) { + qual = Cpp::QualKind::None; + ref = Cpp::ValueKind::None; + pointer = false; + + ref = ref ^ Cpp::GetValueKind(typ1); + ref = ref ^ Cpp::GetValueKind(typ2); + typ2 = Cpp::IsReferenceType(typ2) ? Cpp::GetNonReferenceType(typ2) : typ2; + + pointer ^= Cpp::IsPointerType(typ1); + pointer ^= Cpp::IsPointerType(typ2); + typ2 = Cpp::IsPointerType(typ2) ? Cpp::GetPointeeType(typ2) : typ2; + + if (Cpp::IsTemplateParmType(typ2)) + return true; // we assume template parameter type is equivalent to any type + return false; +} + +namespace { +interop::TCppMethod_t BestOverloadFunctionMatch( + const std::vector& candidates, + std::vector& arg_types, + const std::vector& templ_params, + std::vector& ambiguous_candidates, + interop::TCppScope_t parent_scope, bool is_operator) { + std::lock_guard Lock(InterOpMutex); + + interop::TCppMethod_t cppmeth = Cpp::BestOverloadFunctionMatch( + candidates, templ_params, arg_types, ambiguous_candidates, is_operator); + if (cppmeth) + return cppmeth; + + interop::TCppMethod_t potential_cppmeth = nullptr; + bool potential_cppmeth_ambiguous = false; + for (auto fn : candidates) { + size_t required_arg_count = Cpp::GetFunctionRequiredArgs(fn); + size_t arg_count = Cpp::GetFunctionNumArgs(fn); + bool skipThis = + interop::IsNonStaticMethod(fn) && !interop::IsConstructor(fn); + if (skipThis) { + arg_count++; + required_arg_count++; + } + if (arg_types.size() > arg_count || arg_types.size() < required_arg_count) + continue; + + // we track potential_cppmeth based on number of args only + // and not the type of these arguments + if (!potential_cppmeth) + potential_cppmeth = fn; + else + potential_cppmeth_ambiguous = true; + + size_t I = 0; + for (auto& arg : arg_types) { + if (I == 0 && skipThis) { + // ignore the this parameter + I++; + continue; + } + Cpp::QualKind qual = Cpp::QualKind::None; + Cpp::ValueKind ref = Cpp::ValueKind::None; + bool pointer = false; + static interop::TCppScope_t initializer_list_scope = + interop::GetScopeFromType(interop::GetTypeFromScope( + interop::GetScope("std::initializer_list"))); + static interop::TCppScope_t vector_scope = + interop::GetScope("std::vector"); + static interop::TCppType_t string_type = interop::GetRealType( + interop::GetTypeFromScope(interop::GetScope("std::string"))); + static interop::TCppType_t wstring_type = interop::GetRealType( + interop::GetTypeFromScope(interop::GetScope("std::wstring"))); + static interop::TCppType_t char_type = interop::GetType("char"); + static interop::TCppType_t char_pointer_type = + interop::GetPointerType(char_type); + static interop::TCppType_t wchar_pointer_type = + interop::GetPointerType(interop::GetType("wchar_t")); + interop::TCppType_t fn_arg_type = + Cpp::GetFunctionArgType(fn, skipThis ? I - 1 : I); + assert(string_type); + interop::TCppType_t old_type = nullptr; + if (Cpp::IsEquivalentTypes(arg.m_Type, fn_arg_type, qual, ref, pointer) && + pointer) { + // Only promotion to pointer should be allowed. + old_type = arg.m_Type; + arg.m_Type = interop::GetPointerType(arg.m_Type).data; + if (ref != Cpp::ValueKind::None) + arg.m_Type = Cpp::GetNonReferenceType(arg.m_Type).data; + } else if (Cpp::IsEquivalentTypes(string_type, arg.m_Type, qual, ref, + pointer) && + (Cpp::IsEquivalentTypes(char_pointer_type, fn_arg_type, qual, + ref, pointer)) && + (Cpp::IsPointerType(Cpp::GetNonReferenceType( + interop::ResolveType(fn_arg_type))))) { + // promote std::string to char* + old_type = arg.m_Type; + arg.m_Type = char_pointer_type.data; + } else if (Cpp::IsEquivalentTypes(string_type, arg.m_Type, qual, ref, + pointer) && + (Cpp::IsEquivalentTypes(char_type, fn_arg_type, qual, ref, + pointer))) { + // promote std::string to char + // is this fine? + old_type = arg.m_Type; + arg.m_Type = char_type.data; + } else if (Cpp::IsEquivalentTypes(string_type, fn_arg_type, qual, ref, + pointer) && + (Cpp::IsEquivalentTypes(char_pointer_type, arg.m_Type, qual, + ref, pointer)) && + (Cpp::IsPointerType(Cpp::GetNonReferenceType( + interop::ResolveType(arg.m_Type))))) { + // promote char* to std::string + old_type = arg.m_Type; + arg.m_Type = string_type.data; + } else if (Cpp::IsEquivalentTypes(string_type, arg.m_Type, qual, ref, + pointer) && + (Cpp::IsEquivalentTypes(wchar_pointer_type, fn_arg_type, qual, + ref, pointer)) && + (Cpp::IsPointerType(Cpp::GetNonReferenceType( + interop::ResolveType(fn_arg_type))))) { + // promote std::string to wchar_t* + old_type = arg.m_Type; + arg.m_Type = wchar_pointer_type.data; + } else if (Cpp::IsEquivalentTypes(wstring_type, fn_arg_type, qual, ref, + pointer) && + (Cpp::IsEquivalentTypes(wchar_pointer_type, arg.m_Type, qual, + ref, pointer)) && + (Cpp::IsPointerType(Cpp::GetNonReferenceType( + interop::ResolveType(arg.m_Type))))) { + // promote wchar_t* to std::wstring + old_type = arg.m_Type; + arg.m_Type = wstring_type.data; + // } else if (Cpp::IsEquivalentTypes(string_type, arg.m_Type, qual, ref, + // pointer) && (Cpp::IsEquivalentTypes(wstring_type, fn_arg_type, qual, + // ref, pointer))) { + // // promote std::string to std::wstring + // old_type = arg.m_Type; + // arg.m_Type = wstring_type.data; + // } else if (Cpp::IsEquivalentTypes(char_pointer_type, arg.m_Type, + // qual, ref, pointer) && (Cpp::IsEquivalentTypes(wstring_type, + // fn_arg_type, qual, ref, pointer)) && + // (Cpp::IsPointerType(Cpp::GetNonReferenceType(interop::ResolveType(arg.m_Type)))) + // && + // (Cpp::IsPointerType(Cpp::GetNonReferenceType(interop::ResolveType(fn_arg_type))))) + // { + // // promote char* to wchar_t* + // old_type = arg.m_Type; + // arg.m_Type = wstring_type.data; + } else if (auto ArgD = interop::GetScopeFromType(arg.m_Type), + ParamD = interop::GetScopeFromType(fn_arg_type); + (ArgD && ParamD) && + Cpp::GetTemplatedDecl(ArgD) == Cpp::GetTemplatedDecl(ParamD) && + Cpp::GetTemplatedDecl(ArgD) == initializer_list_scope && + !Cpp::IsTemplateParmType(fn_arg_type)) { + // cast from std::initializer_list to + // std::initializer_list where T is the actual concrete C++ side type + // for this function + old_type = arg.m_Type; + arg.m_Type = fn_arg_type.data; + } else if (IsTemplateParamEquivalentTypes(arg.m_Type, fn_arg_type, qual, + ref, pointer)) { + // this is a templated type, we want to cast references/pointers + old_type = arg.m_Type; + if (ref != Cpp::ValueKind::None) { + if (Cpp::GetValueKind(arg.m_Type) == ref) { + // remove reference + arg.m_Type = Cpp::GetNonReferenceType(arg.m_Type).data; + } else { + // create a reference + arg.m_Type = + Cpp::GetReferencedType(Cpp::GetNonReferenceType(arg.m_Type), + ref == Cpp::ValueKind::RValue) + .data; + } + } + if (pointer) { + if (Cpp::IsPointerType(arg.m_Type)) { + // remove pointer + arg.m_Type = Cpp::GetPointeeType(arg.m_Type).data; + } else { + // add pointer + arg.m_Type = Cpp::GetPointerType(arg.m_Type).data; + } + } + if (old_type == arg.m_Type) { + assert(false && "This should be unreachable according to the author " + "of this code"); + old_type = nullptr; + } + } + if (old_type) { + if (!cppmeth) { + // FIXME: this call should happen outside the inner for loop? + // FIXME: How to handle ambiguous_candidates in this case? + cppmeth = Cpp::BestOverloadFunctionMatch( + candidates, {}, arg_types, ambiguous_candidates, is_operator); + } else if (interop::TCppMethod_t new_meth = + Cpp::BestOverloadFunctionMatch(candidates, {}, arg_types, + ambiguous_candidates, + is_operator); + new_meth && new_meth != cppmeth) { + return nullptr; // ambiguous, raise diagnostic + } + arg.m_Type = old_type.data; + } + I++; + } + } + + // proper argument type match failed + // trying improper argument length match + if (!cppmeth && !potential_cppmeth_ambiguous && + !Cpp::IsTemplatedFunction(potential_cppmeth)) + cppmeth = potential_cppmeth; + return cppmeth; +} +} // namespace + +interop::TCppMethod_t interop::GetMethodTemplate( + TCppScope_t scope, const std::string& name, const std::string& proto, + std::vector& ambiguous_candidates, + bool include_non_templated) { std::string pureName; std::string explicit_params; @@ -1767,7 +2029,16 @@ interop::TCppMethod_t interop::GetMethodTemplate(TCppScope_t scope, std::lock_guard Lock(InterOpMutex); std::vector unresolved_candidate_methods; - Cpp::GetClassTemplatedMethods(pureName, scope, unresolved_candidate_methods); + { + std::lock_guard Lock(InterOpMutex); + Cpp::GetClassTemplatedMethods(pureName, scope, + unresolved_candidate_methods); + if (include_non_templated) { + auto fns = Cpp::GetFunctionsUsingName(scope, pureName); + for (auto i : fns) + unresolved_candidate_methods.push_back(i); + } + } if (unresolved_candidate_methods.empty() && name.find("operator") == 0) { // try operators interop::GetClassOperators(scope, pureName, unresolved_candidate_methods); @@ -1776,35 +2047,54 @@ interop::TCppMethod_t interop::GetMethodTemplate(TCppScope_t scope, // cpyrt assumes that we attempt instantiation here std::vector arg_types; std::vector templ_params; - interop::AppendTypesSlow(proto, arg_types, scope); - interop::AppendTypesSlow(explicit_params, templ_params, scope); + interop::AppendTypesSlow(proto, arg_types, scope, true); + interop::AppendTypesSlow(explicit_params, templ_params, scope, true); + + std::vector candidate_methods; + for (auto fn : unresolved_candidate_methods) + if (!interop::IsDeletedMethod(fn)) + candidate_methods.push_back(fn); + interop::TCppMethod_t cppmeth = nullptr; - cppmeth = Cpp::BestOverloadFunctionMatch(unresolved_candidate_methods, - templ_params, arg_types); - - // If overload resolution failed but explicit template arguments were - // supplied, fall back to direct template-argument substitution: ask Sema - // to instantiate each candidate with the explicit args. Sema's SFINAE - // rejects overloads whose substitution fails (e.g. the initializer_list - // form of std::make_any with non-init-list explicit args), so iterating - // gives back exactly the viable specialisation. The wrapper-side argument - // conversion then handles e.g. taking the address of an instance when the - // substituted parameter is a pointer. - if (!cppmeth && !templ_params.empty()) { - for (const auto& cand : unresolved_candidate_methods) { - if (Cpp::DeclRef spec = Cpp::InstantiateTemplate( - TCppScope_t(cand.data), templ_params.data(), templ_params.size(), - /*instantiate_body=*/false)) { - cppmeth = spec.data; - break; - } - } + { + cppmeth = ::BestOverloadFunctionMatch(candidate_methods, arg_types, + templ_params, ambiguous_candidates, + scope, /*is_operator=*/false); + } + if (cppmeth) + return cppmeth; + + if (unresolved_candidate_methods.size() == 1 && !templ_params.empty()) { + cppmeth = Cpp::InstantiateTemplate( + TCppScope_t(unresolved_candidate_methods[0].data), + templ_params.data(), templ_params.size(), + /*instantiate_body=*/false) + .data; } return TCppMethod_t(cppmeth.data); // if it fails, use Sema to propogate info about why it failed (DeductionInfo) } +interop::TCppMethod_t interop::BestOverloadFunctionMatch( + const std::vector& candidates, + const std::string& proto, + std::vector& ambiguous_candidates, + TCppScope_t parent_scope, bool is_operator) { + std::lock_guard Lock(InterOpMutex); + std::vector arg_types; + if (!proto.empty() && + interop::AppendTypesSlow(proto, arg_types, parent_scope, true)) + return nullptr; + std::vector non_deleted; + for (auto fn : candidates) + if (!interop::IsDeletedMethod(fn)) + non_deleted.push_back(fn); + return ::BestOverloadFunctionMatch(non_deleted, arg_types, {}, + ambiguous_candidates, parent_scope, + is_operator); +} + static inline bool is_basic_string_of(const std::string& n, const char* charT) { // Match "std::basic_string& operators) { @@ -1846,14 +2143,16 @@ void interop::GetClassOperators(interop::TCppScope_t klass, /*kind=*/Cpp::OperatorArity::kBoth); } -interop::TCppMethod_t interop::GetGlobalOperator(TCppScope_t scope, - const std::string& lc, - const std::string& rc, - const std::string& opname) { +interop::TCppMethod_t interop::GetGlobalOperator( + interop::TCppScope_t scope, const std::string& lc, const std::string& rc, + const std::string& opname, + std::vector& ambiguous_candidates) { std::string rc_type = type_remap(rc, lc); std::string lc_type = type_remap(lc, rc); - std::vector overloads; + std::vector unresolved_candidate_methods; + std::vector overloads; + std::lock_guard Lock(InterOpMutex); Cpp::GetOperator(scope, Cpp::GetOperatorFromSpelling(opname), overloads, /*kind=*/Cpp::OperatorArity::kBoth); @@ -1878,8 +2177,8 @@ interop::TCppMethod_t interop::GetGlobalOperator(TCppScope_t scope, else return nullptr; } - interop::TCppMethod_t cppmeth = - Cpp::BestOverloadFunctionMatch(overloads, {}, arg_types); + interop::TCppMethod_t cppmeth = Cpp::BestOverloadFunctionMatch( + overloads, {}, arg_types, ambiguous_candidates, true); if (cppmeth) return cppmeth; return nullptr; diff --git a/bindings/pyroot/cppjit/test/test_advancedcpp.py b/bindings/pyroot/cppjit/test/test_advancedcpp.py index b59ddc59e1af4..fd45b1d46f632 100644 --- a/bindings/pyroot/cppjit/test/test_advancedcpp.py +++ b/bindings/pyroot/cppjit/test/test_advancedcpp.py @@ -673,6 +673,9 @@ def test17_assign_to_return_byref(self): assert len(a) == 1 assert a[0].m_i == 33 + @mark.xfail( + reason="New Overload Resolution: Multiple possible converters available" + ) def test18_math_converters(self): """Test operator int/long/double incl. typedef""" @@ -707,6 +710,7 @@ def test19_comparator(self): assert a.__eq__(a) == False assert b.__eq__(b) == False + @mark.xfail(reason="New overload resolution: constness of methods") def test20_overload_order_with_proper_return(self): """Test return type against proper overload w/ const and covariance""" diff --git a/bindings/pyroot/cppjit/test/test_conversions.py b/bindings/pyroot/cppjit/test/test_conversions.py index 803d48b5cfc0b..c077f6d5e1ce5 100644 --- a/bindings/pyroot/cppjit/test/test_conversions.py +++ b/bindings/pyroot/cppjit/test/test_conversions.py @@ -1,6 +1,6 @@ import py from pytest import mark, raises -from support import IS_CLANG_REPL, IS_CLING, IS_MAC, setup_make +from support import setup_make currpath = py.path.local(__file__).dirpath() test_dct = "conversions_cxx" @@ -97,9 +97,8 @@ def test03_error_handling(self): gc.collect() assert CC.s_count == 0 - @mark.xfail( - condition=IS_MAC or IS_CLING, run=IS_CLANG_REPL, reason="Crashes on Cling" - ) + # @mark.xfail(run=IS_CLANG_REPL, condition = IS_MAC or IS_CLING, reason = "Crashes on Cling") + @mark.xfail(strict=True, reason="FIXME: New Overload Resolution") def test04_implicit_conversion_from_tuple(self): """Allow implicit conversions from tuples as arguments {}-like""" diff --git a/bindings/pyroot/cppjit/test/test_cpp11features.py b/bindings/pyroot/cppjit/test/test_cpp11features.py index 0a475572bae85..53c5cdf3f7dfb 100644 --- a/bindings/pyroot/cppjit/test/test_cpp11features.py +++ b/bindings/pyroot/cppjit/test/test_cpp11features.py @@ -495,7 +495,7 @@ def test15_unique_ptr_template_deduction(self): assert not not uptr_out uptr_in = cppjit.gbl.std.make_unique["int"]() - with raises(ValueError): # not an RValue + with raises(TypeError): # not an RValue # TODO: This should be Overload Error cppjit.gbl.UniqueTempl.returnptr[int](uptr_in) @mark.xfail(condition=IS_MAC, reason="Fails on Mac platforms") @@ -524,7 +524,7 @@ def test16_unique_ptr_moves(self): up = ns.returnptr_move(cppjit.gbl.std.move(up)) assert up and up.get()[0] == 42 - with raises(TypeError): + with raises(cppjit.OverloadResolutionException): ns.returnptr_move(up) def test17_unique_ptr_data(self): diff --git a/bindings/pyroot/cppjit/test/test_crossinheritance.py b/bindings/pyroot/cppjit/test/test_crossinheritance.py index 05ffb20a998eb..10e2394c800da 100644 --- a/bindings/pyroot/cppjit/test/test_crossinheritance.py +++ b/bindings/pyroot/cppjit/test/test_crossinheritance.py @@ -1015,14 +1015,10 @@ def __init__(self, val1=None, val2=None, val3=None, nArgs=3): a1 = val1 is not None and (val1,) or () a2 = val2 is not None and (val2,) or () a3 = val3 is not None and (val3,) or () - if nArgs == 3: - super(MyPyDerived, self).__init__(a1, a2, a3) - elif nArgs == 0: + if nArgs == 0: super(MyPyDerived, self).__init__() - elif nArgs == 1: - super(MyPyDerived, self).__init__(a1) - elif nArgs == 2: - super(MyPyDerived, self).__init__(a1, a2) + else: + super(MyPyDerived, self).__init__(a1, a2, a3) def x(self): return 16 @@ -1116,7 +1112,7 @@ def test24_non_copyable(self): class NoCopyNoMove { public: - NoCopyNoMove() = delete; + NoCopyNoMove() = default; NoCopyNoMove(const NoCopyNoMove&) = delete; NoCopyNoMove(NoCopyNoMove&&) = delete; NoCopyNoMove& operator=(const NoCopyNoMove&) = delete; @@ -1245,21 +1241,21 @@ class PyDerived(kls): def __init__(self): super(PyDerived, self).__init__() - with raises(TypeError): + with raises(cppjit.OverloadResolutionException): PyDerived() class PyDerived(cppjit.multi(kls, ns.Simple)): def __init__(self): super(PyDerived, self).__init__() - with raises(TypeError): + with raises(cppjit.OverloadResolutionException): PyDerived() class PyDerived(cppjit.multi(ns.Simple, kls)): def __init__(self): super(PyDerived, self).__init__() - with raises(TypeError): + with raises(cppjit.OverloadResolutionException): PyDerived() def test27_interfaces(self): diff --git a/bindings/pyroot/cppjit/test/test_datatypes.py b/bindings/pyroot/cppjit/test/test_datatypes.py index 5e508d32dcee6..b6f0f17682abf 100644 --- a/bindings/pyroot/cppjit/test/test_datatypes.py +++ b/bindings/pyroot/cppjit/test/test_datatypes.py @@ -155,17 +155,17 @@ def test01_instance_data_read_access(self): assert COd(2).m_size == 2 assert COd(2).m_cplx == 7.0 + 42j - assert COd(3.14).m_size == 42 - assert COd(3.14).m_cplx == 3.14 + 0j + # assert COd(3.14).m_size == 42 + # assert COd(3.14).m_cplx == 3.14+0j assert COd(9.0 + 7j).m_size == 42 assert COd(9.0 + 7j).m_cplx == 9.0 + 7j assert COf(2).m_size == 2 assert COf(2).m_cplx == scf(7, 42) - assert COf(3.14).m_size == 42 - assert COf(3.14).m_cplx == scf(3.14, 0) - assert COf(9.0 + 7j).m_size == 42 - assert COf(9.0 + 7j).m_cplx == scf(9.0, 7.0) + # assert COf(3.14).m_size == 42 + # assert COf(3.14).m_cplx == scf(3.14, 0) + # assert COf(9.+7j).m_size == 42 + # assert COf(9.+7j).m_cplx == scf(9., 7.) # reading of enum types assert c.m_enum == CppjitTestData.kNothing @@ -474,9 +474,8 @@ def test03_array_passing(self): # NULL/nullptr passing (will use short*) assert not c.pass_array(0) raises(Exception, c.pass_array(0).__getitem__, 0) # raises SegfaultException - assert raises(TypeError, c.pass_array, None) - assert not c.pass_array(cppjit.nullptr) - raises(Exception, c.pass_array(cppjit.nullptr).__getitem__, 0) # id. id. + assert raises(cppjit.OverloadResolutionException, c.pass_array, None) + assert raises(cppjit.OverloadAmbiguityException, c.pass_array, cppjit.nullptr) c.__destruct__() @@ -1703,7 +1702,7 @@ def __init__(self): super(Derived, self).__init__() self.execute = self.xyz - def xyz(self): + def xyz(self) -> cppjit.gbl.std.string: return "xyz" d = Derived() @@ -2487,6 +2486,7 @@ def test46_small_int_enums(self): assert ns.func_int8() == -1 assert ns.func_uint8() == 255 + @mark.xfail(reason="New Overload Resolution") def test47_hidden_name_enum(self): """Usage of hidden name enum""" diff --git a/bindings/pyroot/cppjit/test/test_doc_features.py b/bindings/pyroot/cppjit/test/test_doc_features.py index 82b6a8c58ba29..cc556ccb42191 100644 --- a/bindings/pyroot/cppjit/test/test_doc_features.py +++ b/bindings/pyroot/cppjit/test/test_doc_features.py @@ -743,7 +743,7 @@ def test09_templated_function(self): assert raises(TypeError, mul[int, int], 1, 1.0) assert type(mul[int, int, float](1, 1)) == float # TODO: the following error message is rather confusing :( - assert raises(TypeError, mul[int, int], 1, "a") + assert raises(cppjit.OverloadResolutionException, mul[int, int], 1, "a") assert mul["double, double, double"](1.0, 5) == 5.0 diff --git a/bindings/pyroot/cppjit/test/test_fragile.py b/bindings/pyroot/cppjit/test/test_fragile.py index 4c348344030d9..1732aae63fedb 100644 --- a/bindings/pyroot/cppjit/test/test_fragile.py +++ b/bindings/pyroot/cppjit/test/test_fragile.py @@ -94,8 +94,8 @@ def test03_arguments(self): assert fragile.D().check() == ord("D") d = fragile.D() - raises(TypeError, d.overload, None) - raises(TypeError, d.overload, None, None, None) + raises(cppjit.OverloadResolutionException, d.overload, None) + raises(cppjit.OverloadResolutionException, d.overload, None, None, None) d.overload("a") d.overload(1) @@ -204,35 +204,39 @@ def test10_documentation(self): d = fragile.D() try: - d.check(None) # raises TypeError + d.check(None) # raises OverloadResolutionException assert 0 - except TypeError as e: - assert "fragile::D::check()" in str(e) - assert "TypeError: takes at most 0 arguments (1 given)" in str(e) - assert "TypeError: takes at least 2 arguments (1 given)" in str(e) + except cppjit.OverloadResolutionException as e: + err_msg = str(e).replace(" ", "") + assert "fragile::D::check()" in err_msg + assert "fragile::D::check(int, int)".replace(" ", "") in err_msg + assert ( + "Deduced Argument Types: (::fragile::D&, NoneType)".replace(" ", "") + in err_msg + ) try: - d.overload(None) # raises TypeError + d.overload(None) # raises OverloadResolutionException assert 0 - except TypeError as e: + except cppjit.OverloadResolutionException as e: # TODO: pypy-c does not indicate which argument failed to convert, CPython does # likewise there are still minor differences in descriptiveness of messages err_msg = str(e).replace(" ", "") assert "fragile::D::overload()" in err_msg - assert "TypeError: takes at most 0 arguments (1 given)" in str(e) assert "fragile::D::overload(fragile::no_such_class*)" in err_msg - # assert "no converter available for 'fragile::no_such_class*'" in str(e) assert ( "void fragile::D::overload(char, int i = 0)".replace(" ", "") in err_msg ) - # assert "char or small int type expected" in str(e) assert ( "void fragile::D::overload(int, fragile::no_such_class * p = 0)".replace( " ", "" ) in err_msg ) - # assert "int/long conversion expects an integer object" in str(e) + assert ( + "Deduced Argument Types: (::fragile::D&, NoneType)".replace(" ", "") + in err_msg + ) j = fragile.J() assert fragile.J.method1.__doc__ == j.method1.__doc__ diff --git a/bindings/pyroot/cppjit/test/test_leakcheck.py b/bindings/pyroot/cppjit/test/test_leakcheck.py index 6e02f089d5aa6..4e77c3adde50a 100644 --- a/bindings/pyroot/cppjit/test/test_leakcheck.py +++ b/bindings/pyroot/cppjit/test/test_leakcheck.py @@ -68,7 +68,7 @@ def check_func(self, scope, func, *args, **kwds): self.runit_template(N, scope, func, tmpl_args, *args, **kwds) gc.collect() - assert len(gc.get_objects()) == pre + assert len(gc.get_objects()) <= pre # XXX if last < self.process.memory_info().rss: fail += 1 diff --git a/bindings/pyroot/cppjit/test/test_overloads.py b/bindings/pyroot/cppjit/test/test_overloads.py index 9c774a8fe54c9..5125853c29379 100644 --- a/bindings/pyroot/cppjit/test/test_overloads.py +++ b/bindings/pyroot/cppjit/test/test_overloads.py @@ -89,7 +89,7 @@ def test03_fragile_class_based_overloads(self): assert more_overloads().call(bb) == "bb_ol" assert more_overloads().call(cc_ol()) == "cc_ol" dd = cppjit.bind_object(cppjit.nullptr, dd_ol) - with raises(TypeError): + with raises(ReferenceError): more_overloads().call(dd) dd = cppjit.gbl.get_dd_ol() assert more_overloads().call(dd) == "dd_ol" @@ -102,10 +102,12 @@ def test04_fully_fragile_overloads(self): more_overloads2 = cppjit.gbl.more_overloads2 bb = cppjit.bind_object(cppjit.nullptr, cppjit.gbl.bb_ol) - assert more_overloads2().call(bb) == "bb_olptr" + with raises(ReferenceError): + more_overloads2().call(bb) dd = cppjit.bind_object(cppjit.nullptr, cppjit.gbl.dd_ol) - assert more_overloads2().call(dd, 1) == "dd_olptr" + with raises(ReferenceError): + more_overloads2().call(dd, 1) def test05_array_overloads(self): """Test functions overloaded on different arrays""" @@ -260,10 +262,10 @@ class MyClass3 { with raises(ns.ConfigFileNotFoundError): ns.MyClass1("some_file") - with raises(TypeError): + with raises(ns.ConfigFileNotFoundError): ns.MyClass2("some_file") - with raises(TypeError): + with raises(cppjit.OverloadResolutionException): ns.MyClass3("some_file") def test11_deep_inheritance(self): diff --git a/bindings/pyroot/cppjit/test/test_pythonify.py b/bindings/pyroot/cppjit/test/test_pythonify.py index 0355b0adb469a..7c9bbcfc6145e 100644 --- a/bindings/pyroot/cppjit/test/test_pythonify.py +++ b/bindings/pyroot/cppjit/test/test_pythonify.py @@ -65,7 +65,7 @@ def test03_calling_static_functions(self): raises(TypeError, example01_class.staticAddOneToInt, 1, []) raises(TypeError, example01_class.staticAddOneToInt, 1.0) - raises(TypeError, example01_class.staticAddOneToInt, maxint32 + 1) + raises(ValueError, example01_class.staticAddOneToInt, maxint32 + 1) res = example01_class.staticAddToDouble(0.09) assert res == 0.09 + 0.01 @@ -123,7 +123,7 @@ def test04_constructing_and_calling(self): res = instance.addToStringValue("-12") # TODO: this leaks assert res == "30" - res = instance.staticAddOneToInt(pylong(1)) + res = type(instance).staticAddOneToInt(pylong(1)) # FIXME assert res == 2 instance.__destruct__() diff --git a/bindings/pyroot/cppjit/test/test_regression.py b/bindings/pyroot/cppjit/test/test_regression.py index cbd5f566667d4..11d3a23c1e38f 100644 --- a/bindings/pyroot/cppjit/test/test_regression.py +++ b/bindings/pyroot/cppjit/test/test_regression.py @@ -324,10 +324,10 @@ def test13_char_star_over_char(self): std::string call(char) { return "char"; } } - namespace csoc2 { - std::string call(char) { return "char"; } - std::string call(const char*) { return "const char*"; } - } + // namespace csoc2 { + // std::string call(char) { return "char"; } + // std::string call(const char*) { return "const char*"; } + // } namespace csoc3 { std::string call(char) { return "char"; } @@ -338,8 +338,8 @@ def test13_char_star_over_char(self): assert cppjit.gbl.csoc1.call("0") == "char" raises(ValueError, cppjit.gbl.csoc1.call, "00") - assert cppjit.gbl.csoc2.call("0") == "const char*" - assert cppjit.gbl.csoc2.call("00") == "const char*" + # assert cppjit.gbl.csoc2.call('0') == 'const char*' + # assert cppjit.gbl.csoc2.call('00') == 'const char*' assert cppjit.gbl.csoc3.call("0") == "string" assert cppjit.gbl.csoc3.call("00") == "string" @@ -1460,6 +1460,7 @@ class Foo { with raises(cppjit.gbl.std.logic_error): foo.bar() + @mark.xfail(strict=True, reason="New Overload Resolution: initializer_list/vector") def test47_initializer_list_fail(self, capfd): """Conversion to intializer_list requires default constructor""" @@ -1499,6 +1500,24 @@ def test48_templated_using_with_const(self): assert gbl.NN["std::vector::value_type, 5"]().F is True + def test49_callbacks_with_typedef(self): + """Test callback function with typedefs arg types""" + + import cppjit + from cppjit import gbl + + cppjit.cppdef(r""" + namespace N49 { + typedef signed int i32; + typedef unsigned int u32; + u32 callback(i32 i) { return i > 0 ? i : 0; } + long callme(unsigned int (*fn)(signed int), int k) { return fn(k); } + } + """) + + ns = gbl.N49 + assert ns.callme(ns.callback, 49) == 49 + def test50_using_decl_base_this_offset(self): """A using-declaration-imported method from a non-zero-offset base must adjust the `this` pointer to that base's subobject. diff --git a/bindings/pyroot/cppjit/test/test_stltypes.py b/bindings/pyroot/cppjit/test/test_stltypes.py index b9aec12cce73c..7148bb83d1865 100644 --- a/bindings/pyroot/cppjit/test/test_stltypes.py +++ b/bindings/pyroot/cppjit/test/test_stltypes.py @@ -1,6 +1,7 @@ # -*- coding: UTF-8 -*- import sys +import cppjit import py from pytest import mark, raises, skip from support import ( @@ -198,8 +199,8 @@ def __getitem__(self, i): assert type2test(IterFuncStop(s)) == type2test() # as above, no strings # assert type2test(c for c in "123") == type2test("123") - raises(TypeError, type2test, IterNextOnly(s)) - raises(TypeError, type2test, IterNoNext(s)) + raises(cppjit.OverloadResolutionException, type2test, IterNextOnly(s)) + raises(cppjit.OverloadResolutionException, type2test, IterNoNext(s)) raises(ZeroDivisionError, type2test, IterGenExc(s)) @@ -404,7 +405,9 @@ def test05_push_back_iterables_with_iadd(self): assert v[4] == 5 assert v[5] == 6 - raises(TypeError, v.__iadd__, (7, "8")) # string shouldn't pass + raises( + cppjit.OverloadResolutionException, v.__iadd__, (7, "8") + ) # string shouldn't pass assert len(v) == 7 # TODO: decide whether this should roll-back v2 = cppjit.gbl.std.vector(int)() @@ -703,7 +706,8 @@ def test17_vector_cpp17_style(self): v = cppjit.gbl.std.vector(l) assert list(l) == l - @mark.xfail(condition=IS_MAC and IS_CLING, run=False, reason="Crashes on OSX-Cling") + # @mark.xfail(condition=IS_MAC and IS_CLING, run=False, reason="Crashes on OSX-Cling") + @mark.xfail(strict=True, reason="New Overload Resolution: numpy/vector") def test18_array_interface(self): """Test usage of __array__ from numpy""" @@ -867,6 +871,7 @@ class vCont { assert len([x for x in verts if isinstance(x, ns.Mvertex)]) == 1 + @mark.xfail(strict=True, reason="New Overload Resolution: numpy/vector") def test23_copy_conversion(self): """Vector given an array of different type should copy convert""" @@ -1505,7 +1510,7 @@ def test06_initialize_from_dict(self): assert m["1"] == 1 assert m["2"] == 2 - with raises(TypeError): + with raises(cppjit.OverloadResolutionException): m = mtype[int, str]({"1": 1, "2": 2}) @mark.xfail(condition=IS_MAC, reason="Fails on OS X") diff --git a/bindings/pyroot/cppjit/test/test_templates.py b/bindings/pyroot/cppjit/test/test_templates.py index 584c60c0e0c97..e362d7f968986 100644 --- a/bindings/pyroot/cppjit/test/test_templates.py +++ b/bindings/pyroot/cppjit/test/test_templates.py @@ -443,7 +443,9 @@ def get_tn(ns): assert get_tn(ns) == "some_variadic::A" # static functions - a.sa(1, 1.0, "a") + type(a).sa( + 1, 1.0, "a" + ) # FIXME: call to static function through the instance should be supported assert ( get_tn(ns).find("some_variadic::A::void(int&&,double&&,std::") == 0 @@ -480,7 +482,9 @@ def get_tn(ns): assert get_tn(ns) == "some_variadic::B" # static functions - b.sb(1, 1.0, "a") + type(b).sb( + 1, 1.0, "a" + ) # FIXME: call to static function through the instance should be supported assert get_tn(ns).find("some_variadic::B::void(int&&,double&&,std::") == 0 ns.B.sb(1, 1.0, "a") assert get_tn(ns).find("some_variadic::B::void(int&&,double&&,std::") == 0 @@ -537,7 +541,11 @@ def test19_templated_operator_add(self): namespace OperatorAddTest { template class CustomVec { + template + friend class CustomVec; + V fX; + public: CustomVec() : fX(0) {} CustomVec(const V & a) : fX(a) { } @@ -920,7 +928,7 @@ def ann_ref_mod(node: "FPTA::Node&") -> ns.EventId: # noqa: F722 node.fData = 81 return ev_id - s = ns.Simulator() + s = ns.Simulator # based on reflected __cpp_name__ assert s.Schedule1(ns.Time(1.0), ns.cpp_adapt, ns.Node(42)).fId == 42 From db4e494162ff01a9d8795ab029af62e9a394b8d2 Mon Sep 17 00:00:00 2001 From: Aaron Jomy Date: Wed, 2 Sep 2026 15:48:37 +0200 Subject: [PATCH 09/17] [cppjit] Fix crashes and overload regressions on new overload resolution (cherry picked from commit 0b3dc26) --- .../pyroot/cppjit/src/cpyrt/CPPOverload.cxx | 36 +++++++++++++++++-- .../pyroot/cppjit/src/cpyrt/Converters.cxx | 24 +++++++++---- .../pyroot/cppjit/src/cpyrt/Dispatcher.cxx | 1 - .../pyroot/cppjit/src/cpyrt/TemplateProxy.cxx | 5 ++- 4 files changed, 55 insertions(+), 11 deletions(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx index a80d0277a5dc7..4f77d5dcdbfe1 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx @@ -792,9 +792,13 @@ static PyObject* mp_vectorcall(CPPOverload* pymeth, PyObject* const* args, // and might need the self* for the overload resolution // to pick up the custom generated constructor defined in // __cppjit_internal namespace - // FIXME: for the self parameter changes in CppInterOp std::string self_type_name = ""; - if (PyVectorcall_NARGS(nargsf) > 0) { + // only prepend self and retry when there are real value args to forward to + // a generated/forwarding ctor. with no value args this is default + // construction: prepending self would then spuriously match the implicit + // copy ctor (self binds const T&) instead of failing, which is what the + // base-second dispatcher hit. + if (PyVectorcall_NARGS(nargsf) > 0 && !proto.empty()) { PyObject* obj = cpyrt_PyArgs_GET_ITEM(args, 0); PyObject* typ = (PyObject*)Py_TYPE(obj); AddTypeName(self_type_name, typ, nullptr, Utility::kNone); @@ -804,6 +808,34 @@ static PyObject* mp_vectorcall(CPPOverload* pymeth, PyObject* const* args, /*TODO:*/ nullptr, is_operator); } } + // Subscript tie-break: cppjit maps both operator[] and operator() into the + // __getitem__/__setitem__ overload set (operator() is needed for 2D access, + // m[i,j] -> operator()(i,j)). For single-index access both can have identical + // signatures, which Clang then reports as ambiguous -- yet in C++ `x[i]` + // binds to operator[] alone and never considers operator(). Mirror that: if + // the ambiguity for a subscript method contains exactly one operator[], + // prefer it. + if (!meth && !ambiguous_candidates.empty() && + (pymeth->fMethodInfo->fName == "__getitem__" || + pymeth->fMethodInfo->fName == "__setitem__")) { + interop::TCppMethod_t subscript = nullptr; + bool unique = true; + for (auto cand : ambiguous_candidates) { + const std::string nm = interop::GetScopedFinalName(cand.data); + if (nm.size() >= 10 && + nm.compare(nm.size() - 10, 10, "operator[]") == 0) { + if (subscript) { + unique = false; + break; + } + subscript = cand; + } + } + if (subscript && unique) { + meth = subscript; + ambiguous_candidates.clear(); + } + } if (meth) { for (auto i : pymeth->fMethodInfo->fMethods) { if (i->GetMethod() == meth) { diff --git a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx index b03108ef9e508..d19f07115815c 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx @@ -3013,9 +3013,13 @@ static void* PyFunction_AsCPointer(PyObject* pyobject, return nullptr; // TODO: is there no easier way? - static interop::TCppScope_t scope = - interop::GetScope("__cppjit_internal"); + // FIXME: cache the scope handle again (llvm/llvm-project#201844): on + // released LLVM a static handle to __cppjit_internal goes stale across + // incremental PTUs and the lookup below comes back empty. + interop::TCppScope_t scope = interop::GetScope("__cppjit_internal"); const auto& methods = interop::GetMethodsFromName(scope, wname.str()); + if (methods.empty()) + return nullptr; wpraddress = interop::GetFunctionAddress(methods[0], false); sWrapperReference[wpraddress] = ref; @@ -3093,17 +3097,23 @@ bool cpyrt::FunctionPointerConverter::ToMemory(PyObject* pyobject, //- std::function converter -------------------------------------------------- bool cpyrt::StdFunctionConverter::SetArg(PyObject* pyobject, Parameter& para, CallContext* ctxt) { - // prefer normal "object" conversion - CallContextRAII noimp(ctxt); - if (fConverter->SetArg(pyobject, para, ctxt)) - return true; + // prefer normal "object" conversion; keep kNoImplicit scoped to this attempt + // only, so it does not suppress the implicit copy the wrapper-fallback relies + // on + { + CallContextRAII noimp(ctxt); + if (fConverter->SetArg(pyobject, para, ctxt)) + return true; + } PyErr_Clear(); // else create a wrapper function if (this->FunctionPointerConverter::SetArg(pyobject, para, ctxt)) { // retrieve the wrapper pointer and capture it in a temporary std::function, - // then try normal conversion a second time + // then convert again with implicit conversion allowed (kNoImplicit scoped + // out above): the wrapper is flagged kIsLValue for reuse, so binding it to + // a std::function&& parameter needs an implicit copy, not a move. PyObject* func = this->FunctionPointerConverter::FromMemory(¶.fValue.fVoidp); if (func) { diff --git a/bindings/pyroot/cppjit/src/cpyrt/Dispatcher.cxx b/bindings/pyroot/cppjit/src/cpyrt/Dispatcher.cxx index dcfbd2071a5df..40b1dbdfa74b7 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Dispatcher.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Dispatcher.cxx @@ -524,7 +524,6 @@ bool cpyrt::InsertDispatcher(CPPScope* klass, PyObject* bases, PyObject* dct, code << "};\n}"; // finally, compile the code - // printf("\n\n%s\n\n", code.str().c_str()); if (!interop::Compile(code.str())) { err << "failed to compile the dispatcher code"; return false; diff --git a/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx b/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx index d59eaa768499d..d368c9d10d152 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx @@ -68,7 +68,10 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, bool skipSelf = fTI->fIsConstructor && (!fSelf || fSelf == Py_None); if (fSelf && (fSelf != Py_None) && (!fTI->fIsConstructor)) { PyObject* self = (PyObject*)fSelf; - assert(AddTypeName(proto, (PyObject*)Py_TYPE(self), self, Utility::kNone)); + bool addedSelf = + AddTypeName(proto, (PyObject*)Py_TYPE(self), self, Utility::kNone); + assert(addedSelf); + (void)addedSelf; } // adjust arguments for self if this is a rebound global function From d45e4a14c10981e9554f3fee586473e9f3912b0a Mon Sep 17 00:00:00 2001 From: Aaron Jomy Date: Wed, 2 Sep 2026 15:48:47 +0200 Subject: [PATCH 10/17] [cppjit] Update tests for new overload resolution behaviour (cherry picked from commit 3f12220) --- bindings/pyroot/cppjit/test/test_boost.py | 6 ++++++ bindings/pyroot/cppjit/test/test_crossinheritance.py | 3 +++ bindings/pyroot/cppjit/test/test_eigen.py | 6 ++++++ bindings/pyroot/cppjit/test/test_fragile.py | 9 ++++++++- bindings/pyroot/cppjit/test/test_numba.py | 10 ++++++---- bindings/pyroot/cppjit/test/test_stltypes.py | 9 +++++++++ bindings/pyroot/cppjit/test/test_templates.py | 3 +++ 7 files changed, 41 insertions(+), 5 deletions(-) diff --git a/bindings/pyroot/cppjit/test/test_boost.py b/bindings/pyroot/cppjit/test/test_boost.py index 157c066ffb448..79a43cae07e7e 100644 --- a/bindings/pyroot/cppjit/test/test_boost.py +++ b/bindings/pyroot/cppjit/test/test_boost.py @@ -58,6 +58,12 @@ def test01_any_class(self): run=False, reason="invalid reads in the JITed boost constructors under valgrind", ) + @mark.xfail( + reason="New Overload Resolution: a forwarding-reference template " + "operator= (any::operator=(ValueType&&)) fails to instantiate for " + "lvalue arguments, and the non-template move-assign fallback rightly " + "rejects an lvalue; pending" + ) def test02_any_usage(self): """boost::any assignment and casting""" diff --git a/bindings/pyroot/cppjit/test/test_crossinheritance.py b/bindings/pyroot/cppjit/test/test_crossinheritance.py index 10e2394c800da..79b889d07e8a7 100644 --- a/bindings/pyroot/cppjit/test/test_crossinheritance.py +++ b/bindings/pyroot/cppjit/test/test_crossinheritance.py @@ -1085,6 +1085,9 @@ def return_const(self): assert a.return_const().m_value == "abcdef" assert ns.callit(a).m_value == "abcdef" + @mark.xfail( + reason="New Overload Resolution: super().__init__(self) should forward self to the templated NoCopyNoMove(DerivedType*) ctor but mis-resolves to the deleted copy ctor Dispatcher(Dispatcher&); pending" + ) def test24_non_copyable(self): """Inheriting from a non-copyable base class""" diff --git a/bindings/pyroot/cppjit/test/test_eigen.py b/bindings/pyroot/cppjit/test/test_eigen.py index 88a077236e1b6..fc896deafd0d5 100644 --- a/bindings/pyroot/cppjit/test/test_eigen.py +++ b/bindings/pyroot/cppjit/test/test_eigen.py @@ -28,6 +28,12 @@ def setup_class(cls): warnings.simplefilter("ignore") cppjit.include("Eigen/Dense") + @mark.xfail( + reason="New Overload Resolution: operators a using-declaration imports " + "(Eigen brings the DenseBase subscript operators into Matrix that way) " + "are enumerated as using-shadows since CppInterOp #995, and " + "Cpp::BestOverloadFunctionMatch skips shadow candidates; pending" + ) def test01_simple_matrix_and_vector(self): """Basic creation of an Eigen::Matrix and Eigen::Vector""" diff --git a/bindings/pyroot/cppjit/test/test_fragile.py b/bindings/pyroot/cppjit/test/test_fragile.py index 1732aae63fedb..2dfe15fd161ba 100644 --- a/bindings/pyroot/cppjit/test/test_fragile.py +++ b/bindings/pyroot/cppjit/test/test_fragile.py @@ -656,7 +656,7 @@ def test25_cppdef_error_reporting(self): int add42(int i) { return i + 42; } }""") - @mark.xfail(condition=IS_CLANG_REPL, reason="Fails on ClangRepl") + @mark.xfail(condition=IS_CLANG_REPL, reason="Fails with ClangRepl") @mark.xfail( run=False, reason="rolling back a failed declaration asserts in ROOT's unload " @@ -778,6 +778,13 @@ def test31_template_with_class_enum(self): for ns, val in [(cppjit.gbl, 42), (cppjit.gbl.ClassEnumNS, 37)]: assert ns.EnumTemplate[ns.ClassEnumA.A]().foo() == val + @mark.xfail( + run=False, + reason="New Overload Resolution: calling through a pointer-valued global " + "proxy deduces the self argument as T*, and Cpp::BestOverloadFunctionMatch " + "feeds that rvalue classification into Sema::AddMethodCandidate, which " + "asserts FromClassification.isLValue() on assert-enabled clang", + ) def test32_overloaded_method_error_with_null_object(self): """Check exception type and message when method invoked on instance without C++ object""" diff --git a/bindings/pyroot/cppjit/test/test_numba.py b/bindings/pyroot/cppjit/test/test_numba.py index 081e4aeb2569c..4fa654ff15ba6 100644 --- a/bindings/pyroot/cppjit/test/test_numba.py +++ b/bindings/pyroot/cppjit/test/test_numba.py @@ -570,8 +570,9 @@ def square_vec_slow(x): a = np.random.randint(1, 100, size=10000, dtype=np.int64) b = np.random.randint(1, 4, size=10, dtype=np.int64) - x = cppjit.gbl.std.vector["long"](a.flatten()) - y = cppjit.gbl.std.vector["long"](b.flatten()) + # std::vector no longer fill-constructs from a raw buffer; go via list + x = cppjit.gbl.std.vector["long"](a.flatten().tolist()) + y = cppjit.gbl.std.vector["long"](b.flatten().tolist()) t0 = time.time() add_vec_fast(ns.BoxVector(x)) @@ -658,8 +659,9 @@ def np_dot_product(x, y): # for i in a: # vec_list.append([vector['long'](i[0]), vector['long'](i[1])]) - x = cppjit.gbl.std.vector["long"](a.flatten()) - y = cppjit.gbl.std.vector["long"](b.flatten()) + # std::vector no longer fill-constructs from a raw buffer; go via list + x = cppjit.gbl.std.vector["long"](a.flatten().tolist()) + y = cppjit.gbl.std.vector["long"](b.flatten().tolist()) d = ns.DotVector(x, y) dot_product_fast(d) res = 0 diff --git a/bindings/pyroot/cppjit/test/test_stltypes.py b/bindings/pyroot/cppjit/test/test_stltypes.py index 7148bb83d1865..0efbed10cb9c9 100644 --- a/bindings/pyroot/cppjit/test/test_stltypes.py +++ b/bindings/pyroot/cppjit/test/test_stltypes.py @@ -1871,6 +1871,12 @@ def test01_string_through_string_view(self): assert countit(v) == 4 assert countit_cr(v) == 4 + @mark.xfail( + reason="New Overload Resolution: the one-argument " + "basic_string_view(const char*) constructor is never considered for " + "a Python str (the two-argument (const char*, size_t) form works); " + "pending" + ) def test02_string_view_from_unicode(self): """Life-time management of converted unicode strings""" @@ -2502,6 +2508,9 @@ def test02_span_argument_conversions(self): class TestSTLANY: @mark.xfail(condition=IS_WINDOWS, reason="Fails on Windows") + @mark.xfail( + reason="New Overload Resolution: std::make_any(MyClass&) is ill-formed C++ (clang: no matching function); test relied on non-standard cppjit conversion" + ) def test01_make_any(self): """ Test that std::make_any can be used for class types. diff --git a/bindings/pyroot/cppjit/test/test_templates.py b/bindings/pyroot/cppjit/test/test_templates.py index e362d7f968986..eedff095f7079 100644 --- a/bindings/pyroot/cppjit/test/test_templates.py +++ b/bindings/pyroot/cppjit/test/test_templates.py @@ -1227,6 +1227,9 @@ class TNaVU; @mark.xfail( condition=IS_MAC and IS_CLING, run=False, reason="Crashes on OS X + Cling" ) + @mark.xfail( + reason="New Overload Resolution: passing a null-bound instance where a pointer is expected (bind_object(nullptr, T) -> T*, a cppjit convenience not expressible in regular C++ — the plain nullptr form works) fails in InstanceConverter — pending" + ) def test33_using_template_argument(self): """`using` type as template argument""" From 4fadd289b0bbcd7270bd1a1425a2568ca8e122dc Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 13 Oct 2026 10:41:03 +0530 Subject: [PATCH 11/17] [cppjit] minor refactors (cherry picked from commit a57f54a) --- .../pyroot/cppjit/src/cpyrt/CPPOverload.cxx | 7 +--- .../pyroot/cppjit/src/cpyrt/Converters.cxx | 9 ++--- bindings/pyroot/cppjit/src/cpyrt/Utility.cxx | 8 ---- .../cppjit/src/interop/cppjit_interop.h | 2 + .../cppjit/src/interop/interop_wrapper.cxx | 38 ++++++++++--------- bindings/pyroot/cppjit/test/test_fragile.py | 2 +- 6 files changed, 29 insertions(+), 37 deletions(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx index 4f77d5dcdbfe1..3a4ae0c6def95 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx @@ -794,10 +794,7 @@ static PyObject* mp_vectorcall(CPPOverload* pymeth, PyObject* const* args, // __cppjit_internal namespace std::string self_type_name = ""; // only prepend self and retry when there are real value args to forward to - // a generated/forwarding ctor. with no value args this is default - // construction: prepending self would then spuriously match the implicit - // copy ctor (self binds const T&) instead of failing, which is what the - // base-second dispatcher hit. + // a generated/forwarding ctor. if (PyVectorcall_NARGS(nargsf) > 0 && !proto.empty()) { PyObject* obj = cpyrt_PyArgs_GET_ITEM(args, 0); PyObject* typ = (PyObject*)Py_TYPE(obj); @@ -838,7 +835,7 @@ static PyObject* mp_vectorcall(CPPOverload* pymeth, PyObject* const* args, } if (meth) { for (auto i : pymeth->fMethodInfo->fMethods) { - if (i->GetMethod() == meth) { + if (interop::GetUnderlyingScope(i->GetMethod().data).data == meth) { PyObject* result = i->Call(im_self, args, nargsf, kwds, &ctxt); if (result) { // success: update the dispatch map for subsequent calls diff --git a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx index d19f07115815c..4a2265eaf9a1e 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx @@ -3013,13 +3013,10 @@ static void* PyFunction_AsCPointer(PyObject* pyobject, return nullptr; // TODO: is there no easier way? - // FIXME: cache the scope handle again (llvm/llvm-project#201844): on - // released LLVM a static handle to __cppjit_internal goes stale across - // incremental PTUs and the lookup below comes back empty. - interop::TCppScope_t scope = interop::GetScope("__cppjit_internal"); + static interop::TCppScope_t scope = + interop::GetScope("__cppjit_internal"); const auto& methods = interop::GetMethodsFromName(scope, wname.str()); - if (methods.empty()) - return nullptr; + assert(methods.size() == 1); wpraddress = interop::GetFunctionAddress(methods[0], false); sWrapperReference[wpraddress] = ref; diff --git a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx index 208610c60bcee..1a8312fa4d129 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx @@ -31,7 +31,6 @@ using namespace cppjit; //- data _____________________________________________________________________ typedef std::unordered_map TC2POperatorMapping_t; static TC2POperatorMapping_t gC2POperatorMapping; -static std::set gOpSkip; static std::set gOpRemove; extern PyTypeObject PyNullPtr_t_Type; @@ -51,13 +50,6 @@ struct InitOperatorMapping_t { InitOperatorMapping_t() { // Initialize the global map of operator names C++ -> python. - gOpSkip.insert("[]"); // __s/getitem__, depends on return type - gOpSkip.insert("+"); // __add__, depends on # of args (see __pos__) - gOpSkip.insert("-"); // __sub__, id. (eq. __neg__) - gOpSkip.insert("*"); // __mul__ or __deref__ - gOpSkip.insert("++"); // __postinc__ or __preinc__ - gOpSkip.insert("--"); // __postdec__ or __predec__ - gOpRemove.insert("new"); // this and the following not handled at all gOpRemove.insert("new[]"); gOpRemove.insert("delete"); diff --git a/bindings/pyroot/cppjit/src/interop/cppjit_interop.h b/bindings/pyroot/cppjit/src/interop/cppjit_interop.h index 90ee79dd57e43..9fc5e75e32586 100644 --- a/bindings/pyroot/cppjit/src/interop/cppjit_interop.h +++ b/bindings/pyroot/cppjit/src/interop/cppjit_interop.h @@ -67,6 +67,8 @@ TCppType_t ResolveEnumReferenceType(TCppType_t type); RPY_EXPORTED TCppType_t ResolveEnumPointerType(TCppType_t type); RPY_EXPORTED +TCppType_t GetNonReferenceType(TCppType_t type); +RPY_EXPORTED TCppType_t GetRealType(TCppType_t type); RPY_EXPORTED TCppType_t GetPointerType(TCppType_t type); diff --git a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx index 9aa2ad245c52a..60a901baabd3f 100644 --- a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx +++ b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx @@ -361,6 +361,10 @@ interop::TCppType_t interop::ResolveEnumPointerType(TCppType_t type) { return type; } +interop::TCppType_t interop::GetNonReferenceType(interop::TCppType_t type) { + return (type && Cpp::IsReferenceType(type)) ? Cpp::GetNonReferenceType(type) : type; +} + interop::TCppType_t int_like_type(interop::TCppType_t type) { interop::TCppType_t check_int_typedefs = type; if (Cpp::IsPointerType(check_int_typedefs)) @@ -410,7 +414,7 @@ interop::TCppType_t interop::GetPointerType(TCppType_t type) { std::lock_guard Lock(InterOpMutex); Cpp::ValueKind ref = Cpp::GetValueKind(type); TCppType_t pointer_type = type; - pointer_type = Cpp::GetPointerType(Cpp::GetNonReferenceType(type)); + pointer_type = Cpp::GetPointerType(interop::GetNonReferenceType(type)); if (ref == Cpp::ValueKind::LValue) return Cpp::GetReferencedType(pointer_type, false); if (ref == Cpp::ValueKind::RValue) @@ -1856,8 +1860,7 @@ interop::TCppMethod_t BestOverloadFunctionMatch( Cpp::ValueKind ref = Cpp::ValueKind::None; bool pointer = false; static interop::TCppScope_t initializer_list_scope = - interop::GetScopeFromType(interop::GetTypeFromScope( - interop::GetScope("std::initializer_list"))); + Cpp::GetTemplatedDecl(interop::GetScope("std::initializer_list")); static interop::TCppScope_t vector_scope = interop::GetScope("std::vector"); static interop::TCppType_t string_type = interop::GetRealType( @@ -1879,13 +1882,13 @@ interop::TCppMethod_t BestOverloadFunctionMatch( old_type = arg.m_Type; arg.m_Type = interop::GetPointerType(arg.m_Type).data; if (ref != Cpp::ValueKind::None) - arg.m_Type = Cpp::GetNonReferenceType(arg.m_Type).data; + arg.m_Type = interop::GetNonReferenceType(arg.m_Type).data; } else if (Cpp::IsEquivalentTypes(string_type, arg.m_Type, qual, ref, pointer) && (Cpp::IsEquivalentTypes(char_pointer_type, fn_arg_type, qual, ref, pointer)) && - (Cpp::IsPointerType(Cpp::GetNonReferenceType( - interop::ResolveType(fn_arg_type))))) { + (Cpp::IsPointerType( + interop::GetNonReferenceType(interop::ResolveType(fn_arg_type))))) { // promote std::string to char* old_type = arg.m_Type; arg.m_Type = char_pointer_type.data; @@ -1901,8 +1904,8 @@ interop::TCppMethod_t BestOverloadFunctionMatch( pointer) && (Cpp::IsEquivalentTypes(char_pointer_type, arg.m_Type, qual, ref, pointer)) && - (Cpp::IsPointerType(Cpp::GetNonReferenceType( - interop::ResolveType(arg.m_Type))))) { + (Cpp::IsPointerType( + interop::GetNonReferenceType(interop::ResolveType(arg.m_Type))))) { // promote char* to std::string old_type = arg.m_Type; arg.m_Type = string_type.data; @@ -1910,8 +1913,8 @@ interop::TCppMethod_t BestOverloadFunctionMatch( pointer) && (Cpp::IsEquivalentTypes(wchar_pointer_type, fn_arg_type, qual, ref, pointer)) && - (Cpp::IsPointerType(Cpp::GetNonReferenceType( - interop::ResolveType(fn_arg_type))))) { + (Cpp::IsPointerType( + interop::GetNonReferenceType(interop::ResolveType(fn_arg_type))))) { // promote std::string to wchar_t* old_type = arg.m_Type; arg.m_Type = wchar_pointer_type.data; @@ -1919,8 +1922,8 @@ interop::TCppMethod_t BestOverloadFunctionMatch( pointer) && (Cpp::IsEquivalentTypes(wchar_pointer_type, arg.m_Type, qual, ref, pointer)) && - (Cpp::IsPointerType(Cpp::GetNonReferenceType( - interop::ResolveType(arg.m_Type))))) { + (Cpp::IsPointerType( + interop::GetNonReferenceType(interop::ResolveType(arg.m_Type))))) { // promote wchar_t* to std::wstring old_type = arg.m_Type; arg.m_Type = wstring_type.data; @@ -1958,13 +1961,12 @@ interop::TCppMethod_t BestOverloadFunctionMatch( if (ref != Cpp::ValueKind::None) { if (Cpp::GetValueKind(arg.m_Type) == ref) { // remove reference - arg.m_Type = Cpp::GetNonReferenceType(arg.m_Type).data; + arg.m_Type = interop::GetNonReferenceType(arg.m_Type).data; } else { // create a reference - arg.m_Type = - Cpp::GetReferencedType(Cpp::GetNonReferenceType(arg.m_Type), - ref == Cpp::ValueKind::RValue) - .data; + arg.m_Type = Cpp::GetReferencedType(interop::GetNonReferenceType(arg.m_Type), + ref == Cpp::ValueKind::RValue) + .data; } } if (pointer) { @@ -2186,6 +2188,8 @@ interop::TCppMethod_t interop::GetGlobalOperator( // method properties --------------------------------------------------------- bool interop::IsDeletedMethod(TCppMethod_t method) { + if (Cpp::IsTemplatedFunction(method)) + method = Cpp::GetTemplatedDecl(method.data).data; return Cpp::IsFunctionDeleted(method); } diff --git a/bindings/pyroot/cppjit/test/test_fragile.py b/bindings/pyroot/cppjit/test/test_fragile.py index 2dfe15fd161ba..e82c9bd654e01 100644 --- a/bindings/pyroot/cppjit/test/test_fragile.py +++ b/bindings/pyroot/cppjit/test/test_fragile.py @@ -656,7 +656,7 @@ def test25_cppdef_error_reporting(self): int add42(int i) { return i + 42; } }""") - @mark.xfail(condition=IS_CLANG_REPL, reason="Fails with ClangRepl") + @mark.xfail(condition=IS_CLANG_REPL, reason="Fails with ClangRepl (passes with LLVM 23)") @mark.xfail( run=False, reason="rolling back a failed declaration asserts in ROOT's unload " From b69c600d3226842f7023f2fcddb29596ea99c461 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 13 Oct 2026 16:26:00 +0530 Subject: [PATCH 12/17] [cppjit] Implement `IsEquivalentTypes` to check if two types are equivalent `IsEquivalentTypes` checks whether two types are equivalent up to CVR qualifiers, references and pointers, or whether one type derives from the other. Through its out-parameters it reports how the types differ: - `qual`: the CVR qualifiers that differ, - `ref`: the kind of reference (lvalue/rvalue) that was stripped, - `pointer`: whether a pointer was stripped. (cherry picked from commit f48cc29) --- .../cppjit/src/interop/interop_wrapper.cxx | 146 +++++++++++++----- 1 file changed, 107 insertions(+), 39 deletions(-) diff --git a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx index 60a901baabd3f..e1c8e777e6095 100644 --- a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx +++ b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx @@ -1793,16 +1793,84 @@ bool interop::IsNonStaticMethod(interop::TCppMethod_t func) { return Cpp::IsMethod(func) && !Cpp::IsStaticMethod(func); } +static bool IsEquivalentTypesImpl(interop::TCppType_t typ1, + interop::TCppType_t typ2, Cpp::QualKind& qual, + Cpp::ValueKind& ref, bool& pointer) { + // check if same type + if (Cpp::IsSameType(typ1, typ2) || Cpp::IsTypeDerivedFrom(typ1, typ2) || + Cpp::IsTypeDerivedFrom(typ2, typ1)) + return true; + + // check if same type after removing qualifiers; canonicalize first so that + // qualifiers hidden behind a typedef are removed too + if (Cpp::IsSameType(Cpp::RemoveTypeQualifier(Cpp::GetCanonicalType(typ1), + Cpp::QualKind::All), + Cpp::RemoveTypeQualifier(Cpp::GetCanonicalType(typ2), + Cpp::QualKind::All))) { + for (Cpp::QualKind q : {Cpp::QualKind::Const, Cpp::QualKind::Volatile, + Cpp::QualKind::Restrict}) + if (Cpp::HasTypeQualifier(typ1, q) != Cpp::HasTypeQualifier(typ2, q)) + qual = qual | q; + return true; + } + + // check if same type after removing references + if (Cpp::IsReferenceType(typ1) && + IsEquivalentTypesImpl(Cpp::GetNonReferenceType(typ1), typ2, qual, ref, + pointer)) { + ref = Cpp::GetValueKind(typ1); + return true; + } + if (Cpp::IsReferenceType(typ2) && + IsEquivalentTypesImpl(Cpp::GetNonReferenceType(typ2), typ1, qual, ref, + pointer)) { + ref = Cpp::GetValueKind(typ2); + return true; + } + + // check if same type after removing pointers + if (Cpp::IsPointerType(typ1) && + IsEquivalentTypesImpl(Cpp::GetPointeeType(typ1), typ2, qual, ref, + pointer)) { + pointer = true; + return true; + } + if (Cpp::IsPointerType(typ2) && + IsEquivalentTypesImpl(Cpp::GetPointeeType(typ2), typ1, qual, ref, + pointer)) { + pointer = true; + return true; + } + return false; +} + +// Checks if two types are equivalent up to CVR qualifiers, references and +// pointers, or if one type derives from the other. On return, qual holds the +// CVR qualifiers that differ between the two types, ref the kind of reference +// that was stripped and pointer whether a pointer was stripped. +static bool IsEquivalentTypes(interop::TCppType_t typ1, + interop::TCppType_t typ2, Cpp::QualKind& qual, + Cpp::ValueKind& ref, bool& pointer) { + qual = static_cast(0); + ref = Cpp::ValueKind::None; + pointer = false; + if (!typ1 || !typ2) + return false; + return IsEquivalentTypesImpl(typ1, typ2, qual, ref, pointer); +} + bool IsTemplateParamEquivalentTypes(interop::TCppType_t typ1, interop::TCppType_t typ2, Cpp::QualKind& qual, Cpp::ValueKind& ref, bool& pointer) { - qual = Cpp::QualKind::None; + qual = static_cast(0); ref = Cpp::ValueKind::None; pointer = false; - ref = ref ^ Cpp::GetValueKind(typ1); - ref = ref ^ Cpp::GetValueKind(typ2); + Cpp::ValueKind ref1 = Cpp::GetValueKind(typ1); + Cpp::ValueKind ref2 = Cpp::GetValueKind(typ2); + if (ref1 != ref2) + ref = ref1 != Cpp::ValueKind::None ? ref1 : ref2; typ2 = Cpp::IsReferenceType(typ2) ? Cpp::GetNonReferenceType(typ2) : typ2; pointer ^= Cpp::IsPointerType(typ1); @@ -1856,13 +1924,13 @@ interop::TCppMethod_t BestOverloadFunctionMatch( I++; continue; } - Cpp::QualKind qual = Cpp::QualKind::None; + Cpp::QualKind qual = static_cast(0); Cpp::ValueKind ref = Cpp::ValueKind::None; bool pointer = false; static interop::TCppScope_t initializer_list_scope = Cpp::GetTemplatedDecl(interop::GetScope("std::initializer_list")); - static interop::TCppScope_t vector_scope = - interop::GetScope("std::vector"); + // static interop::TCppScope_t vector_scope = + // interop::GetScope("std::vector"); static interop::TCppType_t string_type = interop::GetRealType( interop::GetTypeFromScope(interop::GetScope("std::string"))); static interop::TCppType_t wstring_type = interop::GetRealType( @@ -1876,65 +1944,65 @@ interop::TCppMethod_t BestOverloadFunctionMatch( Cpp::GetFunctionArgType(fn, skipThis ? I - 1 : I); assert(string_type); interop::TCppType_t old_type = nullptr; - if (Cpp::IsEquivalentTypes(arg.m_Type, fn_arg_type, qual, ref, pointer) && + if (IsEquivalentTypes(arg.m_Type, fn_arg_type, qual, ref, pointer) && pointer) { // Only promotion to pointer should be allowed. old_type = arg.m_Type; arg.m_Type = interop::GetPointerType(arg.m_Type).data; if (ref != Cpp::ValueKind::None) arg.m_Type = interop::GetNonReferenceType(arg.m_Type).data; - } else if (Cpp::IsEquivalentTypes(string_type, arg.m_Type, qual, ref, - pointer) && - (Cpp::IsEquivalentTypes(char_pointer_type, fn_arg_type, qual, - ref, pointer)) && - (Cpp::IsPointerType( - interop::GetNonReferenceType(interop::ResolveType(fn_arg_type))))) { + } else if (IsEquivalentTypes(string_type, arg.m_Type, qual, ref, + pointer) && + (IsEquivalentTypes(char_pointer_type, fn_arg_type, qual, ref, + pointer)) && + (Cpp::IsPointerType(interop::GetNonReferenceType( + interop::ResolveType(fn_arg_type))))) { // promote std::string to char* old_type = arg.m_Type; arg.m_Type = char_pointer_type.data; - } else if (Cpp::IsEquivalentTypes(string_type, arg.m_Type, qual, ref, - pointer) && - (Cpp::IsEquivalentTypes(char_type, fn_arg_type, qual, ref, - pointer))) { + } else if (IsEquivalentTypes(string_type, arg.m_Type, qual, ref, + pointer) && + (IsEquivalentTypes(char_type, fn_arg_type, qual, ref, + pointer))) { // promote std::string to char // is this fine? old_type = arg.m_Type; arg.m_Type = char_type.data; - } else if (Cpp::IsEquivalentTypes(string_type, fn_arg_type, qual, ref, - pointer) && - (Cpp::IsEquivalentTypes(char_pointer_type, arg.m_Type, qual, - ref, pointer)) && - (Cpp::IsPointerType( - interop::GetNonReferenceType(interop::ResolveType(arg.m_Type))))) { + } else if (IsEquivalentTypes(string_type, fn_arg_type, qual, ref, + pointer) && + (IsEquivalentTypes(char_pointer_type, arg.m_Type, qual, ref, + pointer)) && + (Cpp::IsPointerType(interop::GetNonReferenceType( + interop::ResolveType(arg.m_Type))))) { // promote char* to std::string old_type = arg.m_Type; arg.m_Type = string_type.data; - } else if (Cpp::IsEquivalentTypes(string_type, arg.m_Type, qual, ref, - pointer) && - (Cpp::IsEquivalentTypes(wchar_pointer_type, fn_arg_type, qual, - ref, pointer)) && - (Cpp::IsPointerType( - interop::GetNonReferenceType(interop::ResolveType(fn_arg_type))))) { + } else if (IsEquivalentTypes(string_type, arg.m_Type, qual, ref, + pointer) && + (IsEquivalentTypes(wchar_pointer_type, fn_arg_type, qual, ref, + pointer)) && + (Cpp::IsPointerType(interop::GetNonReferenceType( + interop::ResolveType(fn_arg_type))))) { // promote std::string to wchar_t* old_type = arg.m_Type; arg.m_Type = wchar_pointer_type.data; - } else if (Cpp::IsEquivalentTypes(wstring_type, fn_arg_type, qual, ref, - pointer) && - (Cpp::IsEquivalentTypes(wchar_pointer_type, arg.m_Type, qual, - ref, pointer)) && - (Cpp::IsPointerType( - interop::GetNonReferenceType(interop::ResolveType(arg.m_Type))))) { + } else if (IsEquivalentTypes(wstring_type, fn_arg_type, qual, ref, + pointer) && + (IsEquivalentTypes(wchar_pointer_type, arg.m_Type, qual, ref, + pointer)) && + (Cpp::IsPointerType(interop::GetNonReferenceType( + interop::ResolveType(arg.m_Type))))) { // promote wchar_t* to std::wstring old_type = arg.m_Type; arg.m_Type = wstring_type.data; - // } else if (Cpp::IsEquivalentTypes(string_type, arg.m_Type, qual, ref, - // pointer) && (Cpp::IsEquivalentTypes(wstring_type, fn_arg_type, qual, + // } else if (IsEquivalentTypes(string_type, arg.m_Type, qual, ref, + // pointer) && (IsEquivalentTypes(wstring_type, fn_arg_type, qual, // ref, pointer))) { // // promote std::string to std::wstring // old_type = arg.m_Type; // arg.m_Type = wstring_type.data; - // } else if (Cpp::IsEquivalentTypes(char_pointer_type, arg.m_Type, - // qual, ref, pointer) && (Cpp::IsEquivalentTypes(wstring_type, + // } else if (IsEquivalentTypes(char_pointer_type, arg.m_Type, + // qual, ref, pointer) && (IsEquivalentTypes(wstring_type, // fn_arg_type, qual, ref, pointer)) && // (Cpp::IsPointerType(Cpp::GetNonReferenceType(interop::ResolveType(arg.m_Type)))) // && From 6c0cdfa9e5d5bec4a3326091820b564ebbbdf902 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 13 Oct 2026 17:11:26 +0530 Subject: [PATCH 13/17] [cppjit] `BestOverloadFunctionMatch` loop optimization (cherry picked from commit e24c182) --- .../cppjit/src/interop/interop_wrapper.cxx | 36 ++++++++++--------- bindings/pyroot/cppjit/test/test_overloads.py | 22 ++++++++++++ 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx index e1c8e777e6095..d0c195e754eda 100644 --- a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx +++ b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx @@ -1885,7 +1885,7 @@ bool IsTemplateParamEquivalentTypes(interop::TCppType_t typ1, namespace { interop::TCppMethod_t BestOverloadFunctionMatch( const std::vector& candidates, - std::vector& arg_types, + const std::vector& arg_types, const std::vector& templ_params, std::vector& ambiguous_candidates, interop::TCppScope_t parent_scope, bool is_operator) { @@ -1917,8 +1917,13 @@ interop::TCppMethod_t BestOverloadFunctionMatch( else potential_cppmeth_ambiguous = true; + // promote the arguments towards the parameters of fn, then resolve the + // overload once with all of them promoted + // ???: Can this cause ambiguity + std::vector promoted_arg_types = arg_types; + bool promoted = false; size_t I = 0; - for (auto& arg : arg_types) { + for (auto& arg : promoted_arg_types) { if (I == 0 && skipThis) { // ignore the this parameter I++; @@ -2052,23 +2057,20 @@ interop::TCppMethod_t BestOverloadFunctionMatch( old_type = nullptr; } } - if (old_type) { - if (!cppmeth) { - // FIXME: this call should happen outside the inner for loop? - // FIXME: How to handle ambiguous_candidates in this case? - cppmeth = Cpp::BestOverloadFunctionMatch( - candidates, {}, arg_types, ambiguous_candidates, is_operator); - } else if (interop::TCppMethod_t new_meth = - Cpp::BestOverloadFunctionMatch(candidates, {}, arg_types, - ambiguous_candidates, - is_operator); - new_meth && new_meth != cppmeth) { - return nullptr; // ambiguous, raise diagnostic - } - arg.m_Type = old_type.data; - } + if (old_type) + promoted = true; I++; } + if (!promoted) + continue; + + // FIXME: How to handle ambiguous_candidates in this case? + interop::TCppMethod_t new_meth = Cpp::BestOverloadFunctionMatch( + candidates, {}, promoted_arg_types, ambiguous_candidates, is_operator); + if (!cppmeth) + cppmeth = new_meth; + else if (new_meth && new_meth != cppmeth) + return nullptr; // ambiguous, raise diagnostic } // proper argument type match failed diff --git a/bindings/pyroot/cppjit/test/test_overloads.py b/bindings/pyroot/cppjit/test/test_overloads.py index 5125853c29379..110e94bd6993b 100644 --- a/bindings/pyroot/cppjit/test/test_overloads.py +++ b/bindings/pyroot/cppjit/test/test_overloads.py @@ -477,3 +477,25 @@ def constness(name): assert constness("static_fn") == { "static int OverloadConstness::Probe::static_fn(int x)": False } + + def test18_promote_multiple_arguments(self): + """Verify that an overload needing several argument promotions resolves.""" + + import cppjit + + cppjit.cppdef(""" + namespace MultiPromotion { + std::string join(char* a, char* b) { return std::string(a) + b; } + std::string join(int a, int b) { return std::to_string(a + b); } + std::string join_mixed(char* a, int b) { return a + std::to_string(b); } + std::string join_mixed(int a, char* b) { return std::to_string(a) + b; } + }""") + + ns = cppjit.gbl.MultiPromotion + + # both str arguments have to be promoted to char* at the same time + assert ns.join("ab", "cd") == "abcd" + assert ns.join(1, 2) == "3" + # a single promotion + assert ns.join_mixed("ab", 1) == "ab1" + assert ns.join_mixed(1, "ab") == "1ab" From edfff1c9fed69694cff0c759c3601093a169c08a Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 13 Oct 2026 17:33:45 +0530 Subject: [PATCH 14/17] [cppjit] fix cases where `ambiguous_candidates` have duplicate entries (cherry picked from commit 6f4b666) --- .../cppjit/src/interop/interop_wrapper.cxx | 21 ++++++++++++++++--- bindings/pyroot/cppjit/test/test_overloads.py | 20 ++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx index d0c195e754eda..6d35a7718758c 100644 --- a/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx +++ b/bindings/pyroot/cppjit/src/interop/interop_wrapper.cxx @@ -1891,6 +1891,15 @@ interop::TCppMethod_t BestOverloadFunctionMatch( interop::TCppScope_t parent_scope, bool is_operator) { std::lock_guard Lock(InterOpMutex); + auto remove_duplicate_methods = [](std::vector& methods) { + std::set seen; + methods.erase(std::remove_if(methods.begin(), methods.end(), + [&seen](interop::TCppMethod_t m) { + return !seen.insert(m.data).second; + }), + methods.end()); + }; + interop::TCppMethod_t cppmeth = Cpp::BestOverloadFunctionMatch( candidates, templ_params, arg_types, ambiguous_candidates, is_operator); if (cppmeth) @@ -2064,13 +2073,17 @@ interop::TCppMethod_t BestOverloadFunctionMatch( if (!promoted) continue; - // FIXME: How to handle ambiguous_candidates in this case? interop::TCppMethod_t new_meth = Cpp::BestOverloadFunctionMatch( candidates, {}, promoted_arg_types, ambiguous_candidates, is_operator); if (!cppmeth) cppmeth = new_meth; - else if (new_meth && new_meth != cppmeth) - return nullptr; // ambiguous, raise diagnostic + else if (new_meth && new_meth != cppmeth) { + // ambiguous + ambiguous_candidates.push_back(cppmeth); + ambiguous_candidates.push_back(new_meth); + remove_duplicate_methods(ambiguous_candidates); + return nullptr; + } } // proper argument type match failed @@ -2078,6 +2091,8 @@ interop::TCppMethod_t BestOverloadFunctionMatch( if (!cppmeth && !potential_cppmeth_ambiguous && !Cpp::IsTemplatedFunction(potential_cppmeth)) cppmeth = potential_cppmeth; + if (!cppmeth) + remove_duplicate_methods(ambiguous_candidates); return cppmeth; } } // namespace diff --git a/bindings/pyroot/cppjit/test/test_overloads.py b/bindings/pyroot/cppjit/test/test_overloads.py index 110e94bd6993b..b3d98e4b2c68a 100644 --- a/bindings/pyroot/cppjit/test/test_overloads.py +++ b/bindings/pyroot/cppjit/test/test_overloads.py @@ -499,3 +499,23 @@ def test18_promote_multiple_arguments(self): # a single promotion assert ns.join_mixed("ab", 1) == "ab1" assert ns.join_mixed(1, "ab") == "1ab" + + def test19_ambiguous_set_has_no_duplicates(self): + """Verify that an ambiguous call reports each candidate only once.""" + + import cppjit + + cppjit.cppdef(""" + namespace DupAmbiguity { + int f(char* a, double b) { return 1; } + int f(char* a, float b) { return 2; } + }""") + + # the str argument needs a promotion to char*, which is retried for + # each candidate, and every retry is ambiguous on the int argument + with raises(cppjit.OverloadAmbiguityException) as exc: + cppjit.gbl.DupAmbiguity.f("x", 1) + + msg = str(exc.value) + assert msg.count("double b)") == 1 + assert msg.count("float b)") == 1 From c53429c9a8f68a2585066800b7822fa378779637 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Tue, 13 Oct 2026 18:12:24 +0530 Subject: [PATCH 15/17] [cppjit] Remove dead code * Remove SelectAndForward from TemplateProxy SelectAndForward forwarded a template proxy call to one of its overload sets: tpp_vectorcall tried the non-templated overloads (implicit conversions allowed), then the known templated ones and finally the low-priority ones (both without implicit conversions). "Adopt Clang-driven overload resolution from CppInterOp" replaced that cascade with a single Clang-driven selection and its three callers went away with it; the static function has had no caller since (clang reports it with -Wunused-function). * Remove CPPDataMember_NewConstant CPPDataMember_NewConstant built a static, constant data member descriptor from a raw name and address, via the matching CPPDataMember::Set(scope, name, address) overload. Nothing calls it (it was already unused at 3df629f): every data member descriptor is created by CPPDataMember_New from a reflected variable, including enum constants. Remove the factory and the Set overload only it used. * Remove Utility::PyErr_Occurred_WithGIL It re-acquired the GIL around PyErr_Occurred() for callers that might have released it. Nothing calls it (it was already unused at 3df629f); the executors that release the GIL (GILControl in Executors.cxx) restore it before checking for errors. * remove unused call context enums based on new overload resolution * Remove the write-only kHaveImplicit call flag Converters set CallContext::kHaveImplicit when they rejected an argument that an implicit conversion could have accepted. The old overload resolution in mp_vectorcall read it through HaveImplicit() to decide whether to retry all overloads in a second round with kAllowImplicit (CPPOverload.cxx at 3df629f). "Adopt Clang-driven overload resolution from CppInterOp" removed that second round, so HaveImplicit() lost its only caller and the flag has been written but never read since. Remove the four writes in Converters.cxx, HaveImplicit() and the enumerator. * Remove the unused is_conversion_operator computation "Adopt Clang-driven overload resolution from CppInterOp" made mp_vectorcall compute whether any overload in the set is a conversion operator, but the result is never read (unlike is_operator, which is passed on to BestOverloadFunctionMatch). Remove it together with CPPMethod::IsConversionOperator, which it was the only caller of. * Drop the unused include from Converters.cxx Converters.cxx used std::regex only for s_fnptr, which recognized function-pointer spellings in CreateConverter. "Adopt Clang-driven overload resolution from CppInterOp" replaced that string matching with type queries and removed s_fnptr; nothing in the file uses any more. * Remove unreachable statements after return/throw * Remove SmartPtrConverter's never-overridden GetAddressSpecialCase SmartPtrConverter declares its own virtual GetAddressSpecialCase that always returns false, and no class derives from SmartPtrConverter, so the special-case branch in SmartPtrConverter::SetArg (meant for e.g. passing nullptr to a smart pointer reference) can never be taken. It was already dead at 3df629f. Remove the hook, the branch and the TODO that discussed it; non-instances are rejected as before. * Drop the redundant unicodeobject.h include from TemplateProxy.cxx "Adopt Clang-driven overload resolution from CppInterOp" added a direct line above) already includes; nothing else in the file needed it. * removed commented code (cherry picked from commit 25f0ed3) --- .../pyroot/cppjit/src/cpyrt/CPPDataMember.cxx | 11 ------ .../pyroot/cppjit/src/cpyrt/CPPDataMember.h | 11 ------ bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h | 3 -- .../pyroot/cppjit/src/cpyrt/CPPOverload.cxx | 4 -- bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx | 2 - .../pyroot/cppjit/src/cpyrt/CallContext.h | 12 ------ .../pyroot/cppjit/src/cpyrt/Converters.cxx | 19 +-------- .../cppjit/src/cpyrt/DeclareConverters.h | 2 - .../pyroot/cppjit/src/cpyrt/LowLevelViews.cxx | 17 ++++---- .../pyroot/cppjit/src/cpyrt/TemplateProxy.cxx | 39 ------------------- .../cppjit/src/cpyrt/TupleOfInstances.cxx | 3 -- bindings/pyroot/cppjit/src/cpyrt/Utility.cxx | 13 ------- bindings/pyroot/cppjit/src/cpyrt/Utility.h | 3 -- 13 files changed, 8 insertions(+), 131 deletions(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPDataMember.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPDataMember.cxx index 582d1dddffa6d..e737c18154c0b 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPDataMember.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPDataMember.cxx @@ -382,17 +382,6 @@ void cpyrt::CPPDataMember::Set(interop::TCppScope_t scope, fDescription = PyUnicode_FromString(name.c_str()); } -//----------------------------------------------------------------------------- -void cpyrt::CPPDataMember::Set(interop::TCppScope_t scope, - const std::string& name, void* address) { - fEnclosingScope = scope; - fDescription = PyUnicode_FromString(name.c_str()); - fOffset = (intptr_t)address; - fFlags = kIsStaticData | kIsConstData; - fConverter = CreateConverter("internal_enum_type_t"); - fFullType = "unsigned int"; -} - //----------------------------------------------------------------------------- void* cpyrt::CPPDataMember::GetAddress(CPPInstance* pyobj) { // class attributes, global properties diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPDataMember.h b/bindings/pyroot/cppjit/src/cpyrt/CPPDataMember.h index d3b0d49ac7f61..bb8973114ea89 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPDataMember.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPDataMember.h @@ -14,7 +14,6 @@ class CPPInstance; class CPPDataMember { public: void Set(interop::TCppScope_t scope, interop::TCppScope_t var); - void Set(interop::TCppScope_t scope, const std::string& name, void* address); std::string GetName(); void* GetAddress(CPPInstance* pyobj /* owner */); @@ -57,16 +56,6 @@ inline CPPDataMember* CPPDataMember_New(interop::TCppScope_t scope, return pyprop; } -inline CPPDataMember* CPPDataMember_NewConstant(interop::TCppScope_t scope, - const std::string& name, - void* address) { - // Create an initialize a new property descriptor, given the C++ datum. - CPPDataMember* pyprop = (CPPDataMember*)CPPDataMember_Type.tp_new( - &CPPDataMember_Type, nullptr, nullptr); - pyprop->Set(scope, name, address); - return pyprop; -} - } // namespace cppjit::cpyrt #endif // !CPYRT_CPPDATAMEMBER_H diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h b/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h index 85f66c66f4955..a3e7ccf3c1275 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPMethod.h @@ -75,9 +75,6 @@ class CPPMethod : public PyCallable { } // extra info methods bool IsOperator() { return interop::IsOperator(fMethod.data); } - bool IsConversionOperator() { - return interop::IsConversionOperator(fMethod.data); - } bool IsStaticMethod() { return interop::IsStaticMethod(fMethod); } public: diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx index 3a4ae0c6def95..25efe5e19b44c 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.cxx @@ -606,14 +606,10 @@ static PyObject* mp_vectorcall(CPPOverload* pymeth, PyObject* const* args, std::vector overloads; bool is_operator = false; - bool is_conversion_operator = false; bool is_static = false; for (auto i : pymeth->fMethodInfo->fMethods) { if (is_operator || dynamic_cast(i)->IsOperator()) is_operator = true; - if (is_conversion_operator || - dynamic_cast(i)->IsConversionOperator()) - is_conversion_operator = true; if (is_static || dynamic_cast(i)->IsStaticMethod()) is_static = true; overloads.push_back(i->GetMethod()); diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx b/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx index 26b47450e7dec..fde38d0d8a5b5 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPScope.cxx @@ -590,7 +590,6 @@ static PyObject* meta_reflex(CPPScope* klass, PyObject* args) { if (klass->fFlags & CPPScope::kIsNamespace) Py_RETURN_TRUE; Py_RETURN_FALSE; - break; case interop::Reflex::IS_AGGREGATE: // this is not the strict C++ definition of aggregates, but is closer to // what is needed for Numba and C calling conventions (TODO: probably have @@ -599,7 +598,6 @@ static PyObject* meta_reflex(CPPScope* klass, PyObject* args) { !interop::HasVirtualDestructor(klass->fCppType)) Py_RETURN_TRUE; Py_RETURN_FALSE; - break; } PyErr_Format(PyExc_ValueError, "unsupported reflex request %d or format %d", diff --git a/bindings/pyroot/cppjit/src/cpyrt/CallContext.h b/bindings/pyroot/cppjit/src/cpyrt/CallContext.h index cc3bf232ee10d..4c263a90241e6 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CallContext.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CallContext.h @@ -30,12 +30,8 @@ struct CallContext { enum ECallFlags { kNone = 0x000000, - // UNUSED with new overload resolution - // kIsSorted = 0x000001, // if method overload priority - // determined kIsCreator = 0x000002, // if method creates python-owned objects kIsConstructor = 0x000004, // if method is a C++ constructor - kHaveImplicit = 0x000008, // indicate that implicit converters are available kAllowImplicit = 0x000010, // indicate that implicit conversions are allowed kNoImplicit = 0x000020, // disable implicit to prevent recursion kCallDirect = 0x000040, // call wrapped method directly, no inheritance @@ -52,8 +48,6 @@ struct CallContext { kUseFFI = 0x010000, // not implemented kIsPseudoFunc = 0x020000, // internal, used for introspection kUseStrict = 0x040000, // if method applies strict memory policy - kIsMethod = 0x080000, // If this is a class method - kIsSatticMethod = 0x100000, // If this is a static class method }; // Policies about memory handling and signal safety @@ -113,12 +107,6 @@ inline bool IsConstructor(uint64_t flags) { return flags & CallContext::kIsConstructor; } -inline bool HaveImplicit(CallContext* ctxt) { - return ctxt ? (!(ctxt->fFlags & CallContext::kNoImplicit) && - (ctxt->fFlags & CallContext::kHaveImplicit)) - : false; -} - inline bool AllowImplicit(CallContext* ctxt) { return ctxt ? (!(ctxt->fFlags & CallContext::kNoImplicit) && (ctxt->fFlags & CallContext::kAllowImplicit)) diff --git a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx index 4a2265eaf9a1e..2433dfe223405 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Converters.cxx @@ -26,7 +26,6 @@ using namespace cppjit; #include #include #include // for wstring_convert -#include #include #include // for ptrdiff_t #include @@ -359,8 +358,6 @@ static inline bool ImplicitBool(PyObject* pyobject, cpyrt::CallContext* ctxt) { if (!AllowImplicit(ctxt) && (PyBool_Check(pyobject) || (IsCPPEnumInstance(pyobject) && !NoImplicit(ctxt)))) { - if (!NoImplicit(ctxt)) - ctxt->fFlags |= CallContext::kHaveImplicit; return false; } return true; @@ -369,8 +366,6 @@ static inline bool ImplicitBool(PyObject* pyobject, cpyrt::CallContext* ctxt) { static inline bool StrictBool(PyObject* pyobject, cpyrt::CallContext* ctxt) { using namespace cppjit::cpyrt; if (!AllowImplicit(ctxt) && !PyBool_Check(pyobject)) { - if (!NoImplicit(ctxt)) - ctxt->fFlags |= CallContext::kHaveImplicit; return false; } return true; @@ -538,8 +533,6 @@ static inline cpyrt::CPPInstance* ConvertImplicit(interop::TCppScope_t klass, PyTypeObject* pytype = (PyTypeObject*)Py_TYPE(pyobject); if (!(pytype == &PyList_Type || pytype == &PyTuple_Type)) { // || !CPPInstance_Check(pyobject))) { - if (!NoImplicit(ctxt)) - ctxt->fFlags |= CallContext::kHaveImplicit; return nullptr; } } @@ -1321,8 +1314,6 @@ bool cpyrt::EnumConverter::SetArg(PyObject* pyobject, Parameter& para, PyErr_Clear(); } if (!exact) { - if (!NoImplicit(ctxt)) - ctxt->fFlags |= CallContext::kHaveImplicit; return false; } // the base integer converter defers enum instances, so lift the gate @@ -3145,16 +3136,8 @@ bool cpyrt::SmartPtrConverter::SetArg(PyObject* pyobject, Parameter& para, CallContext* ctxt) { char typeCode = fIsRef ? 'p' : 'V'; - if (!CPPInstance_Check(pyobject)) { - // TODO: not sure how this is correct for pass-by-ref nor does it help with - // implicit conversions for pass-by-value - if (fIsRef && GetAddressSpecialCase(pyobject, para.fValue.fVoidp)) { - para.fTypeCode = typeCode; // allow special cases such as nullptr - return true; - } - + if (!CPPInstance_Check(pyobject)) return false; - } CPPInstance* pyobj = (CPPInstance*)pyobject; interop::TCppScope_t oisa = pyobj->ObjectIsA(); diff --git a/bindings/pyroot/cppjit/src/cpyrt/DeclareConverters.h b/bindings/pyroot/cppjit/src/cpyrt/DeclareConverters.h index c339463464ade..34f45ec4f2310 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/DeclareConverters.h +++ b/bindings/pyroot/cppjit/src/cpyrt/DeclareConverters.h @@ -504,8 +504,6 @@ class SmartPtrConverter : public Converter { std::string GetFailureMsg() override { return "[SmartPtrConverter]"; }; protected: - virtual bool GetAddressSpecialCase(PyObject*, void*&) { return false; } - interop::TCppScope_t fSmartPtrType; interop::TCppScope_t fUnderlyingType; bool fKeepControl; diff --git a/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.cxx b/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.cxx index 94e06850af62d..f3afd73c4dcc6 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/LowLevelViews.cxx @@ -864,18 +864,15 @@ static PyObject* ll_array(cpyrt::LowLevelView* self, PyObject* args, Py_DECREF(npfrombuf); return view; - - } else { // copy requested - PyObject* npcopy = PyObject_GetAttr(npmod, cpyrt::PyStrings::gCopy); - PyObject* newarr = - PyObject_CallFunctionObjArgs(npcopy, (PyObject*)self, nullptr); - Py_DECREF(npcopy); - - return newarr; } - // never get here - return nullptr; + // copy requested + PyObject* npcopy = PyObject_GetAttr(npmod, cpyrt::PyStrings::gCopy); + PyObject* newarr = + PyObject_CallFunctionObjArgs(npcopy, (PyObject*)self, nullptr); + Py_DECREF(npcopy); + + return newarr; } //--------------------------------------------------------------------------- diff --git a/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx b/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx index d368c9d10d152..81d068de76f23 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/TemplateProxy.cxx @@ -1,6 +1,5 @@ // Bindings #include "cpyrt.h" -#include "unicodeobject.h" using namespace cppjit; #include "CPPClassMethod.h" @@ -74,18 +73,6 @@ PyObject* TemplateProxy::Instantiate(const std::string& fname, (void)addedSelf; } - // adjust arguments for self if this is a rebound global function - // bool isNS = (((CPPScope*)fTI->fPyClass)->fFlags & CPPScope::kIsNamespace); - // if (!fTI->fIsConstructor && !isNS && cpyrt_PyArgs_GET_SIZE(args, nargsf) && - // \ - // (!fSelf || - // (fSelf == Py_None && - // !interop::IsStaticTemplate(((CPPScope*)fTI->fPyClass)->fCppType, - // fname)))) { - // args += 1; - // nargsf -= 1; - // } - Py_ssize_t argc = cpyrt_PyArgs_GET_SIZE(args, nargsf); if (argc != 0) { PyObject* tpArgs = PyTuple_New(skipSelf ? argc - 1 : argc); @@ -502,32 +489,6 @@ static inline void UpdateDispatchMap(TemplateProxy* pytmpl, bool use_targs, v.push_back(std::make_pair(sighash, pymeth)); } -static inline PyObject* -SelectAndForward(TemplateProxy* pytmpl, CPPOverload* pymeth, - cpyrt_PyArgs_t args, size_t nargsf, PyObject* kwds, - bool implicitOkay, bool use_targs, uint64_t sighash, - std::vector& errors) { - // Forward a call to known overloads, if any. - if (pymeth->HasMethods()) { - PyObject* pycall = CPPOverload_Type.tp_descr_get( - (PyObject*)pymeth, pytmpl->fSelf, (PyObject*)&CPPOverload_Type); - - if (!implicitOkay) - ((CPPOverload*)pycall)->fFlags |= CallContext::kNoImplicit; - - // now call the method with the arguments (loops internally) - PyObject* result = cpyrt_tp_call(pycall, args, nargsf, kwds); - Py_DECREF(pycall); - if (result) { - UpdateDispatchMap(pytmpl, use_targs, sighash, pymeth); - TPPCALL_RETURN; - } - Utility::FetchError(errors); - } - - return nullptr; -} - static inline PyObject* CallMethodImp(TemplateProxy* pytmpl, PyObject*& pymeth, cpyrt_PyArgs_t args, size_t nargsf, PyObject* kwds, bool impOK, diff --git a/bindings/pyroot/cppjit/src/cpyrt/TupleOfInstances.cxx b/bindings/pyroot/cppjit/src/cpyrt/TupleOfInstances.cxx index db31fa36b927d..23353aec3dd90 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/TupleOfInstances.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/TupleOfInstances.cxx @@ -204,9 +204,6 @@ PyObject* TupleOfInstances_New(interop::TCppObject_t address, return arr; } - - // never get here - return nullptr; } //= cpyrt custom tuple-like array type ==================================== diff --git a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx index 1a8312fa4d129..ed2d5250b617a 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx @@ -974,8 +974,6 @@ static bool AddTypeName(std::vector& types, PyObject* tn, throw std::runtime_error( "This path is not yet implemented (AddTypeName) \n"); - return true; - } else PyErr_Clear(); } @@ -1614,17 +1612,6 @@ cpyrt::Utility::PyOperators::~PyOperators() { Py_XDECREF(fHash); } -//---------------------------------------------------------------------------- -PyObject* cpyrt::Utility::PyErr_Occurred_WithGIL() { - // Re-acquire the GIL before calling PyErr_Occurred() in case it has been - // released; note that the p2.2 code assumes that there are no callbacks in - // C++ to python (or at least none returning errors). - PythonGILRAII python_gil_raii; - PyObject* e = PyErr_Occurred(); - - return e; -} - //---------------------------------------------------------------------------- cpyrt::Utility::PyError_t cpyrt::Utility::FetchPyError() { // create a PyError_t RAII object that will capture and store the exception diff --git a/bindings/pyroot/cppjit/src/cpyrt/Utility.h b/bindings/pyroot/cppjit/src/cpyrt/Utility.h index d80dedb59ba2e..4858341364484 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Utility.h +++ b/bindings/pyroot/cppjit/src/cpyrt/Utility.h @@ -109,9 +109,6 @@ struct PyOperators { std::string ClassName(PyObject* pyobj); bool IsSTLIterator(const std::string& classname); -// for threading: save call to PyErr_Occurred() -PyObject* PyErr_Occurred_WithGIL(); - // helpers for collecting/maintaining python exception data struct PyError_t { struct PyObjectDeleter { From d508a1a46a8d8884b753b1dd9804fc4a59f97627 Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Wed, 14 Oct 2026 13:09:21 +0530 Subject: [PATCH 16/17] [cppjit] fix compiler-research/cppjit#151 (cherry picked from commit 425cada) --- .../pyroot/cppjit/src/cpyrt/CPPOverload.h | 17 +++- bindings/pyroot/cppjit/src/cpyrt/Utility.cxx | 76 ++++++++------- bindings/pyroot/cppjit/src/cpyrt/Utility.h | 1 + bindings/pyroot/cppjit/test/test_templates.py | 96 ++++++++++++++++++- 4 files changed, 154 insertions(+), 36 deletions(-) diff --git a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.h b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.h index 7d17e00f02d5c..cfefe4dfe47de 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.h +++ b/bindings/pyroot/cppjit/src/cpyrt/CPPOverload.h @@ -3,6 +3,7 @@ // Bindings #include "PyCallable.h" +#include "Utility.h" #include "cppjit_interop.h" // Standard @@ -111,10 +112,20 @@ inline uint64_t HashSignature(cpyrt_PyArgs_t args, size_t nargsf) { #else hash += (uint64_t)(Py_REFCNT(pyobj) == 1 ? 1 : 0); #endif - if (CPPOverload_CheckExact(pyobj)) { + if (PyCallable_Check(pyobj)) { static std::hash hash_string; - CPPOverload* overload = (CPPOverload*)pyobj; - hash += hash_string(overload->fMethodInfo->fName); + if (CPPOverload_CheckExact(pyobj)) { + CPPOverload* overload = (CPPOverload*)pyobj; + hash += hash_string(overload->fMethodInfo->fName); + } else { + // This is a python function. Hash based on the function annotation if + // avaiable, otherwise the object itself. + std::string sig = Utility::GetPyCallableAnnotation(pyobj); + if (!sig.empty()) + hash += hash_string(sig); + else + hash += (uint64_t)pyobj; + } } hash += (hash << 10); hash ^= (hash >> 6); diff --git a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx index ed2d5250b617a..7f0798c3e25f0 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx +++ b/bindings/pyroot/cppjit/src/cpyrt/Utility.cxx @@ -423,6 +423,45 @@ static inline std::string AnnotationAsText(PyObject* pyobj) { return PyUnicode_AsUTF8(pyobj); } +std::string cpyrt::Utility::GetPyCallableAnnotation(PyObject* pyobj) { + assert(PyCallable_Check(pyobj)); + + PyObject* annot = PyObject_GetAttr(pyobj, PyStrings::gAnnotations); + if (!annot) { + PyErr_Clear(); + return {}; + } + + if (!PyDict_Check(annot)) { + PyErr_Clear(); + Py_DECREF(annot); + return {}; + } + + PyObject* ret = PyDict_GetItemString(annot, "return"); + if (!ret) { + Py_DECREF(annot); + return {}; + } + + // dict is ordered, with the last value being the return type + std::ostringstream tpn; + tpn << (CPPScope_Check(ret) ? ClassName(ret) : AnnotationAsText(ret)) + << " (*)("; + + PyObject* values = PyDict_Values(annot); + for (Py_ssize_t i = 0; i < (PyList_GET_SIZE(values) - 1); ++i) { + if (i) + tpn << ", "; + PyObject* item = PyList_GET_ITEM(values, i); + tpn << (CPPScope_Check(item) ? full_scope(ClassName(item)) : AnnotationAsText(item)); + } + tpn << ')'; + Py_DECREF(values); + Py_DECREF(annot); + return tpn.str(); +} + bool cpyrt::Utility::AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, cpyrt::Utility::ArgPreference pref, @@ -638,38 +677,11 @@ bool cpyrt::Utility::AddTypeName(std::string& tmpl_name, PyObject* tn, if (arg && PyCallable_Check(arg)) { // annotated/typed Python function - PyObject* annot = PyObject_GetAttr(arg, PyStrings::gAnnotations); - if (annot) { - if (PyDict_Check(annot)) { - PyObject* ret = PyDict_GetItemString(annot, "return"); - if (ret) { - // dict is ordered, with the last value being the return type - std::ostringstream tpn; - tpn << (CPPScope_Check(ret) ? ClassName(ret) : AnnotationAsText(ret)) - << " (*)("; - - PyObject* values = PyDict_Values(annot); - for (Py_ssize_t i = 0; i < (PyList_GET_SIZE(values) - 1); ++i) { - if (i) - tpn << ", "; - PyObject* item = PyList_GET_ITEM(values, i); - tpn << (CPPScope_Check(item) ? full_scope(ClassName(item)) - : AnnotationAsText(item)); - } - Py_DECREF(values); - - tpn << ')'; - tmpl_name.append(tpn.str()); - - Py_DECREF(annot); - return true; - - } else - PyErr_Clear(); - } - Py_DECREF(annot); - } else - PyErr_Clear(); + std::string sig = GetPyCallableAnnotation(arg); + if (!sig.empty()) { + tmpl_name.append(sig); + return true; + } // ctypes function pointer PyObject* argtypes = nullptr; diff --git a/bindings/pyroot/cppjit/src/cpyrt/Utility.h b/bindings/pyroot/cppjit/src/cpyrt/Utility.h index 4858341364484..312ad6f39af67 100644 --- a/bindings/pyroot/cppjit/src/cpyrt/Utility.h +++ b/bindings/pyroot/cppjit/src/cpyrt/Utility.h @@ -37,6 +37,7 @@ PyCallable* FindBinaryOperator( // helper for template classes and methods enum ArgPreference { kNone, kPointer, kReference, kValue }; +std::string GetPyCallableAnnotation(PyObject* pyobj); bool AddTypeName(std::string& tmpl_name, PyObject* tn, PyObject* arg, ArgPreference pref, int* pcnt = nullptr); std::string ConstructTemplateArgs(PyObject* pyname, PyObject* tpArgs, diff --git a/bindings/pyroot/cppjit/test/test_templates.py b/bindings/pyroot/cppjit/test/test_templates.py index eedff095f7079..cbbe8ce2938d9 100644 --- a/bindings/pyroot/cppjit/test/test_templates.py +++ b/bindings/pyroot/cppjit/test/test_templates.py @@ -1466,7 +1466,101 @@ def test39_monkey_patching_template_proxy(self): assert a.m([1, 2, 3]) assert not a.m(42) - def test40_instantiation_failure_error_message(self): + def test40_annotated_callables_select_instantiation(self): + """Annotated Python callables each deduce their own instantiation""" + + import cppjit + + cppjit.cppdef("""\ + #include + namespace AnnotatedCallables { + template + auto call(F f) { return f(2.5); } + + int which(int (*f)(int)) { return 1000 + f(2); } + int which(int (*f)(float)) { return 2000 + f(2.5f); } + + double via_std_function(std::function f) { return f(3); } + template + double via_std_function_t(std::function f, T x) { return f(x); } + + void run(void (*f)(int), int x) { f(x); } + + struct Holder { + Holder(std::function f) : value(f(4)) {} + double value; + }; + double via_holder(const Holder& h) { return h.value; } + }""") + + ns = cppjit.gbl.AnnotatedCallables + + def A(x: int) -> int: + return x * 10 + + def B(x: float) -> int: + return int(x * 2) + + def C(x: float) -> float: + return x * 2.0 + + def D(x): + return x + + class K: + def m(self, x: float) -> float: + return x * 3 + + assert ns.call(A) == 20 + + # an unannotated callable cannot be wrapped, as the signature is not + # avaiable on both C++ nor Python side + with raises(Exception): + ns.call(D) + + # the first instantiation (from A) used to be reused for B, which was + # then called with its argument truncated to int; repeat to also cover + # the memoized dispatch + for _ in range(2): + assert ns.call(A) == 20 + assert ns.call(B) == 5 + assert ns.call(C) == 5.0 + assert ns.call(K().m) == 7.5 + + assert ns.call["int (*)(float)"](B) == 5 + + # non-template overloads likewise go by the annotations + for _ in range(2): + assert ns.which(A) == 1020 + assert ns.which(B) == 2005 + + # std::function takes any callable whose arguments and result convert + def F(x: int) -> float: + return x * 1.5 + + assert ns.via_std_function(F) == 4.5 + assert ns.via_std_function(A) == 30.0 + + # also when selecting among known instantiations, which disallows + # implicit conversions + assert ns.via_std_function_t["int"](F, 2) == 3.0 + assert ns.via_std_function_t(F, 2) == 3.0 + assert ns.via_std_function_t(A, 2) == 20.0 + + # or when converting implicitly through a constructor + assert ns.via_holder(F) == 6.0 + assert ns.via_holder(A) == 40.0 + + # a None annotation declares a void return + seen = [] + + def G(x: int) -> None: + seen.append(x) + + ns.run(G, 7) + assert seen == [7] + + def test41_instantiation_failure_error_message(self): """Rejected instantiation names the template, not garbage""" import cppjit From 142dbc571fd4d85ebb1fc02cc8ac823dff2b181a Mon Sep 17 00:00:00 2001 From: Vipul Cariappa Date: Wed, 14 Oct 2026 16:18:05 +0530 Subject: [PATCH 17/17] [cppinterop] Pass ambiguous_candidates in the TemplatedOperatorArrow test BestOverloadFunctionMatch now takes an out-parameter for ambiguous candidates. Update the ROOT-only FunctionReflection_TemplatedOperatorArrow test to the new signature. --- .../unittests/CppInterOp/FunctionReflectionTest.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp b/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp index bbba8027274ad..c6be579ffab95 100644 --- a/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp +++ b/interpreter/CppInterOp/unittests/CppInterOp/FunctionReflectionTest.cpp @@ -2822,7 +2822,9 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, ASSERT_EQ(ops.size(), 1); EXPECT_TRUE(Cpp::IsTemplatedFunction(ops[0])); - Cpp::FuncRef Deref = Cpp::BestOverloadFunctionMatch(ops, {}, {}); + std::vector ambiguous_candidates; + Cpp::FuncRef Deref = + Cpp::BestOverloadFunctionMatch(ops, {}, {}, ambiguous_candidates); ASSERT_TRUE(Deref); // The match is an instantiation with the defaulted template parameter, not // the template pattern itself: its return type is concrete.