[nullables] Extend nullable lifting to arbitrary combinations of 'conv' and 'binary.numeric'.

ICSharpCode.Decompiler.Tests/TestCases/Pretty/LiftedOperators.cs

@@ -814,6 +814,16 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 		{
 			return nf + nd + f;
 		}
+
+		static long? InArithmetic3(int? a, long? b, int? c, long d)
+		{
+			return a + b + c + d;
+		}
+
+		long? InReturnAfterArithmetic(int? a)
+		{
+			return a * a;
+		}
 	}
 
 	class LiftedExplicitConversions
@@ -843,4 +853,72 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 			}
 		}
 	}
+
+	class NullCoalescing
+	{
+		static void Print<T>(T x)
+		{
+			Console.WriteLine(x);
+		}
+
+		static void Objects(object a, object b)
+		{
+			Print(a ?? b);
+		}
+
+		static void Nullables(int? a, int? b)
+		{
+			Print(a ?? b);
+		}
+
+		static void NullableWithNonNullableFallback(int? a, int b)
+		{
+			Print(a ?? b);
+		}
+
+		static void Chain(int? a, int? b, int? c, int d)
+		{
+			Print(a ?? b ?? c ?? d);
+		}
+
+		static void ChainWithImplicitConversions(int? a, short? b, long? c, byte d)
+		{
+			Print(a ?? b ?? c ?? d);
+		}
+
+		static void ChainWithComputation(int? a, short? b, long? c, byte d)
+		{
+			Print(a + 1 ?? b + 2 ?? c + 3 ?? d + 4);
+		}
+
+		static object ReturnObjects(object a, object b)
+		{
+			return a ?? b;
+		}
+
+		static int? ReturnNullables(int? a, int? b)
+		{
+			return a ?? b;
+		}
+
+		static int ReturnNullableWithNonNullableFallback(int? a, int b)
+		{
+			return a ?? b;
+		}
+
+		static int? ReturnChain(int? a, int? b, int? c, int d)
+		{
+			return a ?? b ?? c ?? d;
+		}
+
+		static long? ReturnChainWithImplicitConversions(int? a, short? b, long? c, byte d)
+		{
+			return a ?? b ?? c ?? d;
+		}
+
+		static long? ReturnChainWithComputation(int? a, short? b, long? c, byte d)
+		{
+			return a + 1 ?? b + 2 ?? c + 3 ?? d + 4;
+		}
+	}
 }

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -107,6 +107,7 @@ namespace ICSharpCode.Decompiler.CSharp
 						new CachedDelegateInitialization(),
 						new ILInlining(),
 						new TransformAssignment(),
+						new NullableLiftingBlockTransform(),
 						new CopyPropagation(),
 						new LoopingBlockTransform(
 							// per-block transforms that depend on each other, and thus need to loop (fixpoint iteration).

ICSharpCode.Decompiler/DecompilerSettings.cs

@@ -102,7 +102,22 @@ namespace ICSharpCode.Decompiler
 				}
 			}
 		}
-		
+
+		bool liftNullables = true;
+
+		/// <summary>
+		/// Use lifted operators for nullables.
+		/// </summary>
+		public bool LiftNullables {
+			get { return liftNullables; }
+			set {
+				if (liftNullables != value) {
+					liftNullables = value;
+					OnPropertyChanged();
+				}
+			}
+		}
+
 		bool automaticProperties = true;
 		
 		/// <summary>

ICSharpCode.Decompiler/IL/Instructions/BinaryNumericInstruction.cs

@@ -72,18 +72,24 @@ namespace ICSharpCode.Decompiler.IL
 		/// If either input is null, the instruction evaluates to default(UnderlyingResultType?).
 		/// (this result type is underspecified, since there may be multiple C# types for the stack type)
 		/// </remarks>
-		public bool IsLifted { get; set; }
+		public bool IsLifted { get; }
 
 		readonly StackType resultType;
 
 		public BinaryNumericInstruction(BinaryNumericOperator op, ILInstruction left, ILInstruction right, bool checkForOverflow, Sign sign)
+			: this(op, left, right, left.ResultType, right.ResultType, checkForOverflow, sign)
+		{
+		}
+
+		public BinaryNumericInstruction(BinaryNumericOperator op, ILInstruction left, ILInstruction right, StackType leftInputType, StackType rightInputType, bool checkForOverflow, Sign sign, bool isLifted = false)
 			: base(OpCode.BinaryNumericInstruction, left, right)
 		{
 			this.CheckForOverflow = checkForOverflow;
 			this.Sign = sign;
 			this.Operator = op;
-			this.LeftInputType = left.ResultType;
-			this.RightInputType = right.ResultType;
+			this.LeftInputType = leftInputType;
+			this.RightInputType = rightInputType;
+			this.IsLifted = isLifted;
 			this.resultType = ComputeResultType(op, LeftInputType, RightInputType);
 			Debug.Assert(resultType != StackType.Unknown);
 		}

ICSharpCode.Decompiler/IL/Instructions/Block.cs

@@ -234,6 +234,19 @@ namespace ICSharpCode.Decompiler.IL
 				return null;
 			}
 		}
+
+		/// <summary>
+		/// If inst is a block consisting of a single instruction, returns that instruction.
+		/// Otherwise, returns the input instruction.
+		/// </summary>
+		public static ILInstruction Unwrap(ILInstruction inst)
+		{
+			if (inst is Block block) {
+				if (block.Instructions.Count == 1 && block.finalInstruction.MatchNop())
+					return block.Instructions[0];
+			}
+			return inst;
+		}
 	}
 	
 	public enum BlockType {

ICSharpCode.Decompiler/IL/Instructions/Conv.cs

@@ -103,7 +103,7 @@ namespace ICSharpCode.Decompiler.IL
 		/// If the value is null, the conversion evaluates to default(TargetType?).
 		/// (this result type is underspecified, since there may be multiple C# types for the TargetType)
 		/// </remarks>
-		public bool IsLifted { get; set; }
+		public bool IsLifted { get; }
 
 		/// <summary>
 		/// Gets the stack type of the input type.
@@ -139,7 +139,7 @@ namespace ICSharpCode.Decompiler.IL
 		{
 		}
 
-		public Conv(ILInstruction argument, StackType inputType, Sign inputSign, PrimitiveType targetType, bool checkForOverflow)
+		public Conv(ILInstruction argument, StackType inputType, Sign inputSign, PrimitiveType targetType, bool checkForOverflow, bool isLifted = false)
 			: base(OpCode.Conv, argument)
 		{
 			bool needsSign = checkForOverflow || targetType == PrimitiveType.R4 || targetType == PrimitiveType.R8;
@@ -149,6 +149,7 @@ namespace ICSharpCode.Decompiler.IL
 			this.TargetType = targetType;
 			this.CheckForOverflow = checkForOverflow;
 			this.Kind = GetConversionKind(targetType, this.InputType, this.InputSign);
+			this.IsLifted = isLifted;
 		}
 
 		internal override void CheckInvariant(ILPhase phase)

ICSharpCode.Decompiler/IL/Instructions/IfInstruction.cs

@@ -56,11 +56,12 @@ namespace ICSharpCode.Decompiler.IL
 		{
 			base.CheckInvariant(phase);
 			Debug.Assert(condition.ResultType == StackType.I4);
+			Debug.Assert(trueInst.ResultType == falseInst.ResultType);
 		}
 		
 		public override StackType ResultType {
 			get {
-				return CommonResultType(trueInst.ResultType, falseInst.ResultType);
+				return trueInst.ResultType;
 			}
 		}
 		

ICSharpCode.Decompiler/IL/Instructions/NullCoalescingInstruction.cs

@@ -16,20 +16,23 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 
-using System;
-using System.Collections.Generic;
-using System.Threading;
-using ICSharpCode.Decompiler.IL.Transforms;
-using Mono.Cecil;
-using ICSharpCode.Decompiler.Disassembler;
-using System.Linq;
-using ICSharpCode.Decompiler.TypeSystem;
-using ICSharpCode.Decompiler.Util;
 using System.Diagnostics;
 
 namespace ICSharpCode.Decompiler.IL
 {
 	/// <summary>Null coalescing operator expression. <c>if.notnull(valueInst, fallbackInst)</c></summary>
+	/// <remarks>
+	/// This instruction can used in 3 different cases:
+	/// Case 1: both ValueInst and FallbackInst are of reference type.
+	///    Semantics: equivalent to "valueInst != null ? valueInst : fallbackInst",
+	///               except that valueInst is evaluated only once.
+	/// Case 2: both ValueInst and FallbackInst are of type Nullable{T}.
+	///    Semantics: equivalent to "valueInst.HasValue ? valueInst : fallbackInst",
+	///               except that valueInst is evaluated only once.
+	/// Case 3: ValueInst is Nullable{T}, but FallbackInst is non-nullable value type.
+	///    Semantics: equivalent to "valueInst.HasValue ? valueInst.Value : fallbackInst",
+	///               except that valueInst is evaluated only once.
+	/// </remarks>
 	partial class NullCoalescingInstruction
 	{
 		public NullCoalescingInstruction(ILInstruction valueInst, ILInstruction fallbackInst) : base(OpCode.NullCoalescingInstruction)
@@ -38,9 +41,15 @@ namespace ICSharpCode.Decompiler.IL
 			this.FallbackInst = fallbackInst;
 		}
 
+		internal override void CheckInvariant(ILPhase phase)
+		{
+			base.CheckInvariant(phase);
+			Debug.Assert(valueInst.ResultType == StackType.O || valueInst.ResultType == fallbackInst.ResultType);
+		}
+
 		public override StackType ResultType {
 			get {
-				return CommonResultType(valueInst.ResultType, fallbackInst.ResultType);
+				return fallbackInst.ResultType;
 			}
 		}
 
@@ -52,7 +61,9 @@ namespace ICSharpCode.Decompiler.IL
 
 		protected override InstructionFlags ComputeFlags()
 		{
-			return InstructionFlags.ControlFlow | SemanticHelper.CombineBranches(valueInst.Flags, fallbackInst.Flags);
+			// valueInst is always executed; fallbackInst only sometimes
+			return InstructionFlags.ControlFlow | valueInst.Flags
+				| SemanticHelper.CombineBranches(InstructionFlags.None, fallbackInst.Flags);
 		}
 
 		public override void WriteTo(ITextOutput output)
@@ -63,7 +74,6 @@ namespace ICSharpCode.Decompiler.IL
 			output.Write(", ");
 			fallbackInst.WriteTo(output);
 			output.Write(")");
-
 		}
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs

@@ -289,7 +289,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			base.VisitIfInstruction(inst);
 
 			inst = HandleConditionalOperator(inst);
-			NullableLiftingTransform.Run(inst, context);
+			new NullableLiftingTransform(context).Run(inst);
 		}
 
 		IfInstruction HandleConditionalOperator(IfInstruction inst)

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -276,7 +276,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			}
 			
 			var parent = loadInst.Parent;
-			if (parent is ILiftableInstruction liftable && liftable.IsLifted && NullableType.IsNullable(v.Type)) {
+			if (parent is ILiftableInstruction liftable && liftable.IsLifted) {
 				return true; // inline into lifted operators
 			}
 			// decide based on the target into which we are inlining

ICSharpCode.Decompiler/IL/Transforms/NullCoalescingTransform.cs

@@ -49,16 +49,23 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// </summary>
 		bool TransformNullCoalescing(Block block, int i)
 		{
-			if (i == 0) return false;
-			if (!(block.Instructions[i] is IfInstruction ifInstruction) || !(block.Instructions[i - 1] is StLoc stloc) || stloc.Variable.Kind != VariableKind.StackSlot)
+			if (i == 0)
 				return false;
-			if (!ifInstruction.Condition.MatchCompEquals(out var left, out var right) || !left.MatchLdLoc(stloc.Variable) || !right.MatchLdNull())
+			if (!(block.Instructions[i - 1] is StLoc stloc))
 				return false;
-			if (!ifInstruction.FalseInst.MatchNop() || !(ifInstruction.TrueInst is Block b) || b.Instructions.Count != 1 || !(b.Instructions[0] is StLoc fallbackStore) || fallbackStore.Variable != stloc.Variable)
+			if (stloc.Variable.Kind != VariableKind.StackSlot)
 				return false;
-			context.Step("TransformNullCoalescing", stloc);
-			stloc.Value = new NullCoalescingInstruction(stloc.Value, fallbackStore.Value);
-			return true;
+			if (!block.Instructions[i].MatchIfInstruction(out var condition, out var trueInst))
+				return false;
+			trueInst = Block.Unwrap(trueInst);
+			if (condition.MatchCompEquals(out var left, out var right) && left.MatchLdLoc(stloc.Variable) && right.MatchLdNull()
+				&& trueInst.MatchStLoc(stloc.Variable, out var fallbackValue)
+			) {
+				context.Step("TransformNullCoalescing", stloc);
+				stloc.Value = new NullCoalescingInstruction(stloc.Value, fallbackValue);
+				return true; // returning true removes the if instruction
+			}
+			return false;
 		}
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs

@@ -16,88 +16,192 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 // DEALINGS IN THE SOFTWARE.
 
+using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using ICSharpCode.Decompiler.TypeSystem;
 
 namespace ICSharpCode.Decompiler.IL.Transforms
 {
-	class NullableLiftingTransform
+	/// <summary>
+	/// Nullable lifting gets run in two places:
+	///  * the usual form looks at an if-else, and runs within the ExpressionTransforms
+	///  * the NullableLiftingBlockTransform handles the cases where Roslyn generates
+	///    two 'ret' statements for the null/non-null cases of a lifted operator.
+	/// </summary>
+	struct NullableLiftingTransform
 	{
-		public static void Run(IfInstruction inst, ILTransformContext context)
+		readonly ILTransformContext context;
+		List<ILVariable> nullableVars;
+
+		public NullableLiftingTransform(ILTransformContext context)
 		{
-			// if (call Nullable<inputUType>.get_HasValue(ldloca v))
-			//          newobj Nullable<utype>.ctor(...)
-			// else
-			//          default.value System.Nullable<utype>[[System.Int32]]
-			if (MatchHasValueCall(inst.Condition, out var v)
-				&& MatchNullableCtor(inst.TrueInst, out var utype, out var arg)
-				&& MatchNull(inst.FalseInst, utype))
-			{
-				ILInstruction lifted;
-				if (MatchGetValueOrDefault(arg, v)) {
-					// v != null ? call GetValueOrDefault(ldloca v) : null
-					// => conv.nop.lifted(ldloc v)
-					// This case is handled separately from LiftUnary() because
-					// that doesn't introduce nop-conversions.
-					context.Step("if => conv.nop.lifted", inst);
-					var inputUType = NullableType.GetUnderlyingType(v.Type);
-					lifted = new Conv(new LdLoc(v), inputUType.GetStackType(), inputUType.GetSign(), utype.ToPrimitiveType(), false) {
-						IsLifted = true,
-						ILRange = inst.ILRange
-					};
-				} else {
-					lifted = LiftUnary(arg, v, context);
-				}
-				if (lifted != null) {
-					Debug.Assert(IsLifted(lifted));
-					inst.ReplaceWith(lifted);
-				}
-			}
+			this.context = context;
+			this.nullableVars = null;
 		}
 
+		#region Run
 		/// <summary>
-		/// Lifting for unary expressions.
-		/// Recurses into the instruction and checks that everything can be lifted,
-		/// and that the chain ends with `call GetValueOrDefault(ldloca inputVar)`.
+		/// Main entry point into the normal code path of this transform.
+		/// Called by expression transform.
 		/// </summary>
-		static ILInstruction LiftUnary(ILInstruction inst, ILVariable inputVar, ILTransformContext context)
+		public bool Run(IfInstruction ifInst)
+		{
+			if (!context.Settings.LiftNullables)
+				return false;
+			// Detect pattern:
+			// if (condition)
+			//      newobj Nullable<utype>..ctor(exprToLift)
+			// else
+			//      default.value System.Nullable<utype>
+			if (!AnalyzeTopLevelCondition(ifInst.Condition, out bool negativeCondition))
+				return false;
+			ILInstruction trueInst = negativeCondition ? ifInst.FalseInst : ifInst.TrueInst;
+			ILInstruction falseInst = negativeCondition ? ifInst.TrueInst : ifInst.FalseInst;
+			var lifted = Lift(ifInst, trueInst, falseInst);
+			if (lifted != null) {
+				ifInst.ReplaceWith(lifted);
+				return true;
+			}
+			return false;
+		}
+
+		public bool RunBlock(Block block)
 		{
-			if (MatchGetValueOrDefault(inst, inputVar)) {
-				// We found the end: the whole chain can be lifted.
-				context.Step("NullableLiftingTransform.LiftUnary", inst);
+			if (!context.Settings.LiftNullables)
+				return false;
+			//  if (!condition) Block {
+			//    leave IL_0000 (default.value System.Nullable`1[[System.Int64]])
+			//  }
+			//  leave IL_0000 (newobj .ctor(exprToLift))
+			IfInstruction ifInst;
+			if (block.Instructions.Last() is Leave elseLeave) {
+				ifInst = block.Instructions.SecondToLastOrDefault() as IfInstruction;
+				if (ifInst == null || !ifInst.FalseInst.MatchNop())
+					return false;
+			} else {
+				return false;
+			}
+			if (!(Block.Unwrap(ifInst.TrueInst) is Leave thenLeave))
+				return false;
+			if (elseLeave.TargetContainer != thenLeave.TargetContainer)
+				return false;
+			if (!AnalyzeTopLevelCondition(ifInst.Condition, out bool negativeCondition))
+				return false;
+			ILInstruction trueInst = negativeCondition ? elseLeave.Value : thenLeave.Value;
+			ILInstruction falseInst = negativeCondition ? thenLeave.Value : elseLeave.Value;
+			var lifted = Lift(ifInst, trueInst, falseInst);
+			if (lifted != null) {
+				thenLeave.Value = lifted;
+				ifInst.ReplaceWith(thenLeave);
+				block.Instructions.Remove(elseLeave);
+				return true;
+			}
+			return false;
+		}
+		#endregion
+
+		#region AnalyzeCondition
+		bool AnalyzeTopLevelCondition(ILInstruction condition, out bool negativeCondition)
+		{
+			negativeCondition = false;
+			while (condition.MatchLogicNot(out var arg)) {
+				condition = arg;
+				negativeCondition = !negativeCondition;
+			}
+			return AnalyzeCondition(condition);
+		}
+
+		bool AnalyzeCondition(ILInstruction condition)
+		{
+			if (MatchHasValueCall(condition, out var v)) {
+				if (nullableVars == null)
+					nullableVars = new List<ILVariable>();
+				nullableVars.Add(v);
+				return true;
+			} else if (condition is BinaryNumericInstruction bitand) {
+				if (!(bitand.Operator == BinaryNumericOperator.BitAnd && bitand.ResultType == StackType.I4))
+					return false;
+				return AnalyzeCondition(bitand.Left) && AnalyzeCondition(bitand.Right);
+			}
+			return false;
+		}
+		#endregion
+
+		#region DoLift
+		ILInstruction Lift(IfInstruction ifInst, ILInstruction trueInst, ILInstruction falseInst)
+		{
+			if (!MatchNullableCtor(trueInst, out var utype, out var exprToLift))
+				return null;
+			if (!MatchNull(falseInst, utype))
+				return null;
+			ILInstruction lifted;
+			if (nullableVars.Count == 1 && MatchGetValueOrDefault(exprToLift, nullableVars[0])) {
+				// v != null ? call GetValueOrDefault(ldloca v) : null
+				// => conv.nop.lifted(ldloc v)
+				// This case is handled separately from DoLift() because
+				// that doesn't introduce nop-conversions.
+				context.Step("if => conv.nop.lifted", ifInst);
+				var inputUType = NullableType.GetUnderlyingType(nullableVars[0].Type);
+				lifted = new Conv(
+					new LdLoc(nullableVars[0]),
+					inputUType.GetStackType(), inputUType.GetSign(), utype.ToPrimitiveType(),
+					checkForOverflow: false,
+					isLifted: true
+				) {
+					ILRange = ifInst.ILRange
+				};
+			} else {
+				context.Step("NullableLiftingTransform.DoLift", ifInst);
+				lifted = DoLift(exprToLift);
+			}
+			if (lifted != null) {
+				Debug.Assert(lifted is ILiftableInstruction liftable && liftable.IsLifted
+					&& liftable.UnderlyingResultType == exprToLift.ResultType);
+			}
+			return lifted;
+		}
+
+		// Lifts the specified instruction.
+		// Creates a new lifted instruction without modifying the input instruction.
+		// If lifting fails, returns null.
+		ILInstruction DoLift(ILInstruction inst)
+		{
+			if (MatchGetValueOrDefault(inst, out ILVariable inputVar) && nullableVars.Contains(inputVar)) {
+				// n.GetValueOrDefault() lifted => n.
 				return new LdLoc(inputVar) { ILRange = inst.ILRange };
 			} else if (inst is Conv conv) {
-				var arg = LiftUnary(conv.Argument, inputVar, context);
+				var arg = DoLift(conv.Argument);
 				if (arg != null) {
-					conv.Argument = arg;
-					conv.IsLifted = true;
-					return conv;
+					return new Conv(arg, conv.InputType, conv.InputSign, conv.TargetType, conv.CheckForOverflow, isLifted: true) {
+						ILRange = conv.ILRange
+					};
 				}
 			} else if (inst is BinaryNumericInstruction binary) {
-				if (SemanticHelper.IsPure(binary.Right.Flags)) {
-					var arg = LiftUnary(binary.Left, inputVar, context);
-					if (arg != null) {
-						binary.Left = arg;
-						binary.IsLifted = true;
-						return binary;
-					}
+				var left = DoLift(binary.Left);
+				var right = DoLift(binary.Right);
+				if (left != null && right == null && SemanticHelper.IsPure(binary.Right.Flags)) {
+					// Embed non-nullable pure expression in lifted expression.
+					right = binary.Right.Clone();
 				}
-				if (SemanticHelper.IsPure(binary.Left.Flags)) {
-					var arg = LiftUnary(binary.Right, inputVar, context);
-					if (arg != null) {
-						binary.Right = arg;
-						binary.IsLifted = true;
-						return binary;
-					}
+				if (left == null && right != null && SemanticHelper.IsPure(binary.Left.Flags)) {
+					// Embed non-nullable pure expression in lifted expression.
+					left = binary.Left.Clone();
+				}
+				if (left != null && right != null) {
+					return new BinaryNumericInstruction(
+						binary.Operator, left, right,
+						binary.LeftInputType, binary.RightInputType,
+						binary.CheckForOverflow, binary.Sign,
+						isLifted: true
+					) {
+						ILRange = binary.ILRange
+					};
 				}
 			}
 			return null;
 		}
-
-		static bool IsLifted(ILInstruction inst)
-		{
-			return inst is ILiftableInstruction liftable && liftable.IsLifted;
-		}
+		#endregion
 
 		#region Match...Call
 		/// <summary>
@@ -186,4 +290,12 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		}
 		#endregion
 	}
+
+	class NullableLiftingBlockTransform : IBlockTransform
+	{
+		public void Run(Block block, BlockTransformContext context)
+		{
+			new NullableLiftingTransform(context).RunBlock(block);
+		}
+	}
 }