Repository navigation
[WIP] [cppjit] New overload resolution - #23715
Draft
Vipul-Cariappa wants to merge 71 commits into
Draft
Vipul-Cariappa wants to merge 71 commits into
Vipul-Cariappa wants to merge 71 commits into
Conversation
…nOperator` (root-project#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)
…s 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)
(cherry picked from commit 76c198c)
…ist (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)
…Alias() (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: wlav/CPyCppyy#27 (cherry picked from commit edb749b)
…er-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<long> with its std::less<long> and std::allocator<long> 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 <set>\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)
…ler-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#19017 (commit 700fee6), 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 552d291. 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<double>` 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)
(cherry picked from commit c866488)
(cherry picked from commit 0b3dc26)
(cherry picked from commit 3f12220)
(cherry picked from commit a57f54a)
…valent `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)
(cherry picked from commit e24c182)
(cherry picked from commit 6f4b666)
* 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 <regex> 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 <regex> 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)
(cherry picked from commit 425cada)
…test BestOverloadFunctionMatch now takes an out-parameter for ambiguous candidates. Update the ROOT-only FunctionReflection_TemplatedOperatorArrow test to the new signature.
Test Results 21 files 21 suites 3d 6h 26m 22s ⏱️ For more details on these failures, see this check. Results for commit 297d7a4. ♻️ This comment has been updated with latest results. |
`BestOverloadFunctionMatch` returns the declaration that was passed in as a candidate, but `mp_vectorcall` compared it against the *canonical* declaration of each method in the overload. When a function has more than one declaration and the one in the overload is not the first (e.g. ROOT forward-declares functions of autoloaded libraries before the header is parsed), the comparison never matched and the call failed with "Overload Resolution Failed" even though Clang found a unique best match. Compare the canonical declarations on both sides. Assisted-by: claude-code:claude-opus-5-5
`AddTypeName` spells a `LowLevelView` argument with the view's `fCppTypeName`, which was set to the converter name of the element type. For (un)signed char and std::byte views that name is the internal converter alias `UCharAsInt`/`SCharAsInt`, which is not a C++ type, so the overload resolution could never match e.g. `uint8_t* arr[8]` for an array created with `cppjit.ll.array_new["uint8_t*"]`. Record the C++ spelling of the element type instead; the converter name is unchanged. Fixes test_regression.py::test37_array_of_pointers_argument. Assisted-by: claude-code:claude-opus-5-5
`AddTypeName` had no case for ctypes objects and fell back to the Python type's `__name__`, putting non-existent C++ types such as `c_void_p` or `c_int` into the prototype handed to Clang. Overload resolution then only succeeded through the argument-count fallback, and always failed for templates and for overload sets with several candidates of the same arity, e.g. `cppjit.ll.array_delete["void"](ctypes.c_void_p())` or ROOT's `TTree::Branch(name, ctypes.c_void_p(buf), leaflist)`. A ctypes object is a C object owned by Python, so spell it as an lvalue of its C type: `c_int` -> `int&`, `c_void_p` -> `void*&`, `c_char_p` -> `char*&`, `POINTER(c_int)()` -> `int*&`. An lvalue also binds to pointer parameters through the existing reference-to-pointer promotion, which keeps passing e.g. a `c_int` to an `int*` working. Arrays are spelled as a pointer to their elements and `ctypes.byref(obj)` as a pointer to `obj`. Fixes test_lowlevel.py::test17_array_delete_multidim. Assisted-by: claude-code:claude-opus-5-5
…intee A smart pointer proxy behaves as the object it points to: its Python type is the pointee class, so the methods found on it are those of the pointee (the methods of the smart pointer itself are reached through `__smartptr__()`). The implicit object argument in the overload prototype was however spelled as the smart pointer type (e.g. `std::unique_ptr<A>&, int` for `p.get(1)`), so Clang found no viable candidate whenever the method was overloaded, and the call only worked when the argument-count fallback happened to pick a unique method. Add a `smart_as_pointee` option to `Utility::AddTypeName`, which spells a smart pointer proxy as an lvalue of its pointee, and use it for the object of bound calls of normal and templated methods, and for the first argument of unbound calls of non-static methods (e.g. `A.get(p, 1)`), which is the object as well. Fixes e.g. `RDataFrame.Define()` on the `std::unique_ptr<RDataFrame>` returned by `ROOT.RDF.FromNumpy()`, and `RFile.Put()`. Assisted-by: claude-code:claude-opus-5-5
…prototypes numpy scalars and arrays fell back to the Python type's `__name__` in `AddTypeName`, i.e. the non-existent C++ types `float64` or `ndarray`, so overload resolution only succeeded through the argument-count fallback. Overloaded functions such as `TProfile::Fill(Double_t, Double_t)` vs. `Fill(const char*, Double_t)` (hsimple.py) or `TGraph(Int_t, const Double_t*, const Double_t*)` failed. Spell a numpy scalar as a value of the C type of its dtype, and an array as a pointer to its elements, the equivalent of a C array decaying into a pointer (as is already done for `array.array`). This does not allow implicit copies of arrays into e.g. a `std::vector`. numpy is only looked up if it was already imported. Assisted-by: claude-code:claude-opus-5-5
Calling a Python callable with arguments of unsuitable types raises a TypeError, and code using the bindings relies on that, e.g. ROOT's distributed RDataFrame validates its constructor arguments with `except TypeError`, as do several pythonizations that probe overloads. The new OverloadResolutionException (and OverloadAmbiguityException) derived directly from Exception, so these errors escaped such handlers. Make it a subclass of TypeError; code that catches the specific exception is unaffected. Assisted-by: claude-code:claude-opus-5-5
`AddTypeName` appended nothing for an empty Python list or tuple, so the
overload prototype became malformed (e.g. `std::string, ` for
`RDataFrame("tree", [])`) and the overload resolution failed outright.
Spell it as `std::initializer_list<__cppjit_internal::UnknownType>`: like
for a non-empty list, the overload resolution promotes this to the
`std::initializer_list<T>` parameter of a candidate.
Assisted-by: claude-code:claude-opus-5-5
`TemplateProxy.__overload__(signature)` resolved the signature through `GetMethodTemplate` without the object argument, although the candidates of a class are non-static methods that take it first. Clang therefore used the first parameter type as the object (crashing in Sema for a non-class type), only templates were considered, and when nothing was found NULL was returned without setting an exception. Add `interop::GetMethodBySignature`, which resolves the non-static methods of the set with an object of the class prepended to the signature, and static methods and functions without it (so that no candidate gets an argument of the wrong kind as its object), sharing the candidate lookup and instantiation with `GetMethodTemplate`. Use it in `__overload__`, including the non-templated overloads in the selection by signature, and raise a LookupError if no overload matches. Assisted-by: claude-code:claude-opus-5-5
When the overload resolution selects a function taking `T&&` for a proxy
of a class U that converts to T (e.g. `std::vector<Variant>::push_back`
given an `Axis`, with `Variant(const Axis&)`), C++ initializes the
parameter with a temporary T. `InstanceMoveConverter` instead treated the
U proxy as the object to move and rejected it as not being an rvalue.
Go through the implicit conversion (which creates the movable temporary)
when the proxy is not of class T or a class derived from it. Smart
pointers are still matched by the reference converter.
Fixes e.g. `RDataFrame.Hist([axis1, axis2], ...)` and
`RNTupleWriter.Recreate([("x", "float")], ...)`. It also makes
`boost::any::operator=` work for an lvalue (test_boost.py test02): the
forwarding-reference `operator=(ValueType&&)` still fails to instantiate
for it, but the move assignment `operator=(any&&)` now takes a temporary
`any` converted from it, as C++ would.
Assisted-by: claude-code:claude-opus-5-5
Data members of enum type (static or not, e.g. `std::ios_base::ate`) were
converted with the converter of the underlying integer type and thus
returned as plain Python integers, while enum constants are instances of
the Python enum class. Passed on to a function, a plain integer is spelled
as `int` in the overload prototype and doesn't convert to the enum, so
e.g. `std.ostringstream("(", std.ios_base.ate)` failed (and only worked
in other environments through the argument-count fallback).
Keep the enum type for data members, so that they get an EnumConverter,
and let its FromMemory() return an instance of the enum class, which is
looked up once in the enclosing scope.
Fixes test_templates.py::test04_variadic_function in ROOT.
Assisted-by: claude-code:claude-opus-5-5
…matches When the overload resolution of a TemplateProxy found no candidate for the arguments, it reported "Failed to instantiate" as a generic TypeError, whereas a CPPOverload raises an OverloadResolutionException in the same situation. Whether a method set goes through a TemplateProxy depends on the presence of templated overloads, e.g. the `std::vector` constructors of the standard library in use, so the exception type differed between environments (test_stltypes.py::test16_vector_construction failed with ROOT's C++20 standard library). Raise an OverloadResolutionException (a TypeError) instead. Assisted-by: claude-code:claude-opus-5-5
…structors cppjit can't know the C++ type of a Python callable without annotations and spells it as `std::nullptr_t` in the overload prototype. The `template <class Func> TF1(const char*, Func, ...)` constructor then deduces `Func = std::nullptr_t`, wins the overload resolution, and fails to convert the callable (see the corresponding cppjit issue). Wrap an unannotated Python callable in a function annotated with the signature ROOT expects, `double (*)(double*, double*)`, which selects the function-pointer constructor like a real function pointer does in C++. Assisted-by: claude-code:claude-opus-5-5
The test relied on an implementation detail of the old cppyy overload
dispatch, which tried every constructor and wrapped the C++ exception of
the failed attempts in a generic TypeError. With the Clang-driven overload
resolution, `RDataFrame("events", [])` selects the
`std::initializer_list<std::string>` constructor directly, so the actual
C++ exception surfaces, like it already did for an explicit
`std::vector<std::string>`.
Assisted-by: claude-code:claude-opus-5-5
The old bindings implicitly converted any iterable into a
`std::vector<std::string>`, so e.g.
`RDataFrame("tree", (path + f for f in files))` worked (it is used in the
df103 and df104 tutorials). The Clang-driven overload resolution of
cppjit has no C++ type for a generator.
Turn iterators among the arguments of the pythonized RDataFrame
constructor into lists, which match the `std::initializer_list` and
`std::vector` constructors.
Assisted-by: claude-code:claude-opus-5-5
`ROOT.RDF.RunGraphs([h1, h2])` relied on the old bindings converting a Python list into the `std::vector<RResultHandle>` parameter, converting each RResultPtr on the way. The Clang-driven overload resolution of cppjit doesn't treat a Python list as a braced-init-list (see the corresponding cppjit issue), so the conversion fails. Build the vector of RResultHandles explicitly in the RunGraphs pythonization. Assisted-by: claude-code:claude-opus-5-5
`MakeCSVDataFrame(..., colTypes={"col": "T"})` assigned the Python dict
to the `std::unordered_map<std::string, char>` data member of the RCsvDS
options, relying on the old bindings converting a dict into a map. The
Clang-driven overload resolution of cppjit doesn't model a dict as a
braced list of key/value pairs (see the corresponding cppjit issue), so
no `operator=` matches anymore.
Insert the key/value pairs one by one instead.
Assisted-by: claude-code:claude-opus-5-5
For objects not deriving from TObject, the WriteObject() pythonization calls `WriteObjectAny(const void*, const char*, ...)` with the proxy of the object. With the Clang-driven overload resolution of cppjit a proxy no longer converts to a `void*` parameter, just like an object doesn't in C++, so the call failed to choose between the `const char* classname` and `const TClass*` overloads. Pass the object's address cast to `void*`. Assisted-by: claude-code:claude-opus-5-5
For numpy and array.array buffers the SetBranchAddress() pythonization re-implements the template overload by calling `SetBranchAddress(const char*, void*, TBranch**, TClass*, EDataType, bool)` with keyword arguments. With the Clang-driven overload resolution of cppjit, the buffer is spelled as a typed pointer (e.g. `double*`), so the `template <class T> SetBranchAddress(const char*, T*, TBranch**)` overload was selected; keyword arguments are moreover not considered when resolving templated methods (see the corresponding cppjit issue). Pass the buffer address as a `void*` and all arguments positionally. Assisted-by: claude-code:claude-opus-5-5
`tree.Branch("folder", 0)` fails with the Clang-driven overload resolution
of cppjit: the argument promotions make it consider the
`template <class T> Branch(const char*, T*, ...)` overloads as well and
report an ambiguity, while in C++ an int can't deduce a `T*` and the call
is unique (see the corresponding cppjit issue).
Select the folder overload explicitly in the Branch() pythonization for a
string followed by one or two integers.
Assisted-by: claude-code:claude-opus-5-5
…eir pointer The Branch() pythonization hands an object proxy to the `BranchPtrToPtr<T>(TTree&, const char*, const char*, T**, ...)` helper and relied on the bindings converting the proxy into the address of the pointer it holds. With the Clang-driven overload resolution of cppjit, a proxy spelled as `T&` doesn't match a `T**` parameter (it doesn't in C++ either); only a reference proxy, spelled as a `T*`, is still promoted to `T**`. Keep passing the proxy, which handles reference proxies to the caller's pointer, and fall back to passing the address of the pointer held by the proxy explicitly as a `T**`. Assisted-by: claude-code:claude-opus-5-5
A Python list is not treated like a C++ braced-init-list by the Clang-driven overload resolution of cppjit (see the corresponding cppjit issue). If a RooFit class has several constructors with the same number of parameters, e.g. `RooAddPdf(name, title, [pdf1, pdf2], [frac], True)`, `RooHistPdf`, `RooNDKeysPdf`, `RooRealSumPdf` or `RooMultiVarGaussian`, the constructor taking RooFit collections is therefore not found. Wrap the constructors of the RooFit classes that are not pythonized otherwise to convert lists and tuples of RooAbsArgs to RooArgLists and sets to RooArgSets beforehand, like it is already done for the functions with keyword argument pythonizations. Assisted-by: claude-code:claude-opus-5-5
A tuple given as keyword argument value, e.g. `pdf.plotOn(frame, VisualizeError=(fitres, [par1, par2], 2))`, is unpacked into the arguments of the RooCmdArg function. A list of RooFit objects in there didn't convert to the `const RooArgSet&` parameter with the Clang-driven overload resolution of cppjit (see the corresponding cppjit issue), so convert it beforehand like the other positional arguments. Assisted-by: claude-code:claude-opus-5-5
…ject Adding a legend entry without an object, e.g. a header line, is commonly written as `legend.AddEntry(0, "label", "")` in Python. With the Clang-driven overload resolution of cppjit, a Python integer no longer converts to a pointer, and a null pointer is ambiguous between the `AddEntry(const TObject*, ...)` and `AddEntry(const char*, ...)` overloads, just like in C++. Add a TLegend pythonization that selects the TObject overload with a null pointer when the object is given as 0, None or ROOT.nullptr. Assisted-by: claude-code:claude-opus-5-5
The test relied on an implementation detail of the old cppyy overload dispatch, which tried every overload of `MergeValues` and wrapped the C++ exceptions of the failed attempts in a generic TypeError. With the Clang-driven overload resolution of cppjit the single best overload is called, so the actual `std::invalid_argument` thrown for results of different actions surfaces. Assisted-by: claude-code:claude-opus-5-5
`myfunc(C(), Z())` is ambiguous in C++, and the test documented that the
old bindings silently called the overload with the highest priority
instead ("Would be nice to throw an error in this kind of scenario").
The Clang-driven overload resolution of cppjit reports the ambiguity like
C++ does (as an OverloadAmbiguityException, a TypeError), so test for it.
Assisted-by: claude-code:claude-opus-5-5
`TColor.GetColor(0.5, 0.2, 0.1)` is ambiguous with the Clang-driven overload resolution of cppjit, as it is in C++: a Python float is a double, which converts equally well to the Float_t and the Int_t components of the two GetColor() overloads. The old bindings picked the Float_t overload; this is used e.g. in the df105 and df107 tutorials. Pythonize GetColor() to select the Float_t overload explicitly if any of the color components is a Python float. Assisted-by: claude-code:claude-opus-5-5
…thonizations
When a C++ callable is given to the Define() or Filter() pythonizations,
the Python list of column names (empty by default) is passed on to the
templated `RInterface::Define(std::string_view, F, const ColumnNames_t&)`.
The Clang-driven overload resolution of cppjit doesn't treat a Python list
like a C++ braced-init-list (see the corresponding cppjit issue), so e.g.
`df.Define("x", ROOT.some_function)` failed to resolve (df042 tutorial).
Convert the list of column names to a `std::vector<std::string>`
explicitly.
Assisted-by: claude-code:claude-opus-5-5
…in df101.py The Python version of the tutorial builds the fit functions from formula strings, but passed the number of parameters as fifth argument like the C++ version does for its function pointers. For the formula constructor the fifth parameter is the `TF1::EAddToList` enum class though, and only the lenient int-to-enum conversion of the old bindings let this through. With the Clang-driven overload resolution of cppjit (as in C++) the call fails. Drop the argument: for a formula, the number of parameters is deduced from the formula itself. Assisted-by: claude-code:claude-opus-5-5
`w.Import([pdf, cat1, cat2])` relied on the old bindings converting the list to the `const RooArgSet&` of `RooWorkspace::import()`, as a braced list does in C++. With the Clang-driven overload resolution of cppjit the list is converted to a RooArgList beforehand (see the RooFit collection pythonizations), for which the derived-to-base conversion to `import(const TObject&)` is the better match, so the list itself was imported as a generic object. Convert a sequence given as first argument to a RooArgSet explicitly. Assisted-by: claude-code:claude-opus-5-5
`RVec["int"](f(x) for x in v)` relied on the old bindings converting any iterable into the container. The Clang-driven overload resolution of cppjit has no C++ type for a generator, and cppjit only pythonizes the std::vector constructor for that. Turn an iterator given to the pythonized RVec and std::vector constructors into a list first. Assisted-by: claude-code:claude-opus-5-5
…on sequences `RHist["int"]([axis1, axis2])` and `RHist["int"](nbins, (low, high))` relied on the old bindings converting Python sequences to the `std::vector<RAxisVariant>` and `std::pair<double, double>` parameters. With the Clang-driven overload resolution of cppjit, the list selects the `std::initializer_list<RAxisVariant>` constructor (as in C++), for which cppjit can't build a temporary initializer list since RAxisVariant isn't default constructible, and the tuple doesn't convert to a std::pair (see the corresponding cppjit issue about lists vs. braced-init-lists). Add constructor pythonizations for RHist and RHistEngine that convert a list of axes to a std::vector<RAxisVariant> and an interval tuple to a std::pair. Assisted-by: claude-code:claude-opus-5-5
…Fit.py `fitter.FitFCN(functor, 0, size, True)` relied on the old bindings accepting the Python integer 0 as the `const double* params` null pointer. With the Clang-driven overload resolution of cppjit a Python integer is an `int` value, not a null pointer constant like the literal 0 in C++ (see the corresponding cppjit issue), so no FitFCN overload matched. Say what is meant and pass `ROOT.nullptr`. Assisted-by: claude-code:claude-opus-5-5
… them
`merge_values` passed the `std::unique_ptr<RMergeableValue<T>>` proxies to
`MergeValues`. With the Clang-driven overload resolution of cppjit, an
lvalue unique_ptr selects the `MergeValues(std::unique_ptr<...>, ...)`
overload taking it by value (as it would in C++, where the call then
fails on the deleted copy constructor), and cppjit moved the mergeable out
of the Python proxy, so the merged value was lost ("no C++ object
available").
Pass the objects the smart pointers point to, which selects the overload
merging into the output in place, as intended.
Assisted-by: claude-code:claude-opus-5-5
…delConfig setters `data.table([cat])`, `hist.get([x, y])` and `modelConfig.SetParametersOfInterest([poi])` (and the other ModelConfig setters) relied on the old bindings converting a Python list into the `const RooArgSet&` parameter. These methods have other overloads of the same arity (taking a `const RooAbsCategory&`, an index, or a `const char*` list of names), so with the Clang-driven overload resolution of cppjit, which doesn't treat a list like a braced-init-list (see the corresponding cppjit issue), they don't resolve anymore. Pythonize them to convert lists and tuples of RooFit objects to a RooArgList and sets to a RooArgSet first. Assisted-by: claude-code:claude-opus-5-5
TText has `const char*` and `const wchar_t*` overloads of its constructor and of DrawText(), DrawTextNDC(), SetText(), PaintText() and PaintTextNDC(). With the Clang-driven overload resolution of cppjit, a Python string is promoted to both pointer types and the call is reported as ambiguous, while in C++ a narrow string literal selects the `const char*` overload (see the corresponding cppjit issue). This broke e.g. `ROOT.TText(2, 100, "Signal")` in the rf106 tutorial. Pythonize these methods to select the `const char*` overload explicitly for a Python string. Assisted-by: claude-code:claude-opus-5-5
…explicitly `std.make_any["Derived1*", "Derived1*"](obj)` passes an object where the `Derived1*&&` parameter of that instantiation needs a pointer, which is ill-formed in C++ (`make_any<Derived1*, Derived1*>(obj)`), and only worked with the lenient old bindings. With the Clang-driven overload resolution of cppjit a proxy is spelled as the object, and there is no Python expression that is spelled as a pointer to it (see the corresponding cppjit issue). Select the instantiation by its signature instead, which passes the object's address like `make_any<Derived1*>(&obj)` in C++. Assisted-by: claude-code:claude-opus-5-5
…for other histograms
`df.HistoND(("name", "title", dim, nbins, xmin, xmax), cols)` (and the same
for HistoNSparseD) relied on the old bindings converting the tuple into
the `const THnDModel&` / `const THnSparseDModel&` parameter. The
Clang-driven overload resolution of cppjit spells a tuple as an
`std::initializer_list` of its first element's type rather than a
braced-init-list (see the corresponding cppjit issue), so it no longer
resolves. Add both methods to those whose tuple model argument is turned
into a model object, like Histo1D/2D/3D and Profile1D/2D.
Assisted-by: claude-code:claude-opus-5-5
…Entry `CreateEntry()` returns a `std::unique_ptr<REntry>`, and the pythonized `RNTupleWriter.Fill(entry)` and `RNTupleReader.LoadEntry(index, entry)` passed it on to the C++ methods taking `REntry&`. As in C++, a smart pointer doesn't bind to a reference to the object it points to, so with the new overload resolution `Fill` found no viable overload, and `LoadEntry` only worked through the argument-count fallback. Pass the entry the smart pointer points to (`__smartptr__().get()`). Assisted-by: claude-code:claude-opus-5-5
`vec += buf` with a Python buffer (array.array, numpy array) went through cppjit's fast `__iadd__`, which called `insert(end(), buf)`. No `vector::insert` overload takes a buffer; the old bindings converted it into an initializer_list, which cppjit deliberately doesn't do (it would be a hidden deep copy), so `+=` failed. Appending with `+=` is explicit though, so pythonize `std::vector.__iadd__` to copy the elements of a Python buffer, reusing the copy from the construction of a std::vector or RVec from a numpy array (now appending, with an element-wise fallback for vectors like std::vector<char> whose `data()` has no address). Assisted-by: claude-code:claude-opus-5-5
Contributor
|
Thanks a lot @Vipul-Cariappa for opening the PR! That's really a great help to see where we stand. Given that you needed tons of new Pythonizations to make ROOT interfaces behave as expected, I think getting this into ROOT 6.42 is unfortunately very risky. The ROOT Python interface powered by cppyy (now cppjit) is used by tons of user and experiment code for their own C++ libraries, and just as ROOT has to paper over the behavior changes in cppjit with Python-side Pythonizations, so would our users. We can't expect this from them at this point, and I'd argue even less so in a release that already risks disrupting user code with the migration from cppyy and cppjit. My conclusion from this excellent R & D PR is therefore that the new overload resolution is something we should target as a 6.44 highlight, and not as a last-minute inclusion to 6.42 before branching, and then releasing in November. That would nicely factor the internal move to cppjit from user-facing behavior changes that follow up, which would also make our life easier when fixing regressions. |
Member
I agree -- the code changes are more than I was hoping for. @Vipul-Cariappa, can we make sure we get the this PR to a green state while we are at it? |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The new overload resolution matches more closely to the overload resolution in C++. This should resolve multiple open ROOT issues.