Fix #4136: Re-allow condition-slot comp!=0 simplification for non-IfInstruction operands

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)
 		{

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<float, float, bool> f)
 		{
 			float[] vals = { -1, 0, 3, float.PositiveInfinity, float.NaN };

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();
+			}
+		}
+	}
+}

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);