[cpyrt] Detect enum arguments by type in overload priority - #75
Open
aaronj0 wants to merge 22 commits into
Open
Conversation
…er-research#41) * [cpyrt] Improve error reporting for method calls without C++ object Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * [test] Add test for method calls on an instance without a C++ object Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Grigori Rybkine <Grigori.Rybkine@cern.ch> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…esearch#40) * [cpyrt] Penalize void* arguments in overload priority as intended * [test] Add regression test for void* overload priority --------- Co-authored-by: Emery Conrad <emery.conrad@chicagotrading.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…rch#42) Fixes the long-standing header duplication (previously `Cppyy.h` and `cpp_cppyy.h`) between cpyrt and interop, so that there is only a single source of definitions for the `cppjit::interop` API and types.
Changed definition of a function to the test it is actually used, and added a missing import
…er-research#50) Drops wheel sizes by about half. Previously the entire install tree of CppInterOp was staged that included duplicate shared libs due to versioning. This is fixed by adding a stripped shared-lib option in CppInterOp, leveraged in this patch.
…ch#52) [cpyrt] Clear the error indicator only where a call failed A debug build of CPython asserts when a C API call is made with the error indicator already set, so the __cpp_cross__ annotation, the meta_setattro fallthrough to tp_setattro, and the VectorData alias need it cleared. Clear it only on the failing path: AddToClass reports failure, and the two CPPScope sites can test the value they just produced, so an error raised elsewhere still propagates. meta_getattro returns a new reference; release it instead of leaking it.
* [test] Find eigen and boost under the Homebrew and MacPorts prefixes * [ci] Build and test manylinux and macOS arm64 wheels
…esearch#54) SetArg() created an element converter per call and appended it to fConverters, but Clear() frees only fBuffer, so the vector grew without bound across repeated std::initializer_list conversions. Create each element converter once, on first use of its index, and reuse it.
…iler-research#51) * Serialize and atomize test dictionary builds * Force loadfile scheduling for distributed test runs * Normalize the xfail marker keyword order * Correct the xfail markers and add missing reasons * Add the --run-crashing-xfails collection option * Enable strict xfail * Drop xfail markers that no longer fail on macOS and cling * Make the span tests include their own header
…ompiler-research#66) A deleted function is not callable, yet it still entered the overload set and contributed a spurious conversion error to every failed-call report. That defeats SetDetailedException's rule that failures which are all C++ exceptions of one type are re-raised as that type. Un-xfails test34_no_ctors_in_base in test_crossinheritance.py, which expects TypeError for a class whose only constructors are deleted.
…mpiler-research#64) Expose method const-qualification in func_overloads_types * [cpyrt] Add per-overload 'is_const' to GetSignatureTypes * [test] Add regression test for overload constness reporting Co-authored-by: Yasser Rathore <yasser.rathore@chicagotrading.com> Co-authored-by: rathorey-ctc <rathorey-ctc@users.noreply.github.com>
…h#69) [test] Correct the xfail marks tripping the nightly matrix
…arch#72) * [ci] Skip README-only PRs and drop the dead master push trigger * [ci] Run the nightly matrix on PRs labeled test-nightly
…arch#71) VoidPtrRefConverter was registered only for "const void*&", so a mutable void*& fell under the general ban on non-const pointer references. Passing an object through an opaque void*& handle is supported by design; the exact-match factory lookup runs before the T*& rejection, which stays in effect for typed pointer references.
…rch#70) ExecuteFast() ends with a Windows-only block that drops the result when a Python exception is pending. It is shared by every CPPMethod subclass, including CPPConstructor, whose executor returns the address of the new C++ object cast to PyObject*; decref'ing that corrupts the heap. Ask the method whether its executor hands back a real PyObject* first. The object leaks instead, on the error path of a call that is already failing.
…earch#65) LowLevelView.reshape() verified sizes by comparing the sum of the dimensions instead of the number of elements, their product: reshaping a five element array to (2, 3) was accepted because 5 == 2 + 3, while (1, 5) was rejected. The check is skipped for arrays of unknown size, the ones that typically get reshaped, which is why this went unnoticed. A correct size check is not enough, though: the strides and the converter projecting sub-views are chosen for the rank and layout of the view's C++ type and are not re-derived when reshaping. Any rank change produced a view reading garbage, and even the identity reshape of a fixed int[3][5] corrupted its strides. Reshape therefore now does what it is actually used for: providing the extent of dimensions the type leaves open, such as the size of an array behind a pointer. The rank must match, and only an unknown or empty dimension may be set, with -1 still standing for "unknown"; anything else raises ValueError, including dimensions whose byte size would overflow. The byte length is counted in strides of the outermost dimension as the creators count it, which for views with an itemsize override (const char*[], notably) differs from the itemsize, and the strides themselves are left as the creator laid them down. Also share the "fake max" marking an unknown outermost dimension between the creators and reshape (it was rederived from the itemsize, mistaking the unknown size of row-pointer and itemsize-overridden views for a known one), give the shape property a proper setter (reshape was installed directly despite its mismatching signature, so assignment misreported its result and deletion crashed), refuse to reshape a view without dimensions instead of reading through its null strides, and check allocations in the shape getter.
…ompiler-research#67) Every Python object has a __name__, so an object naming no C++ type reached AddTypeName's fallback, where GetType() returned null; that null was then pushed as a template argument and dereferenced by IsEnumType. Skip such an argument, and report the string-based path's failure as TypeError like the type-based one already does.
GetPriority() deprioritizes enum arguments so that a competing integer overload wins, since C++ has no implicit int->enum conversion, but it detected them with IsEnumScope(GetScope(name)). GetScope does not resolve an enum name to a scope, so the penalty never applied. Query the argument type with IsEnumType as well, per the existing FIXME.
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.
GetPriority() deprioritizes enum arguments so that a competing integer overload wins, since C++ has no implicit int->enum conversion, but it detected them with IsEnumScope(GetScope(name)). GetScope does not resolve an enum name to a scope, so the penalty never applied. Query the argument type with IsEnumType as well, per the existing FIXME.