Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: minor
PR #239 rewrote resolve_adjacency in core/analysis/src/call_graph.rs to keep-first on duplicate FnKeys in all builds. Previously a duplicate tripped debug_assert!(false) — a guard added precisely because "FnKey must be injective or A035/A036 may miss a self-edge", i.e. a tripwire for the same-named-type / canonical-key identity bug class this repo has actually shipped (#63).
The removal itself was justified: the resilient IDE path feeds parse-recovered <error> FnKeys that legitimately collide, and the old assert aborted debug builds on recovered parses. But the tripwire is now gone for the compiler path too, where a genuine duplicate FnKey still means an identity bug that can silently change analysis verdicts (missed self-edges in A035/A036).
Suggested fix: reinstate a guard that tolerates recovered keys but still trips on genuine duplicates — e.g. debug_assert (or at least a log/diagnostic) when a duplicate FnKey does not involve an <error> segment, or gate the assert on the non-resilient (compiler) entry point.
Also: CHANGELOG.md does not record this behavior change (debug_assert removed; silent keep-first in all builds) — worth a line when fixing.
Follow-up from the comprehensive review of PR #239 (LSP server). Line numbers reference the PR branch at review time.
Severity: minor
PR #239 rewrote
resolve_adjacencyincore/analysis/src/call_graph.rsto keep-first on duplicateFnKeys in all builds. Previously a duplicate trippeddebug_assert!(false)— a guard added precisely because "FnKey must be injective or A035/A036 may miss a self-edge", i.e. a tripwire for the same-named-type / canonical-key identity bug class this repo has actually shipped (#63).The removal itself was justified: the resilient IDE path feeds parse-recovered
<error>FnKeys that legitimately collide, and the old assert aborted debug builds on recovered parses. But the tripwire is now gone for the compiler path too, where a genuine duplicate FnKey still means an identity bug that can silently change analysis verdicts (missed self-edges in A035/A036).Suggested fix: reinstate a guard that tolerates recovered keys but still trips on genuine duplicates — e.g. debug_assert (or at least a log/diagnostic) when a duplicate FnKey does not involve an
<error>segment, or gate the assert on the non-resilient (compiler) entry point.Also: CHANGELOG.md does not record this behavior change (debug_assert removed; silent keep-first in all builds) — worth a line when fixing.