Conversation
Signed-off-by: Andrew Studnicky <a.j.studnicky@gmail.com>
|
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. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
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
left a comment
There was a problem hiding this comment.
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:
super.init(...). Callees likesuper.initandself.inithave a lower-case receiver, soreceiver_chain_admits(src/pipeline/registry.c:1179) lets them through, andresolve_name_lookupthen picks the nearest node whose short name isinit. Before this PR there was no Swiftinitto bind to. Now aUIViewControllersubclass'ssuper.init(nibName:bundle:)could resolve to its owninit(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 plusclass Sub: Base { init() { super.init() } }. Ifsuper.initmis-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.- A before/after on a real Swift project (Alamofire, for example): CALLS counts plus a sample of the new
*.initedges. That's the evidence we use to judge graph-quality changes. - 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 aninit_declarationnode. Alang == CBM_LANG_SWIFTbranch incbm_resolve_func_name/resolve_method_name, like the Kotlinsecondary_constructorbranch inextract_class_methods, keeps this explicit.
Nits:
swift_init_multiple_not_collapsed_issue2379asserts that the inits do share one node (correctly, per #2061), so something like..._overloads_share_one_node_...would avoid confusion.find_def_in_modulehas two stacked header comments (test_extraction.c:72-75), and the first describes a different helper.
Thanks again!
What does this PR do?
Fixes #2379. Swift
initanddeinitdeclarations are emitted as methods namedinitanddeinit, 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 skippedmake -j3 -f Makefile.cbm lint-ci lint-tidy-diff— passedscripts/security-audit.sh— passedbuild/c/test-runner extraction— 386 passedChecklist
git commit -s)make -f Makefile.cbm test)make -f Makefile.cbm lint-ci)