C API exceptions#63
Conversation
…dation in Field class
There was a problem hiding this comment.
Pull request overview
This PR introduces an exception-based error model across the Sista C API and the Python extension, adding per-thread “last error” diagnostics and updating Python bindings to raise Python exceptions instead of returning status codes.
Changes:
- Add
sista_getLastErrorCode()/sista_getLastErrorMessage()and convert multiple C API functions fromvoidtointstatus returns. - Update the Python C extension to translate C API status codes into Python exceptions and introduce a
PawnPython type (instead of exposing pawn capsules). - Update demos/docs/tests to reflect the new exception-based and status-code-based error handling.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| python/src/sistamodule.c | Adds status→exception translation helpers, clear_screen, and a Pawn Python type; updates wrappers to raise on C API failures. |
| python/src/sista/init.pyi | Updates typing surface for exception-based behavior and new Pawn type. |
| python/README.md | Documents Pythonic exceptions and updates usage examples and demo instructions. |
| python/demo/test_field.py | Updates demo to use Pawn objects and exercises exception paths. |
| python/demo/test_comprehensive.py | Adds runtime API checks and explicit tests for new exception behavior and Pawn type semantics. |
| include/sista/field.hpp | Updates API docs to reflect new bounds/validation behavior and exception contracts. |
| include/sista/field.cpp | Adds null/bounds guards and returns nullptr for out-of-bounds pawn lookup. |
| include/sista/api.h | Adds enum sista_ErrorCode, last-error API, and changes several APIs to return status codes. |
| include/sista/api.cpp | Implements last-error tracking and updates C API functions to return enum sista_ErrorCode values. |
| demo/Makefile | Adds build target/clean rule for new api-test-errors. |
| demo/api-test.cpp | Updates demo to check return status and print last-error diagnostics on failure. |
| demo/api-test-swap.cpp | Updates swap demo to check return status, use sista_clearScreen, and report errors via last-error. |
| demo/api-test-multiple-styles.c | Updates demo to validate status returns and print last-error diagnostics. |
| demo/api-test-errors.c | New C test validating last-error code/message behavior for failing calls. |
| demo/api-test-cursor.c | Updates cursor demo to validate status returns and print last-error diagnostics. |
| demo/api-test-border.cpp | Updates border demo to validate status returns and print last-error diagnostics. |
Comments suppressed due to low confidence (1)
python/src/sistamodule.c:1040
PawnObjecttracks anowner, butField_move_pawnonly extracts the raw handler and does not verify the pawn belongs to thisField. Passing a Pawn from a different Field can lead to out-of-bounds access/UB in the C++ layer (it indexes by the pawn's current coordinates). Validate( (PawnObject*)pawn_obj )->owner == self(or otherwise associate the pawn with the correct field) and raise a clear exception if it doesn't match.
PawnHandler_t pawn = py_sista_get_pawn_from_object(pawn_obj);
if (pawn == NULL) {
return NULL;
}
FieldHandler_t field = ((FieldObject*)self)->field;
if (field == NULL) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| PawnHandler_t pawn = py_sista_get_pawn_from_object(pawn_obj); | ||
| if (pawn == NULL) { | ||
| PyErr_SetString(PyExc_ValueError, "Invalid PawnHandler_t capsule"); | ||
| return NULL; | ||
| } | ||
|
|
There was a problem hiding this comment.
PawnObject tracks an owner, but SwappableField_add_pawn_to_swap does not verify the pawn belongs to this SwappableField. A Pawn created in a different field can be passed here and cause undefined behavior in the underlying C++ implementation. Check that the pawn's owner matches self (and error out) before calling sista_addPawnToSwap.
There was a problem hiding this comment.
Done in d5e456e. After py_sista_get_pawn_from_object validates the pawn type and liveness, the pawn's owner field is compared against self. If they don't match, a ValueError("Pawn does not belong to this SwappableField") is raised before sista_addPawnToSwap is called.
Agent-Logs-Url: https://github.com/FLAK-ZOSO/Sista/sessions/11623a30-572c-4fff-850f-c262d13548d3 Co-authored-by: FLAK-ZOSO <83119697+FLAK-ZOSO@users.noreply.github.com>
Agent-Logs-Url: https://github.com/FLAK-ZOSO/Sista/sessions/b4201e79-5010-4d4b-a696-b6850241e39f Co-authored-by: FLAK-ZOSO <83119697+FLAK-ZOSO@users.noreply.github.com>
No description provided.