Consolidate duplicated GetLdcValue into ILInstruction

[Copilot]

GetLdcValue was duplicated across IL2NESWriter.Helpers.cs and Transpiler.LocalFrameAllocation.cs with subtly different case ordering, risking silent divergence if one copy gets a bug fix or new opcode support.

• Added GetLdcValue() as a public instance method on ILInstruction, matching the existing GetStlocIndex() pattern
• Replaced both static copies with thin forwarding calls to preserve source compatibility at all call sites

// ILInstruction.cs — single source of truth
public int? GetLdcValue() => OpCode switch
{
    ILOpCode.Ldc_i4_m1 => -1,
    ILOpCode.Ldc_i4_0 => 0,
    // ...
    ILOpCode.Ldc_i4_s or ILOpCode.Ldc_i4 => Integer,
    _ => null
};

// IL2NESWriter.Helpers.cs / Transpiler.LocalFrameAllocation.cs — forwarding
static int? GetLdcValue(ILInstruction instr) => instr.GetLdcValue();

[jonathanpeppers]

/review

[github-actions]

✅ dotnes PR Reviewer completed successfully!

[Copilot]

Pull request overview

This PR removes duplicated GetLdcValue switch logic by making GetLdcValue() a single-source-of-truth instance method on ILInstruction, then updates existing call sites to forward to it (preserving the prior helper signatures).

Changes:

• Added ILInstruction.GetLdcValue() alongside the existing GetStlocIndex() helper.
• Replaced duplicated GetLdcValue(ILInstruction) implementations with thin forwarding wrappers in the transpiler and writer helpers.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
src/dotnes.tasks/Utilities/ILInstruction.cs	Introduces GetLdcValue() on ILInstruction to centralize ldc constant decoding.
src/dotnes.tasks/Utilities/IL2NESWriter.Helpers.cs	Replaces the local GetLdcValue switch with a forwarder to ILInstruction.GetLdcValue().
src/dotnes.tasks/Utilities/Transpiler.LocalFrameAllocation.cs	Replaces the local GetLdcValue switch with a forwarder to ILInstruction.GetLdcValue().

[github-actions]

✅ LGTM — Clean, correct deduplication.

Summary: This PR consolidates two copies of GetLdcValue (from IL2NESWriter.Helpers.cs and Transpiler.LocalFrameAllocation.cs) into a single instance method on ILInstruction, matching the existing GetStlocIndex() pattern. The old call sites become thin forwarding stubs.

Verified:

• The two old copies had different case ordering (Ldc_i4_m1 first vs. last) — functionally identical but a divergence risk. Now eliminated.
• The new canonical implementation covers all Ldc_i4_* opcodes correctly.
• All ~30 callers go through the static forwarding methods, so no call-site changes needed.
• Build ✅, macOS smoke test ✅, Windows smoke test in progress.
• No snapshot or behavioral changes — pure refactoring.

Severity	Count
❌ Error	0
⚠️ Warning	0
💡 Suggestion	2

Generated by dotnes PR Reviewer for issue #492 · ● 1.3M

[github-actions]

🤖 💡 Code organization — Nice consolidation. The pattern mirrors GetStlocIndex() above and unifies the two divergent case orderings (the Transpiler copy had Ldc_i4_m1 as the last case instead of first). Functionally identical but eliminates the divergence risk.

Follow-up thought: GetLdlocIndex in IL2NESWriter.Helpers.cs (lines 18-26) follows the same "static helper wrapping a switch on OpCode" pattern. It could similarly move here as an instance method on ILInstruction in a future PR.

[github-actions]

🤖 💡 YAGNI — Keeping this forwarding method avoids touching ~30 callers, which is a pragmatic choice for this PR. If a future cleanup pass removes these forwarding stubs and updates callers to use instr.GetLdcValue() directly, the call sites will actually read a bit more naturally (since it's an instance method on the object they already have).