fix(registry): resolve aliased direct-symbol imports in import_map (#875) - #979
Conversation
Sorry, something went wrong.
Signed-off-by: LA-10 <lashuha@uwaterloo.com>
|
On where the fix lives, I went with making the resolver handle the value shape build_import_map already produces. The other option would be normalizing at construction instead (store the module QN plus the original leaf name, so the append rule holds for everything). That's arguably cleaner, but it means touching build_import_map in both pass_calls.c and pass_parallel.c and changing the contract at pipeline.h:180. Happy to redo it that way if you'd rather. |
Sorry, something went wrong.
|
Thanks for digging into the import-map shape. I have triaged this as a 0.9.1-rc bug fix for #875. During review we will compare the local resolver fix with the alternative of normalizing build_import_map, and we should keep the LSP alias miss as a separate follow-up so this PR stays focused. |
Sorry, something went wrong.
|
Merged — thank you! The asymmetry analysis (module-valued vs symbol-valued import-map entries) was spot-on, the four-line fix lands exactly at the root cause, and the end-to-end verification on the issue's repro repo is the gold standard. Much appreciated. |
Sorry, something went wrong.
The DeusData#979 direct-hit early return in resolve_import_map fires before suffix handling, so any imported_symbol.method() call whose base symbol is itself an indexed node resolved to the base node instead of base.method — the exact mis-resolution the neighboring comment warns about. On django this degraded the graph by ~11K edges (CALLS 74,685->72,175, TESTS 38,032->29,236); auth.login => Signal.send became an edge onto the user_logged_in Variable node. Take the direct hit only for suffix-less callees — precisely the aliased direct-symbol import case (DeusData#875) the early return was added for. Verified on django: CALLS 74,708 (baseline + the 23 genuine DeusData#979 wins), TESTS 38,035; the DeusData#979 bare-alias test stays green. Reproduce-first: resolve_import_map_alias_with_suffix_hits_method is RED on the unguarded code (returns the bare symbol QN) and GREEN with the guard. Closes DeusData#1000 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
What does this PR do?
trace_path returns no inbound caller for a function called through an import alias (from m import f as g; g()). No CALLS edge is emitted at all.
Root cause (registry.c, resolve_import_map): build_import_map stores key = local_name → value = the IMPORTS edge's target QN. Plain imports target the Module node (value = module QN); aliased direct-symbol imports target the Function node (value = full symbol QN). The bare-callee branch assumes the former and always appends:
For _scan_bash that yields ...security_scan.scan_bash._scan_bash matches nothing, edge dropped. Plain imports work because module + "." + name reconstructs the real QN. That asymmetry is why only aliases break, and why resolve_import_map_bare_function (module-valued vals[]) never caught it.
Fix: if resolved is itself a registered symbol, it is the target return it before appending. Cannot regress plain/dotted imports: pass_definitions.c:337 registers only never Module, so module-valued entries can't match. resolve_import_map is shared by the sequential and parallel pipelines, so one change covers both.
Issue #875
Verification
tests/test_registry.c::resolve_import_map_bare_alias — RED on unmodified clone, GREEN with fix. Existing import_map tests stay green.
End-to-end on willow-2.0 (the issue's public repro), trace_path(scan_bash, inbound):
Call-site identity preserved, per the triage note:
Checklist
unsigned commits (DCO, see CONTRIBUTING.md)