Skip to content

BugFix: Reference counting for comma-separated rung instruction arguments#79

Open
johnpuksta wants to merge 2 commits into
mainfrom
fix/reference-counting-in-rungs
Open

BugFix: Reference counting for comma-separated rung instruction arguments#79
johnpuksta wants to merge 2 commits into
mainfrom
fix/reference-counting-in-rungs

Conversation

@johnpuksta

@johnpuksta johnpuksta commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Problem

When a rung contains instructions with multiple comma-separated arguments (e.g., MOV(Source, Dest) or TON(Timer, Preset, Accum)), only the first argument is recognized as a tag reference. Arguments after commas are silently dropped.

Root Cause

Instruction.ParseArguments() splits instruction text by comma but doesn't trim whitespace from the results. For MOV(Source, Dest), the split produces ["Source", " Dest"]. The leading space on " Dest" causes TagName.IsTag() to fail (the base name regex ^[A-Za-z_][\w:]{0,39}$ rejects leading spaces), so the argument is classified as Unknown instead of Reference.

Fix

  • Instruction.ParseArguments(): Add .Trim() to comma-split argument results
  • ArgumentType.Of(): Trim before type classification (defensive, for direct callers)

Tests Added

  • RungTests: Parameterized tests for multi-arg instructions, duplicate tags across instructions, and branch scenarios
  • ArgumentTests: Leading whitespace tolerance
  • ArgumentTypeTests: Leading whitespace tolerance
  • TagNameTests: IsTag() behavior with whitespace (enforces that trimming is not it's responsibility)
  • InstructionTests: Comma-separated args with whitespace
  • L5XReferencesTests: End-to-end reference counting for same tag in different instructions and same instruction

@johnpuksta
johnpuksta requested a review from tnunnink as a code owner July 8, 2026 03:17
…pe.Of to handle leading whitespace bug preventing proper referencing of multiple items in rungs
@johnpuksta
johnpuksta force-pushed the fix/reference-counting-in-rungs branch from e02a623 to 7a44277 Compare July 8, 2026 03:19
@johnpuksta johnpuksta self-assigned this Jul 8, 2026
@johnpuksta johnpuksta changed the title Fix reference counting for comma-separated instruction arguments BugFix: Reference counting for comma-separated instruction arguments Jul 8, 2026
@johnpuksta johnpuksta changed the title BugFix: Reference counting for comma-separated instruction arguments BugFix: Reference counting for comma-separated rung instruction arguments Jul 8, 2026
@johnpuksta johnpuksta added the bug Something isn't working label Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant