[jogibear9988]

Fixes #561.

A comparison operator returning an interface currently emits ceq (or an invalid object.Equals call) instead of its selected method. The integer result is returned as an object reference, causing the reported fatal CLR error. Emit the exact non-lifted BinaryExpression.Method, preserve its return type and operand evaluation order, and discard ignored results correctly. Prevent Boolean constant interpretation from dropping method-backed binary calls and their side effects.

The production change is 46 added lines in one file. It uses existing emission/local-storage helpers, without constructing replacement expression nodes or changing compiler context layout. Lifted comparison handling stays on its existing path. This is based directly on current master (3aefb5e5) and has no dependency on #558.

Regression coverage includes ==/!=, both operand orders, interface and value results, discarded results, nested try/finally evaluation order, nullable-accepting non-lifted methods, and constant Boolean methods. The modern native fixture checks for the selected call before invoking a potentially corrupt delegate, without asserting an exact IL layout.

Validation at a7deb9e8:

• Production Standard/Light builds pass across their configured targets.

• Full .NET 10 ARM64 and .NET 8 x64 native suites pass (1,714/1,713 legacy cases, plus registered regressions).

• Framework native suites pass: 97 TestX and 1,597 legacy cases.

• All nine targeted regression methods pass for Standard/Light on .NET 8/10, ARM64/x64; the baseline fails in all eight configurations. All 50 isolated raw repro variants pass with the fix, including Framework and debug mode. Strict FEC compilation is used throughout.

Tests ran on Windows 11 ARM64; x64 uses the installed x64 host under emulation. The full .NET 8 ARM64 suite hits the pre-existing by-ref float post-increment crash in Issue346. Isolated clean-master and fixed-source probes reproduce the same crash with identical IL on ARM64; both probes pass on x64 using the same 8.0.28 runtime patch. No test is skipped or changed to hide it. Framework's dynamic-method decoder is unavailable, so its issue-specific raw repros run in child processes rather than the guarded modern fixture.

Review follow-up at f2c784e0: reuse IsParamOrConstantOrDefault to avoid unnecessary spills through ordinary conversions, while retaining intrinsic-emitter handling throughout LightExpression conversion chains. Added converted parameter/constant/default coverage and a nested intrinsic try/finally regression. All 10 Standard / 11 Light targeted methods pass on .NET 8/10, ARM64/x64; full .NET 10 ARM64, .NET 8 x64, and Framework suites pass again. The validation above at a7deb9e8 remains the original baseline/fix and isolated repro evidence.

Readability follow-up at 3a0576b2: make the intrinsic early exit explicit. All targeted methods pass again across the same eight Standard/Light, .NET 8/10, ARM64/x64 configurations.

[dadhi]

generally looks good. will double check tomorrow. thank you!

[dadhi]

I don't understand what this code doing, spillLeft will be overwriten by the next operand, until no more conversion. So what this comment assumes is not true - it overriten by the last operand anyway. // A conversion chain may contain a custom IL emitter.

Also you need to test such special case with Intrinsic.

The current code as whole may can be replaced by:

                // Keep the evaluation stack empty while a nontrivial right operand
                // may emit exception regions or jumps, just as for call arguments.
                var right = expr.Right;
                var operand = right.StripConvertRecursively();
                var spillLeft = operand.NodeType == ExpressionType.Parameter
                    | operand.NodeType == ExpressionType.Constant
                    | operand.NodeType == ExpressionType.Default;
#if LIGHT_EXPRESSION
                spillLeft |= operand.IsIntrinsic;
#endif

[jogibear9988]

Made the early exit explicit in 3a0576b:

if (operand.IsIntrinsic)
{
    spillLeft = true;
    break;
}

The previous loop also stopped once spillLeft became true because its condition was !spillLeft, so later operands could not overwrite that decision. The explicit break makes this easier to see.

The reason for inspecting the conversion chain is that a conversion node itself can be intrinsic. StripConvertRecursively() would skip that node and could leave just its ordinary parameter operand, losing the information that custom IL will be emitted. Also, the simple-node predicate in the suggested replacement needs negation: simple operands should avoid spilling; nontrivial operands need the empty evaluation stack.

The special-case test was added in the previous commit: Converted_intrinsic_right_operand_can_emit_exception_regions. Its intrinsic conversion declares a parameter operand but emits a try/finally expression, and is wrapped in two ordinary conversions. It checks the result, evaluation trace, and operator call count.

Rebuilt Standard/Light for .NET 8, .NET 10, and Framework. Reran all 10 Standard / 11 Light targeted regression methods on .NET 8/10 with ARM64/x64 hosts: all pass, including the intrinsic test. This commit changes only the loop's readability; the full-suite results from f2c784e remain documented separately.

[dadhi]

thank you, merging