[ilasm] Fix AsmParse::error to always write to stderr

[Copilot]

This applies the same fix to AsmParse::error that was made to AsmParse::warn in #125320: suppress error output when the -ERR command line switch (assem->OnErrGo) is enabled, and always use stderr for output when diagnostics are emitted.

Additionally:

• The misleading assem->OnErrGo ? "warning : " : "error : " conditional prefix (dead code after the early return) has been simplified to a fixed "error : " string.
• In assembler.cpp, the 7 call sites that previously used if(OnErrGo) report->error(...); else { report->warn(...); /* fixup */ } have been cleaned up. Each is now if(!OnErrGo) { report->warn(...); /* fixup */ } — when OnErrGo is set, neither the diagnostic nor the fixup is applied (preserving the original invalid output), and when not set, the warning is emitted and the fixup is applied as before.

Note

This PR description was generated with the assistance of GitHub Copilot.

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

Updates native ilasm parsing diagnostics so AsmParse::error() behaves like AsmParse::warn() under -ERR (error-tolerant) mode, reducing unwanted diagnostic output and ensuring consistent stderr usage.

Changes:

• Suppress AsmParse::error() output when assem->OnErrGo (-ERR) is enabled.
• Simplify error output stream selection to always use stderr (when output is emitted).

Build/test status: Not run as part of this review.

[github-actions]

🤖 Copilot Code Review — PR #127339

Note

This review was generated by Copilot and validated across two model families (Claude Opus 4.6 primary, GPT-5.3-Codex secondary).

Holistic Assessment

Motivation: Making AsmParse::error() consistent with AsmParse::warn() — both now return early when OnErrGo is set, and errors always go to stderr. This aligns with the direction established in PR #125320.

Approach: Minimal, targeted change to a single function. The early return was explicitly authored by @jkotas and matches the existing warn() pattern exactly.

Summary: ✅ LGTM. The change is correct and well-motivated. The stderr simplification is clearly right (errors should go to stderr). The early return is safe because main.cpp:726 already handles OnErrGo independently of pParser->Success(). Two minor observations below — neither is merge-blocking.

---

Detailed Findings

✅ Early return safety — success = false no longer set when OnErrGo is true

Verified that this is safe. In main.cpp:726:

if ((pParser->Success() && fAllFilesPresent) || pAsm->OnErrGo)

When OnErrGo is true, the Success() result is irrelevant — assembly proceeds regardless. At line 744, if(pAsm->OnErrGo) exitval = 0; also handles the output path. So skipping success = false has no functional impact. (Confirmed by both models.)

✅ stderr change — correct

The old conditional ((!assem->m_fReportProgress)&&(assem->OnErrGo)) ? stdout : stderr was routing certain errors to stdout, which is incorrect for error output. With the early return, OnErrGo is always false when we reach this line, so stderr is the only correct target. (Confirmed by both models.)

💡 Dead ternary on line 1819 — follow-up cleanup

psz+=sprintf_s(psz, (dwUniBuf >> 1), assem->OnErrGo ? "warning : " : "error : ");

Since the function now returns early when OnErrGo is true, this ternary always evaluates to "error : ". The "warning : " branch is dead code. Could be simplified to just "error : " for clarity, but this is non-blocking. (Flagged by both models.)

💡 Callers gated on OnErrGo are now no-ops — intentional

Several call sites in assembler.cpp use the pattern if(OnErrGo) report->error(...) (lines 389–390, 400–401, 536, 629, 716, 725, 845) to emit diagnostic messages only in OnErrGo mode. With this early return, those calls are now silent no-ops. This appears intentional given the commit was authored by @jkotas, but it's worth noting that these ~8 diagnostic messages are now fully suppressed. If those messages were still desired, they would need to use a different output mechanism. (Flagged by both models.)

Generated by Code Review for issue #127339 · ◷

[jkotas]

Callers gated on OnErrGo are now no-ops

@copilot Clean this up

[Copilot]

Callers gated on OnErrGo are now no-ops

@copilot Clean this up

Done in 37e39b1. Removed all 7 if(OnErrGo) report->error(...) branches in assembler.cpp — each was paired with an else { report->warn(...); /* fixup */ } block, so the cleanup simplifies each to just the warn + fixup unconditionally (the warn itself is also a no-op when OnErrGo is set).

[jkotas]

/azp run runtime-coreclr outerloop

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jkotas]

/ba-g unrelated known failures