GeezerVison

crates/capi/src/longobject.rs (1)

61-83: 🏗️ Heavy lift

Replace the dead test block with enabled coverage of the exported FFI helpers.

#[cfg(false)] means none of this runs, and these tests only exercise PyO3's PyInt, not the new PyLong_From* / PyLong_As* symbols. Please swap this for enabled tests around the FFI helpers so ABI regressions get caught.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/capi/src/longobject.rs` around lines 61 - 83, Replace the dead
#[cfg(false)] test module with an active #[cfg(test)] mod tests that directly
exercises the crate's exported FFI helpers (the PyLong_From* / PyLong_As*
symbols) instead of PyO3's PyInt: remove #[cfg(false)], add tests that call the
exported functions (e.g., the crate's PyLong_From<u64>-style and
PyLong_As<u64>-style helpers via their extern "C" signatures), perform
round-trip conversions (create a long from a Rust integer and extract it back)
and assert correctness so ABI/FFI regressions are caught; keep the test harness
using Python::attach or equivalent to run under the Python GIL as in the
original tests.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

crates/capi/src/longobject.rs (1)

49-58: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the index protocol in the unsigned extractor.

This still rejects valid __index__ providers that PyLong_AsLong accepts, and the overflow text names the signed C type. That makes PyLong_AsUnsignedLongLong observably inconsistent with the rest of this file.

Suggested fix

-        unsafe { &*obj }
-            .to_owned()
-            .try_downcast::<PyInt>(vm)?
-            .as_bigint()
-            .try_into()
-            .map_err(|_| vm.new_overflow_error("Python int too large to convert to C long long"))
+        unsafe { &*obj }
+            .to_owned()
+            .try_index(vm)?
+            .as_bigint()
+            .try_into()
+            .map_err(|_| {
+                vm.new_overflow_error("Python int too large to convert to C unsigned long long")
+            })

#!/bin/bash
set -euo pipefail

echo "Compare the signed and unsigned long extractors:"
sed -n '37,59p' crates/capi/src/longobject.rs

echo
echo "Show the coercion helpers used in this file:"
rg -n -C2 'PyLong_As|try_index\(|try_downcast::<PyInt>' crates/capi/src/longobject.rs

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/capi/src/longobject.rs` around lines 49 - 58, The unsigned extractor
PyLong_AsUnsignedLongLong currently bypasses the index protocol and uses PyInt
downcast and a signed overflow message; update it to mirror the signed extractor
by using the index protocol helper (e.g., call try_index or the same helper used
by PyLong_AsLong) to coerce obj to an integer, then convert the resulting BigInt
to c_ulonglong and return vm.new_overflow_error with text naming the unsigned C
type ("Python int too large to convert to C unsigned long long") on overflow;
locate and follow the pattern in the signed extractor (PyLong_AsLong) and reuse
its coercion/try_index logic and error handling.