Skip to content

fix rename/references missing asm output operands - #3243

Open
DaliVana wants to merge 1 commit into
zigtools:masterfrom
DaliVana:issue3222
Open

fix rename/references missing asm output operands#3243
DaliVana wants to merge 1 commit into
zigtools:masterfrom
DaliVana:issue3222

Conversation

@DaliVana

@DaliVana DaliVana commented Aug 7, 2026

Copy link
Copy Markdown

Fixes #3222

Renaming a variable did not update its occurrence inside an inline asm output operand:

pub fn main() void {
    var bar = 5;
    asm volatile (
        \\ mov $5, %rax
        : [ret] "={rax}" (bar), // <-- not renamed
    );
}

The same occurrence was also missing from textDocument/references and textDocument/documentHighlight.

Cause
In the grammar, an asm output operand without -> is a bare IDENTIFIER — parseAsmOutputItem consumes the token and creates no AST node for it (only the -> type form stores a node). Reference collection (Builder.referenceNode) walks AST nodes, so the operand token was invisible. Asm inputs work because their operand is a full expression node yielded by the child iterator (#1142); outputs with -> likewise.

Fix
Handle .asm_simple/.@"asm" in Builder.referenceNode: for each output item, the operand token is nodeMainToken(output) + 4 (the same layout already relied on in semantic_tokens.zig and the ChildIterator). The -> T form is skipped (already covered as a child node). The operand then goes through the same treatment as the .identifier case: the _/primitive/escaped-identifier guard, lookupSymbolGlobal, alias resolution, and target_symbol.eql — added per matching output, so multiple outputs referencing the same variable all get collected.

This fixes rename, find-references, and document highlight in one place, since all three share symbolReferences.

Tests
Five new cases in test "asm":

  • plain output operand (the issue's repro shape)
  • two outputs referencing the same variable
  • arrow-form and plain-form output mixed in one asm (guards the skip logic)
  • container-level var shadowed by a same-named local used in an asm operand — must not be reported for the container decl (exercises the symbol-identity check negatively)
  • const @"i32" with a bare (i32) operand — must not be reported (asm analogue of the existing escaped-identifier-vs-primitive test)
  • The negative tests were mutation-verified: removing the primitive guard or neutralizing the eql check each makes the suite fail.

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.

lsp renaming doesn't apply to asm blocks parameters

1 participant