Skip to content

fix(extraction): attribute Swift lifecycle calls - #2395

Open
Studnicky wants to merge 1 commit into
DeusData:mainfrom
Studnicky:fix/swift-init-extraction
Open

Studnicky wants to merge 1 commit into
DeusData:mainfrom
Studnicky:fix/swift-init-extraction

Conversation

@Studnicky

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #2379. Swift init and deinit declarations are emitted as methods named init and deinit, so calls in their bodies retain the lifecycle method as their enclosing function instead of falling back to module scope.

The regression coverage exercises struct, class, convenience, failable enum, extension, ordinary-method, and deinitializer cases.

This contribution is written with Codex under the account holder's direction.

Validation

  • make -f Makefile.cbm test — 8,198 passed, 10 skipped
  • make -j3 -f Makefile.cbm lint-ci lint-tidy-diff — passed
  • scripts/security-audit.sh — passed
  • build/c/test-runner extraction — 386 passed

Checklist

  • Every commit is signed off (git commit -s)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test

Signed-off-by: Andrew Studnicky <a.j.studnicky@gmail.com>
@Studnicky
Studnicky requested a review from DeusData as a code owner September 28, 2026 02:34
@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@DeusData DeusData 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.

Thank you, @Studnicky, both for the precise report in #2379 (the minimal repro and the query_graph output made the gap obvious) and for turning it into a fix with eight focused tests. They cover struct, class, convenience, failable, extension and deinit, plus a guard that ordinary methods keep their attribution. Welcome back; #1470 and #1471 are still doing their job.

The extraction side is right: init/deinit become Type.init/Type.deinit Methods, and calls in their bodies now come from them. Because those nodes are also new call targets, we need to see the edge-level result before merging:

  1. super.init(...). Callees like super.init and self.init have a lower-case receiver, so receiver_chain_admits (src/pipeline/registry.c:1179) lets them through, and resolve_name_lookup then picks the nearest node whose short name is init. Before this PR there was no Swift init to bind to. Now a UIViewController subclass's super.init(nibName:bundle:) could resolve to its own init (a self-loop) or to a sibling type's. We keep the graph to true edges only, so could you add a pipeline-level test that asserts the exact CALLS set? Use the #2379 repro plus class Sub: Base { init() { super.init() } }. If super.init mis-binds, please handle it, for example by resolving it through the class's base, or dropping it when the base isn't in the project.
  2. A before/after on a real Swift project (Alamofire, for example): CALLS counts plus a sample of the new *.init edges. That's the evidence we use to judge graph-quality changes.
  3. Gate the naming on Swift in one place. func_name_node (extract_defs.c:212) has no language parameter, and the Slang grammar also has an init_declaration node. A lang == CBM_LANG_SWIFT branch in cbm_resolve_func_name / resolve_method_name, like the Kotlin secondary_constructor branch in extract_class_methods, keeps this explicit.

Nits:

  • swift_init_multiple_not_collapsed_issue2379 asserts that the inits do share one node (correctly, per #2061), so something like ..._overloads_share_one_node_... would avoid confusion.
  • find_def_in_module has two stacked header comments (test_extraction.c:72-75), and the first describes a different helper.

Thanks again!

This branch has not been deployed

No deployments
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.

Swift: initializers are not extracted — calls inside init are attributed to the file

2 participants