Cppyy interop migration (draft PR for CI)#22810
Draft
guitargeek wants to merge 76 commits into
Draft
Conversation
guitargeek
force-pushed
the
cppyy-interop-migration_jonas
branch
from
519ffb7 to
4ac6cd0
Compare
guitargeek
force-pushed
the
cppyy-interop-migration_jonas
branch
2 times, most recently
from
217c0d5 to
2dc76e4
Compare
Test Results 21 files 21 suites 3d 0h 56m 7s ⏱️ For more details on these failures, see this check. Results for commit e7c31ef. ♻️ This comment has been updated with latest results. |
guitargeek
force-pushed
the
cppyy-interop-migration_jonas
branch
from
24580cc to
e8d7c4c
Compare
…stream] Adapts upstream CppInterOp for use with ROOT & Cling: - Add SynthesizingCodeRAII guard for scope/class reflection - Call buildLookup in lookup paths, to populate Cling's lazy lookup tables - Drop class_name:: scope qualification in make_narg_call so virtual dispatch works for pure-virtual methods (TInterpreter::Declare) - Export CppGetProcAddress from libCling's linker script for dispatch mechanism - GetTypeAsString: revert the PrintingPolicy ctor to PrintingPolicy((LangOptions())) instead of deriving it from the ASTContext. Using default LangOptions restores the string form (std::basic_string<char>) that the CPyCppyy factory expects
Source: compres forks cppyy-backend/master 597bcf4 Drops ROOT's old clingwrapper in favour of the CppInterOp-based implementation from the compres fork. Replaces gInterpreter/ROOT-meta TCling API calls with CppInterOp via the dispatch mechanism (cppinterop_dispatch.cxx to load API) Adds recursive_mutex thread safety, switches execution to JitCall. To be followed with ROOT-specific patches.
…patch] CPPINTEROP_DIR is the include-root that contains the CppInterOp/ subdir with Dispatch.h and the tablegen-generated CppInterOpAPI.inc. Pass it straight to Cpp::AddIncludePath instead of concatenating "/include", matching where the build system actually places those headers.
Based on ROOT: - Load libCling via gSystem->DynamicPathName instead of the fork's CPPINTEROP_DIR/lib path - include ROOT core headers and call TClass::GetClass in GetScope for dictionary/module autoloading. - Guard GetActualClass and GetBaseOffset with Cpp::IsComplete to handle incomplete types (e.g. TCling with no public header) - Initialise ROOT globals and required dylibs in ApplicationStarter - Move precommondefs.h include into cpp_cppyy.h; use Cpp::TCppFuncAddr_t for proper alignment - Update CMakeLists for ROOT build-tree integration
Dispatch.h transitively includes the tablegen-generated CppInterOpAPI.inc,
which lives in the build tree, not the source tree. ROOT stages CppInterOp's
public headers + .inc files into ${BINARY_DIR}/etc/cppinterop/CppInterOp/
via the CppInterOpEtc custom target. Point cppyy_backend's include dir
and the runtime CPPINTEROP_DIR macro at that staging dir, and depend on
CppInterOpEtc so staging completes before this library compiles.
Source: compres forks cppyy/master 273ed88 - Calls gCling.EnableAutoLoading() - Adds cppyy.evaluate / cppyy.macro() / cppyy.ll.as_memoryview - Numba pointer/reference support - Metaclass naming fix, isinstance() idiom cleanup - Bug fixes in basic_string / span / npos pythonizations - Fallback decoding paths for non-UTF-8 compiler output To be followed with ROOT-specific patches.
… [upstream] Same as in Pythonize.cxx basic_string name from GetQualifiedCompleteName so the NPOS object is registered for the canonical class name shapes.
ROOT-specific patches on top of the compres fork cppyy/master baseline: __init__.py: - set gInterpereter to gbl.TInterpreter.Instance() - load_library(): use gSystem.Load/FindDynamicLibrary, add Windows winmode=0 for search path - add_library_path(): use gSystem.AddDynamicPath - Drop the CPPYY_API_PATH apipath_extra dispatcher-headers (ROOT installs CPyCppyy API headers) _cpython_cppyy.py: - Wrap `from cppyy_backend import loader` in try/except, fall back to c = None (ROOT does not ship cppyy_backend.so) - Platform: `import libcppyy` vs `import cppyy.libcppyy` (standalone cppyy uses the former; ROOT exposes it as cppyy.libcppyy) - load_reflection_info(): use gSystem.Load ROOT/_facade.py: - Store gInterpreter and gPad in __dict__
Source: compres forks CPyCppyy/master 482ccb7 Brings in fork-ahead developments missing from ROOT's old copy and compatibility with the CppInterOp based backend: - CppInterOp-aligned type system (Cppyy.h) - Type-based CreateConverter / CreateExecutor factory overloads - Instance_FromVoidPtr(scope) overload - PythonGILRAII for exception-safe GIL management - PyError_t RAII rewrite (unique_ptr-based) - GetTemplateArgsTypes for type-based template instantiation - ReduceReturnType/LambdaClass handling - Rvalue forwarding in Dispatcher - GetFailureMsg in Converters - STL string executor returns bound C++ object ROOT-specific CPyCppyyModule.h and CPyCppyyPyModule.cxx are preserved from master.
…tream] Source: ROOT master - Move to using SetGlobalPolicy(ECallFlags, bool) + GlobalPolicyFlags() - Drop the CallContext* argument from UseStrictOwnership() - Guard PyMapping_GetOptionalItemString for Python>3.13 in Dispatcher.cxx
…stream] Source: master
Source: master - Deprecate __mempolicy__ getter/setter with a clear error pointing users to the SetOwnership() pythonization - Drop the per-method memory policy from mp_call (referenced the removed sMemoryPolicy static) - Use GlobalPolicyFlags() for the Clone heuristic - Match the method name prefix before '<' only, so template arguments don't affect Clone detection
…tream] Source: ROOT-master 760b6cc - ToMemory override that lets the array converter copy Python strings (not only buffers) into C++ const char* arrays - ToArrayFromBuffer<> template helper that copies a buffer to an array-converter memory address, lifetime management via SetLifeLine
…eam] Source: ROOT-master - DEFINE_CALL_POLICY_TOGGLE macro generates SetHeuristicMemoryPolicy, SetImplicitSmartPointerConversion, and SetGlobalSignalPolicy used by CallContext::SetGlobalPolicy() - Drop the obsolete kMemoryHeuristics/kMemoryStrict module-level labels (no longer referenced the removed CallContext::kUseStrict) - Add missing guard for Py_INCREF(gThisModule)
… [upstream] Source: ROOT-master - VectorIAdd returns self after the insert. Python += reassigns the lhs to the returned value, so returning the inserted-iterator result silently breaks the idiomatic += pattern on std::vector. Disable by wrapping VectorArray in #if 0 and removes its Utility::AddToClass call (to be fixed at a later stage) - Drop Cppyy::gGlobalScope extern, every caller uses Cppyy::GetGlobalScope()
… args [upstream] GetQualifiedCompleteName returns the canonical class name with default template args spelled out
pyval_from_enum returned the Py_True/Py_False singletons for enums with a bool underlying type, so the values were not instances of the enum's Python class (breaking e.g. enum.Enum construction from the class __dict__) and could not carry __cpp_name__. Construct proper (int-based) instances like every other underlying type; keep the singleton shortcut only when there is no enum class to instantiate (data-member path). Fixes half of compiler-research/cppyy#198. Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)
… sets Deleted functions are not callable and must not enter overload sets: a deleted overload adds a bogus conversion error to every failed-call report, and in particular breaks Utility::SetDetailedException's rule that a set of failures consisting of C++ exceptions of a single type is re-raised as that type (a deleted copy constructor turned the expected ConfigFileNotFoundError of test10_overload_and_exceptions into a generic TypeError). Fixes the test10_overload_and_exceptions case of cppyy-test-overloads.
…e [upstream] When the name lookup was ambiguous (an overload set), the FIXME path blindly returned true, so any function with two or more non-template overloads was presented as a function template. In cppyy this wrapped plain overloaded functions in a TemplateProxy, whose error aggregation does not preserve the C++-exception priority of CPPOverload (e.g. a std::runtime_error thrown by an overload surfaced as a generic "Template method resolution failed" TypeError). Implement the FIXME: cycle through the found decls (following using declarations) and check whether any is actually a templated function. Fixes the test49_overloads_with_runtime_errors case of cppyy-test-regression (root-project#17497 regression).
cppyy-backend's TClingCallFunc::compile_wrapper hardcodes
withAccessControl=false for every wrapper: the callee was already
selected (and public-filtered) by the caller, and compiling the call
may lazily instantiate template bodies that are only valid with checks
relaxed, e.g. a member template accessing a private member of another
specialization of its own class template:
template <class V> class CustomVec {
V fX;
public:
template <class fV> CustomVec operator+(const fV& v)
{ ... fX + v.fX ... } // v.fX private when fV != CustomVec<V>
};
Fixes the test19_templated_operator_add case of cppyy-test-templates.
TypeManip::compound() strips const, so both T*& and T* const& arrive at the converter factory with compound "*&" and were both rejected by the ban on non-const pointer references. Through a T* const& the callee cannot rebind the pointer, which is the ban's sole rationale; check for a const immediately preceding the '&' and let those through to the regular instance-pointer converters.
AppendTypesSlow resolved template-argument expressions by declaring a
trampoline in the global scope, optionally retrying with the name
qualified by the parent. Neither form emulates real C++ name lookup: a
name written relative to a namespace must walk outward with proper
shadowing of outer names, e.g. with
namespace X { class C; }
namespace Y { namespace Z { template<class T> void f(); }
namespace X {} }
Y::Z::f["X::C"] must fail (Y::X shadows ::X and has no C), yet resolved
via the global ::X::C. Reopen the parent namespace (C++17 nested form)
around the trampoline declaration instead, for both the full type
expression and the per-chunk __typeof__ fallback, so the compiler
itself applies the lookup rules.
Fixes the test06_type_deduction_and_scoping case of
cppyy-test-templates.
The string-based ConstructTemplateArgs raised SyntaxError while the type-based variant and ConstructCppName raise TypeError for exactly the same failure. TypeError is also what the documented behavior (and test35_no_possible_cpp_name) expects for a Python object that does not map to a C++ type.
…tream] GetClassDecls listed the base's copy constructor for a class with an inheriting-constructor using declaration, e.g. UsingDerived1(const UsingBase1&) for `using UsingBase1::UsingBase1`. Callers such as cppyy treat the class' own special members as authoritative, and the call layer refuses to invoke special members injected by a using declaration (see make_narg_call_with_return), so these only polluted overload sets and docstrings. Skip parameterless default constructors and copy/move constructors of the base; constructors with defaulted parameters, e.g. Base(int n = 0), remain genuinely inherited and stay. Fixes the test23_using case of cppyy-test-advancedcpp; adapts the GetClassMethods reflection test accordingly.
… [upstream] GetVariableType returned the declared type of the decl it was handed, so `extern std::string arr[];` came back as an incomplete array: no dimension (len() failed) and no element stride (every index read the first element). Prefer the defining declaration's complete type for arrays of unknown bound. Fixes the test04_array_of_strings case of cppyy-test-stltypes.
…ream] Inline constexpr variables such as std::nullopt have no symbol in any loaded library and, when the deferred-decl emission does not fire, GetVariableOffset failed with "Symbols not found". Add a last-resort fallback that ODR-uses the variable in an interpreter-evaluated expression (&::std::nullopt;), forcing CodeGen to emit it - mirroring what TClingDataMemberInfo::Offset does in ROOT. The function-level PushTransactionRAII is narrowed to the AST-manipulating sections, since execution inside a nested transaction is deferred and the evaluation (and final symbol lookup) must run outside of it. Fixes the test10_optional case of cppyy-test-cpp11features (std::nullopt lookup).
VectorArray was disabled wholesale (2c06084) because it was buggy for multi-dimensional vectors: data() on a vector of class instances hands back a proxy of the first element, so forwarding __array__ to it yielded that element instead of the full vector (np.array of a vector<vector<int>> of two elements returned the first inner vector). Re-enable it, but only for vectors of non-class value types; numpy's generic sequence protocol handles instance-element vectors correctly on its own (and now yields the proper (2, 3) shape for the case above). Fixes the test18_array_interface case of cppyy-test-stltypes (zero-copy np.array(v, copy=False) works again).
Names produced by the backend (proxy __name__/__cpp_name__, docstring
signatures) used clang's printer style: "a, b" in template argument
lists, "T *"/"T &", and literal suffixes on non-type arguments
("array<unsigned int, 3UL>"). CPyCppyy itself constructs names in the
upstream cppyy convention - no space after commas, pointers and
references attached, no suffixes - e.g. when looking up cached template
instantiations by name. The mismatch made e.g.
gbl.__dict__['multiply<int,int,int>'] fail after mul(1, 2) cached the
instantiation under 'multiply<int, int, int>', and setattr-based
overriding of instantiations impossible.
Normalize comma spacing and pointer/reference attachment in
GetFinalName, GetScopedFinalName, GetFullName and the per-argument
types of GetMethodSignature, and print non-type template arguments
without literal suffixes. Name-matching sites that had been adapted to
the clang spellings (std::string/std::string_view pythonizations, the
pretty-printing exclusion, the NPOS pythonization) recognize the
canonical spelling as well, and the rdataframe_asnumpy expectation
returns to the ROOT-master form "array<unsigned int,3>".
Fixes the test_doc_strings and test09_templated_function cases of
cppyy-test-doc-features, the test01_using case of cppyy-test-templates
and the npos handling in test09_string_as_str_bytes of
cppyy-test-stltypes.
…Match [upstream] When the best viable function is a not-yet-instantiated template specialization, force the instantiation of its body with diagnostics suppressed and reject the match if the body turns out ill-formed. Callers use this function to probe whether a valid instantiation exists (e.g. CPyCppyy's __cppyy_internal::is_equal fallback for comparison operators), so an ill-formed body must surface as "no viable function" rather than as a stream of hard errors when the wrapper is compiled at call time. This mirrors cling's LookupHelper::overloadFunctionSelector. Note that probing with an expression-SFINAE helper instead is not equivalent: C++20 rewritten (reversed) operator candidates make comparisons like a member operator==(const Base&) ambiguous, which is a hard substitution failure in a SFINAE context but only a warning in a function body. InstantiateTemplate equally refuses to hand out a specialization whose attempted instantiation marked it invalid: any use of it, such as compiling a call wrapper, can only fail. With the error spam gone, test21_access_to_global_variables of cppyy-test-advancedcpp passes and loses its xfail marker.
gROOT is a macro expanding to ROOT::GetROOT(); ROOT master exposed it to cppyy as a TGlobalMappedFunction, a mechanism this backend does not consult, so cppyy.gbl.gROOT raised AttributeError (which, used in xfail conditions, failed collection of the entire cppyy-test-fragile and similar suites). Resolve the name to ROOT::Internal::gROOTLocal, making sure ROOT is initialized first.
Any class with a size() method was given __len__, so len() failed on valid objects whose size() returns a custom (e.g. dimension) object, and bool(obj) went through a broken __len__. Install __len__ only when size() returns an integral value and the class actually models a container: iterable (begin/end) or indexable through a real C++ operator[] (every class "has" __getitem__ via the CPPInstance defaults, so an actual overload is required). The size method and the container interface may live on a base class. Fixes the test11_size_len_pythonization_guards case of cppyy-test-pythonization.
…pstream] CXXRecordDecl::isAbstract() reads the definition data, which a forward-declared class does not have, so asking whether an incomplete class is abstract crashed. Answer false instead (not known to be abstract), reading through the definition when there is one.
Re-apply e93464a (fixes root-project#16062), which was lost in the fork-baseline replacement of Converters.cxx: pointers read from class fields or variables bind as their actual polymorphic class, so e.g. iterating a std::map<int, Base*> yields Derived proxies. One refinement over the original: writable pointer variables whose declared class is abstract stay at the declared class - those proxies track the pointer itself, whose pointee can be replaced from Python (cf. test21_access_to_global_variables, which pins an abstract-typed global while test51_polymorphic_types_in_maps expects the downcast). Fixes the test51_polymorphic_types_in_maps case of cppyy-test-datatypes.
… scope Explicit template arguments like the "RVecF" in df.Take["RVecF"]() were resolved by qualified lookup in the method's class scope and then the global scope only. Names living in a namespace enclosing the class -- e.g. ROOT::RVecF and ROOT::RVec seen from within ROOT::RDF::RInterface<...> -- were missed, so the instantiation failed where real C++ unqualified lookup would succeed. Generalize the namespace-reopening trampoline in AppendTypesSlow: for class parents, walk out to the innermost reopenable enclosing namespace and declare the trampoline inside it, letting the compiler apply real unqualified lookup with proper shadowing. The single-identifier fast path now falls through to the trampoline instead of giving up, and the class-qualified candidate is kept for types nested in the class itself. Fixes the pyroot-numbadeclare test.
…cppyy With the CppInterOp-based cppyy backend, instantiating a function template with the shortened class name AtlasLikeDataVector<CustomStruct> no longer fails: resolving the name autoloads the class dictionary, after which the compiler sees the default second template argument and resolves the correct specialization. The old test pinned the previous backend's limitation, where the first instantiation attempt had to raise and the caller had to retry with a TClass alternative name. Update the test to assert the new behaviour, and strengthen it: foo now returns the demangled typeid name so the test checks that the shortened name really instantiates AtlasLikeDataVector<CustomStruct, DataModel_detail::NoBase>, not merely that the call does not throw. The autoload side effect of registering the alternative class names is still covered, since the REntry pythonization keeps relying on it as a fallback (_try_getptr in _rntuple.py).
Since the cppyy backend is linked statically into libCPyCppyy, the dllimport declarations broke the Windows link: the MSVC linker resolves the __imp_-prefixed references generated by dllimport only against import libraries, never against members of a static library. Because every reference to the backend went through __imp_, no plain-name reference ever pulled the backend objects out of cppyy_backend.lib, leaving all Cppyy functions unresolved when linking libCPyCppyy.dll. Declare the API with plain extern instead. Inside libCPyCppyy the references now resolve directly against the static backend library, and since the definitions remain dllexport (RPY_EXPORTED in precommondefs.h) they are re-exported from libCPyCppyy.dll, so any downstream consumer still resolves them through the import library's function thunks. All CPPYY_IMPORT declarations are functions, and the macro was already plain extern on non-Windows platforms, so nothing changes elsewhere.
When auto-instantiating a templated method from Python argument types,
a Python tuple is typed as std::initializer_list<T>. An initializer_list
can not implicitly convert to std::pair, so calls like
df.Hist(10, (5.0, 15.0), "x")
failed to resolve Hist(std::uint64_t, std::pair<double, double>,
std::string_view). The cling-based backend did not hit this problem
because TClingMethodInfo exposed uninstantiated function templates with
their default signatures as regular callable methods, letting the
converters (which do handle tuple to pair) resolve the call.
Construct an alternative prototype where each 2-element tuple is typed
as std::pair<T1,T2> and retry the lookup with it, only after the
initializer_list form failed, so previously working resolutions are
unaffected.
Fixes pyunittests-tree-dataframe-dataframe-hist.
The type_remap() helper used for global operator lookups compared class names against exact strings like "std::basic_string<wchar_t>". With CppInterOp, class names arrive fully expanded (e.g. "std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >"), so the comparison failed and a Python str mixed with std::wstring was remapped to std::basic_string<char>&, making the lookup for the nonexistent operator+(std::wstring, std::string) come up empty and raising NotImplementedError. Match "std::basic_string<charT" with charT followed by ',' or '>' instead, so names with and without the default template arguments spelled out both work. This fixes TestSTLSTRING::test08_string_operators in test_stltypes.py, which only failed on configurations where the default arguments are printed, and only when the str/wstring mix was the first wstring operation (later ones hit the cached __add__ overload).
…upstream] Sema::ForceDeclarationOfImplicitMembers, Sema::LookupConstructors and DeclContext::buildLookup can pull declarations out of an AST file; when the ASTReader finishes, it hands the deserialized decls to cling's DeclCollector, which requires an open transaction and otherwise aborts with "Missing transaction during deserialization!". Open the PushTransactionRAII before these calls in LookupConstructors, GetNamed, ExistsFunctionTemplate and LookupDatamember. Fixes the CI-only crash in pyunittests-...-cppyy-test-conversions introduced by the std::pair template-lookup retry (e8d7c4c), which made GetMethodTemplate touch a freshly instantiated std::pair specialization whose members were still undeserialized.
At import time, the cppyy frontend declares on Windows an explicit
specialization of std::endl<char> so that JITed code binds to the C++
runtime's exported symbol instead of instantiating its own. In a ROOT
build, headers in the PCH loaded on startup (e.g. RooAbsDataStore.h)
have already implicitly instantiated std::endl<char> at that point,
making the declaration ill-formed:
error: explicit specialization of 'endl<char, ...>' after instantiation
so cppdef raised SyntaxError and "import cppyy" itself failed, taking
down essentially every PyROOT test on the Windows CI. Catch the error
and continue: when the instantiation already exists, JITed code can use
it directly and the workaround is unnecessary.
Lookup::Named performs a redeclaration-style lookup (RedeclarationKind::ForVisibleRedeclaration), for which Sema::LookupQualifiedName deliberately stops at the queried context itself: a redeclaration must live in the same scope. Lookup of an existing member name per [class.qual] however also searches the base classes. GetNamed therefore missed any member inherited from a base, e.g. the `type` member typedef of std::tuple_element, which older libstdc++ (gcc8/alma8) implements by recursive inheritance with `type` declared in a base of the queried specialization; newer libstdc++ declares it directly in the specialization, hiding the problem. The cling backend used LookupHelper with full C++ semantics and was not affected. Retry the lookup with RedeclarationKind::NotForRedeclaration when the scope is a class and the direct lookup found nothing, accepting only an unambiguous result. Fixes test20_tuple_element of cppyy-test-cpp11features on alma8.
When auto-instantiating a templated method from Python argument types, a Python complex fell through to the generic __name__-based fallback in AddTypeName and was typed as "complex", which does not resolve to any C++ type, so the instantiation failed. Map it to std::complex<double>, which matches Python's complex (double precision), like Python floats map to double. This went unnoticed before C++23 because implicit conversion to e.g. std::complex<float> went through the concrete converting constructor complex<float>(const complex<double>&), always present in the overload set, with the argument handled by the dedicated ComplexDConverter. In C++23, libstdc++ (P1467) replaces these constructors with a constructor template, which must first be instantiated from the Python argument types for the conversion to be viable. Fixes test01_instance_data_read_access of cppyy-test-datatypes on the CMAKE_CXX_STANDARD=23 CI configurations (debian13 et al.), where constructing CO<float> from a Python complex could not instantiate CO(std::complex<float>)'s implicit argument conversion.
guitargeek
force-pushed
the
cppyy-interop-migration_jonas
branch
from
74afc43 to
b1923e9
Compare
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
2 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.
Trying out some commits in the CI.