From d48a0641c880f78dc901ac8c7428e17a387988ea Mon Sep 17 00:00:00 2001 From: Sadik00789 Date: Wed, 16 Sep 2026 01:43:13 +0600 Subject: [PATCH] Fix #4136: Re-allow condition-slot comp!=0 simplification for non-IfInstruction operands --- .../PrettyTestRunner.cs | 7 +++ .../TestCases/Correctness/Comparisons.cs | 15 ++++++ .../TestCases/Pretty/Issue4136.cs | 54 +++++++++++++++++++ .../IL/Transforms/ExpressionTransforms.cs | 11 +++- 4 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue4136.cs diff --git a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs index 86c86cbb7..f8b7c5466 100644 --- a/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs +++ b/ICSharpCode.Decompiler.Tests/PrettyTestRunner.cs @@ -699,6 +699,13 @@ namespace ICSharpCode.Decompiler.Tests await RunForLibrary(cscOptions: cscOptions | CompilerOptions.NullableEnable); } + [Test] + public async Task Issue4136([ValueSource(nameof(roslyn3OrNewerOptions))] CompilerOptions cscOptions) + { + // `is null or 0` requires C# 9 (Roslyn 3.11). + await RunForLibrary(cscOptions: cscOptions); + } + [Test] public async Task Issue3452([ValueSource(nameof(roslyn4OrNewerOptions))] CompilerOptions cscOptions) { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/Comparisons.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/Comparisons.cs index 17b30df9c..802169b52 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/Comparisons.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/Comparisons.cs @@ -58,9 +58,24 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness IntBranchInConditionSlot(0, 0x100, true); NegatedIntBranchInConditionSlot(1, 0x100, false); NegatedIntBranchInConditionSlot(0, 0x100, true); + + Console.WriteLine("LiftedNullCoalescingComparison:"); + LiftedNullCoalescingComparison(null); + LiftedNullCoalescingComparison(0); + LiftedNullCoalescingComparison(1); return 0; } + // Issue #4136: a lifted null comparison over a `??` operand. Keeping the `??` form + // (instead of expanding to an explicit HasValue/GetValueOrDefault test) must not + // change the result, especially for the null case. + static void LiftedNullCoalescingComparison(int? value) + { + Console.WriteLine((value ?? 0) == 0); + Console.WriteLine((value ?? 1) == 0); + Console.WriteLine((value ?? 2) != 0); + } + static void TestFloatOp(string name, Func f) { float[] vals = { -1, 0, 3, float.PositiveInfinity, float.NaN }; diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue4136.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue4136.cs new file mode 100644 index 000000000..6dfaf4368 --- /dev/null +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Issue4136.cs @@ -0,0 +1,54 @@ +using System; + +namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty +{ + // https://github.com/icsharpcode/ILSpy/issues/4136 + // Lifted null comparisons over a `??` operand must keep the `??` form; they must not be + // expanded to an explicit `HasValue`/`GetValueOrDefault()` test wrapped in `? true : false`. + internal static class Issue4136 + { + // `data?.Length is null or 0` is lowered to the same guarded shape that + // SixLabors.ImageSharp.Formats.Png.PngEncoderCore.WriteXmpChunk uses: + // if (num.HasValue) if (num.GetValueOrDefault() != 0) goto false; goto true + // Before the fix this produced "(num.HasValue && num.GetValueOrDefault() != 0) ? false : true". + public static void LengthIsNullOrZero(byte[] data) + { +#if EXPECTED_OUTPUT + if ((data?.Length ?? 0) != 0 || 1 == 0) + { + Console.WriteLine(data.Length); + } +#else + if (data?.Length is null or 0) + { + return; + } + Console.WriteLine(data.Length); +#endif + } + + public static void CoalesceOneEqualZero(int? a) + { + if ((a ?? 1) == 0) + { + Console.WriteLine(); + } + } + + public static void CoalesceTwoEqualTwo(int? a) + { + if ((a ?? 2) == 2) + { + Console.WriteLine(); + } + } + + public static void CoalesceOneNotEqualZero(int? a) + { + if ((a ?? 1) != 0) + { + Console.WriteLine(); + } + } + } +} diff --git a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs index 1a29a00d9..78418363f 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs @@ -99,9 +99,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms else if (inst.Kind == ComparisonKind.Inequality && inst.LiftingKind == ComparisonLiftingKind.None && inst.Right.MatchLdcI4(0) && (inst.Left.InferType(context.TypeSystem).IsKnownType(KnownTypeCode.Boolean) - || inst.Left.MatchLdcI4(0) || inst.Left.MatchLdcI4(1))) + || inst.Left.MatchLdcI4(0) || inst.Left.MatchLdcI4(1) + // `x` is used only as a condition here, so its exact value does not matter, + // only whether it is zero or not. This preserves patterns such as + // `comp(call GetValueOrDefault() != 0)` that NullableLiftingTransform relies on. + // IfInstruction is excluded because an int-valued conditional (e.g. + // `(c ? flags : other)`, where only values 0/1 would be lossless) must not be + // turned into a bool, which would truncate it to 8 bits. + || (IfInstruction.IsInConditionSlot(inst) && inst.Left is not IfInstruction))) { - // When `x` is known to be 0 or 1: + // When `x` is known to be 0 or 1 (or is only tested for non-zero): // `comp(x != 0) => x` context.Step("Remove redundant comp(... != 0)", inst); inst.Left.AddILRange(inst);