Skip to content

Deep audit fixes: consolidate macros, harden C-API bridging, fix Python version hardcoding, add CI & tests - #3

Merged
jagtesh merged 8 commits into
mainfrom
copilot/deep-audit-bugs-and-improvements
Jul 4, 2026
Merged

jagtesh merged 8 commits into
mainfrom
copilot/deep-audit-bugs-and-improvements

Conversation

Copilot AI commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

The ApplePy deep audit flagged critical and high-severity bugs across the macro implementations and CPython bridging layer: a stale duplicate macro target, hardcoded python3.13 that breaks the build on any other version, unchecked C-API return values that silently corrupt collections, force-unwraps that crash the interpreter, and zero CI/test coverage for most macros. This PR addresses the critical/high findings and several medium ones.

Macro consolidation

  • Removed Sources/ApplePyMacroCore — a stale duplicate of Sources/ApplePyMacros that had drifted (different error handling) and was never referenced in Package.swift. ApplePyMacros is now the single source of truth.

Python version hardcoding

  • module.modulemap hardcoded link "python3.13", breaking any install without that exact version. Switched ApplePyFFI's pkg-config module to python3-embed (correct for an embedding host process) and dropped the hardcoded link.
  • ApplePyBundle plugin now probes python3, python3.13...python3.9 instead of assuming python3 is on PATH.
  • Example build scripts (Examples/*/build.sh) had the same hardcoded version plus a macOS-only linker flag; fixed both and verified the examples build/run on Linux.

C-API bridging hardening

  • Collections.swift: check return codes on PyList_SetItem, PyDict_SetItem, PySet_Add, PyTuple_SetItem, and *_Size() instead of ignoring failures.
  • Added PyErr_Occurred() checks after PyIter_Next/PyDict_Next loops — both APIs return NULL/0 for "done" and "errored", so a real exception mid-iteration was being silently swallowed.
  • Replaced force-unwrapped swiftPtr in generated _getSwiftValue and PyBridge.load/loadBox with nil-returning failure paths instead of crashing the whole interpreter on a corrupted Swift/Python pairing.
  • Fixed reference leaks in PyEnumMacro-generated registration code on failure paths (e.g. baseBases/variantClass leaked when a later step failed).

Macro-time diagnostics

  • @PyFunction now rejects async functions, generic functions, and inout parameters instead of silently generating broken wrappers.
  • @PyClass rejects generic structs/classes.
  • @PyProperty rejects computed properties.
  • @PyEnum validates case names and parameter labels are valid, non-keyword Python identifiers (previously a param named e.g. class would generate syntactically invalid Python via raw string interpolation).
  • @PyModule now reuses the correct METH_NOARGS/METH_VARARGS flag computed by @PyFunction per function, instead of hardcoding METH_VARARGS for every function regardless of arity.

CI and testing

  • Added a swift test GitHub Actions workflow (macOS + Linux matrix) — previously only a docs-publishing workflow existed.
  • Discovered assertMacroExpansion (from SwiftSyntaxMacrosTestSupport) reports failures via XCTFail, which is silently swallowed when called from Swift Testing @Test functions — every macro-expansion test was passing regardless of actual output. Switched to the framework-agnostic SwiftSyntaxMacrosGenericTestSupport API with a custom failure handler wired to Issue.record, so these tests now genuinely validate expansion. This surfaced a real bug in @PyUnion (emitting broken empty conformance extensions when misapplied to a non-enum), now fixed.
  • Backfilled test coverage for Set<T>, PyTuple2, PyTuple3, bytes/bytearray conversion, and the new macro diagnostics (18 → 36 tests).

Other medium-priority fixes

  • Documented the @unchecked Sendable safety invariant (GIL-serialized access) on PyObjectBox and PyExceptionType.
  • BufferBridge.bytesFromPython now type-checks with PyBytes_Check/PyByteArray_Check before extracting (also adds previously-unsupported bytearray handling) and guards negative size results.
  • Fixed silent Int64 → Int truncation in Primitives.swift; documented Float/Double precision loss.
  • Pinned swift-syntax to 600.0.0..<700.0.0 instead of an open-ended from: range; pinned GitHub Actions by SHA.

Known gap (pre-existing, not introduced here)

@PyClass doesn't currently wire up codegen for @PyMethod instance methods, @PyProperty getters/setters, or @PyStaticMethod — these attributes validate correctly but aren't read by @PyClass's member generation. Flagging for a follow-up since it's a larger change than this audit pass.

@jagtesh jagtesh left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot we should add support for the latest (and upcoming) python versions.

/// This avoids hardcoding a single Python version that may not be
/// installed on the host machine.
private func detectPythonExecutable() throws -> String {
let candidates = ["python3", "python3.13", "python3.12", "python3.11", "python3.10", "python3.9"]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the newer python versions, 3.14 and 3.15

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added python3.14 and python3.15 to the probe list in e28b19b.

Copilot AI commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

@copilot we should add support for the latest (and upcoming) python versions.

Added python3.14 and python3.15 to the interpreter probe list in e28b19b.

Copilot AI requested a review from jagtesh July 3, 2026 23:59
@jagtesh
jagtesh merged commit 0abcd51 into main Jul 4, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants