Repository navigation
[WIP] [cppjit] New overload resolution - #23715
Draft
Vipul-Cariappa wants to merge 17 commits into
Draft
Vipul-Cariappa wants to merge 17 commits into
Vipul-Cariappa wants to merge 17 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 23 files 23 suites 3d 2h 47m 56s ⏱️ For more details on these failures, see this check. Results for commit 142dbc5. |
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
3 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.