[nullables] Add support for '??' operator on nullables.

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

@@ -854,7 +854,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 		}
 	}
 
-	class NullCoalescing
+	class NullCoalescingTests
 	{
 		static void Print<T>(T x)
 		{
@@ -876,6 +876,16 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 			Print(a ?? b);
 		}
 
+		static void NullableWithImplicitConversion(short? a, int? b)
+		{
+			Print(a ?? b);
+		}
+
+		static void NullableWithImplicitConversionAndNonNullableFallback(short? a, int b)
+		{
+			Print(a ?? b);
+		}
+
 		static void Chain(int? a, int? b, int? c, int d)
 		{
 			Print(a ?? b ?? c ?? d);
@@ -888,7 +898,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 
 		static void ChainWithComputation(int? a, short? b, long? c, byte d)
 		{
-			Print(a + 1 ?? b + 2 ?? c + 3 ?? d + 4);
+			Print((a + 1) ?? (b + 2) ?? (c + 3) ?? (d + 4));
 		}
 
 		static object ReturnObjects(object a, object b)
@@ -906,19 +916,19 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
 			return a ?? b;
 		}
 
-		static int? ReturnChain(int? a, int? b, int? c, int d)
+		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)
+		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)
+		static long ReturnChainWithComputation(int? a, short? b, long? c, byte d)
 		{
-			return a + 1 ?? b + 2 ?? c + 3 ?? d + 4;
+			return (a + 1) ?? (b + 2) ?? (c + 3) ?? (d + 4);
 		}
 	}
 }

ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@@ -1729,12 +1729,20 @@ namespace ICSharpCode.Decompiler.CSharp
 			if (rr.IsError) {
 				IType targetType;
 				if (!value.Type.Equals(SpecialType.NullType) && !fallback.Type.Equals(SpecialType.NullType) && !value.Type.Equals(fallback.Type)) {
-					targetType = compilation.FindType(inst.ResultType.ToKnownTypeCode());
+					targetType = compilation.FindType(inst.UnderlyingResultType.ToKnownTypeCode());
 				} else {
 					targetType = value.Type.Equals(SpecialType.NullType) ? fallback.Type : value.Type;
 				}
-				value = value.ConvertTo(targetType, this);
+				if (inst.Kind != NullCoalescingKind.Ref) {
-				fallback = fallback.ConvertTo(targetType, this);
+					value = value.ConvertTo(NullableType.Create(compilation, targetType), this);
+				} else {
+					value = value.ConvertTo(targetType, this);
+				}
+				if (inst.Kind == NullCoalescingKind.Nullable) {
+					value = value.ConvertTo(NullableType.Create(compilation, targetType), this);
+				} else {
+					fallback = fallback.ConvertTo(targetType, this);
+				}
 				rr = new ResolveResult(targetType);
 			}
 			return new BinaryOperatorExpression(value, BinaryOperatorType.NullCoalescing, fallback)

ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs

@@ -244,7 +244,11 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor
 			if (binaryOperatorExpression.Operator == BinaryOperatorType.NullCoalescing) {
 				if (InsertParenthesesForReadability) {
 					ParenthesizeIfRequired(binaryOperatorExpression.Left, Primary);
-					ParenthesizeIfRequired(binaryOperatorExpression.Right, Primary);
+					if (GetBinaryOperatorType(binaryOperatorExpression.Right) == BinaryOperatorType.NullCoalescing) {
+						ParenthesizeIfRequired(binaryOperatorExpression.Right, precedence);
+					} else {
+						ParenthesizeIfRequired(binaryOperatorExpression.Right, Primary);
+					}
 				} else {
 					// ?? is right-associative
 					ParenthesizeIfRequired(binaryOperatorExpression.Left, precedence + 1);

ICSharpCode.Decompiler/IL/Instructions.cs

@@ -1240,7 +1240,7 @@ namespace ICSharpCode.Decompiler.IL
 	/// <summary>Null coalescing operator expression. <c>if.notnull(valueInst, fallbackInst)</c></summary>
 	public sealed partial class NullCoalescingInstruction : ILInstruction
 	{
-		public static readonly SlotInfo ValueInstSlot = new SlotInfo("ValueInst");
+		public static readonly SlotInfo ValueInstSlot = new SlotInfo("ValueInst", canInlineInto: true);
 		ILInstruction valueInst;
 		public ILInstruction ValueInst {
 			get { return this.valueInst; }

ICSharpCode.Decompiler/IL/Instructions.tt

@@ -90,7 +90,7 @@
 		new OpCode("if.notnull", "Null coalescing operator expression. <c>if.notnull(valueInst, fallbackInst)</c>",
 			CustomClassName("NullCoalescingInstruction"),
 			CustomChildren(new []{
-				new ChildInfo("valueInst"),
+				new ChildInfo("valueInst") { CanInlineInto = true },
 				new ChildInfo("fallbackInst"),
 			}), CustomConstructor, CustomComputeFlags, CustomWriteTo),
 		new OpCode("switch", "Switch statement",

ICSharpCode.Decompiler/IL/Instructions/Conv.cs

@@ -149,6 +149,7 @@ namespace ICSharpCode.Decompiler.IL
 			this.TargetType = targetType;
 			this.CheckForOverflow = checkForOverflow;
 			this.Kind = GetConversionKind(targetType, this.InputType, this.InputSign);
+			Debug.Assert(Kind != ConversionKind.Invalid);
 			this.IsLifted = isLifted;
 		}
 

ICSharpCode.Decompiler/IL/Instructions/NullCoalescingInstruction.cs

@@ -20,23 +20,44 @@ using System.Diagnostics;
 
 namespace ICSharpCode.Decompiler.IL
 {
-	/// <summary>Null coalescing operator expression. <c>if.notnull(valueInst, fallbackInst)</c></summary>
+	/// <summary>
-	/// <remarks>
+	/// Kind of null-coalescing operator.
-	/// This instruction can used in 3 different cases:
+	/// ILAst: <c>if.notnull(valueInst, fallbackInst)</c>
-	/// Case 1: both ValueInst and FallbackInst are of reference type.
+	/// C#: <c>value ?? fallback</c>
-	///    Semantics: equivalent to "valueInst != null ? valueInst : fallbackInst",
+	/// </summary>
-	///               except that valueInst is evaluated only once.
+	public enum NullCoalescingKind
-	/// Case 2: both ValueInst and FallbackInst are of type Nullable{T}.
+	{
-	///    Semantics: equivalent to "valueInst.HasValue ? valueInst : fallbackInst",
+		/// <summary>
-	///               except that valueInst is evaluated only once.
+		/// Both ValueInst and FallbackInst are of reference type.
-	/// Case 3: ValueInst is Nullable{T}, but FallbackInst is non-nullable value type.
+		/// 
-	///    Semantics: equivalent to "valueInst.HasValue ? valueInst.Value : fallbackInst",
+		/// Semantics: equivalent to "valueInst != null ? valueInst : fallbackInst",
-	///               except that valueInst is evaluated only once.
+		///            except that valueInst is evaluated only once.
-	/// </remarks>
+		/// </summary>
+		Ref,
+		/// <summary>
+		/// Both ValueInst and FallbackInst are of type Nullable{T}.
+		/// 
+		/// Semantics: equivalent to "valueInst.HasValue ? valueInst : fallbackInst",
+		///            except that valueInst is evaluated only once.
+		/// </summary>
+		Nullable,
+		/// <summary>
+		/// 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.
+		/// </summary>
+		NullableWithValueFallback
+	}
+
 	partial class NullCoalescingInstruction
 	{
-		public NullCoalescingInstruction(ILInstruction valueInst, ILInstruction fallbackInst) : base(OpCode.NullCoalescingInstruction)
+		public readonly NullCoalescingKind Kind;
+		public StackType UnderlyingResultType = StackType.O;
+
+		public NullCoalescingInstruction(NullCoalescingKind kind, ILInstruction valueInst, ILInstruction fallbackInst) : base(OpCode.NullCoalescingInstruction)
 		{
+			this.Kind = kind;
 			this.ValueInst = valueInst;
 			this.FallbackInst = fallbackInst;
 		}
@@ -44,7 +65,9 @@ namespace ICSharpCode.Decompiler.IL
 		internal override void CheckInvariant(ILPhase phase)
 		{
 			base.CheckInvariant(phase);
-			Debug.Assert(valueInst.ResultType == StackType.O || valueInst.ResultType == fallbackInst.ResultType);
+			Debug.Assert(valueInst.ResultType == StackType.O); // lhs is reference type or nullable type
+			Debug.Assert(fallbackInst.ResultType == StackType.O || Kind == NullCoalescingKind.NullableWithValueFallback);
+			Debug.Assert(ResultType == UnderlyingResultType || Kind == NullCoalescingKind.Nullable);
 		}
 
 		public override StackType ResultType {

ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs

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

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -279,6 +279,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			if (parent is ILiftableInstruction liftable && liftable.IsLifted) {
 				return true; // inline into lifted operators
 			}
+			if (parent is NullCoalescingInstruction && NullableType.IsNullable(v.Type)) {
+				return true; // inline nullables into ?? operator
+			}
 			// decide based on the target into which we are inlining
 			switch (next.OpCode) {
 				case OpCode.Leave:

ICSharpCode.Decompiler/IL/Transforms/NullCoalescingTransform.cs

@@ -24,6 +24,12 @@ using System.Threading.Tasks;
 
 namespace ICSharpCode.Decompiler.IL.Transforms
 {
+	/// <summary>
+	/// Transform for constructing the NullCoalescingInstruction (if.notnull(a,b), or in C#: ??)
+	/// Note that this transform only handles the case where a,b are reference types.
+	/// 
+	/// The ?? operator for nullables is handled by NullableLiftingTransform.
+	/// </summary>
 	class NullCoalescingTransform : IBlockTransform
 	{
 		BlockTransformContext context;
@@ -32,7 +38,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		{
 			this.context = context;
 			for (int i = block.Instructions.Count - 1; i >= 0; i--) {
-				if (TransformNullCoalescing(block, i)) {
+				if (TransformRefTypes(block, i)) {
 					block.Instructions.RemoveAt(i);
 					continue;
 				}
@@ -40,6 +46,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		}
 
 		/// <summary>
+		/// Handles NullCoalescingInstruction case 1: reference types.
+		/// 
 		/// stloc s(valueInst)
 		/// if (comp(ldloc s == ldnull)) {
 		///		stloc s(fallbackInst)
@@ -47,7 +55,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		/// =>
 		/// stloc s(if.notnull(valueInst, fallbackInst))
 		/// </summary>
-		bool TransformNullCoalescing(Block block, int i)
+		bool TransformRefTypes(Block block, int i)
 		{
 			if (i == 0)
 				return false;
@@ -61,8 +69,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			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);
+				context.Step("NullCoalescingTransform (reference types)", stloc);
-				stloc.Value = new NullCoalescingInstruction(stloc.Value, fallbackValue);
+				stloc.Value = new NullCoalescingInstruction(NullCoalescingKind.Ref, stloc.Value, fallbackValue);
 				return true; // returning true removes the if instruction
 			}
 			return false;

ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs

@@ -26,9 +26,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 {
 	/// <summary>
 	/// Nullable lifting gets run in two places:
-	///  * the usual form looks at an if-else, and runs within the ExpressionTransforms
+	///  * 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.
+	/// 
+	/// The transform handles the following languages constructs:
+	///  * lifted conversions
+	///  * lifted unary and binary operators
+	///  * the ?? operator with type Nullable{T} on the left-hand-side
 	/// </summary>
 	struct NullableLiftingTransform
 	{
@@ -59,7 +64,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				return false;
 			ILInstruction trueInst = negativeCondition ? ifInst.FalseInst : ifInst.TrueInst;
 			ILInstruction falseInst = negativeCondition ? ifInst.TrueInst : ifInst.FalseInst;
-			var lifted = Lift(ifInst, trueInst, falseInst);
+			var lifted = Lift(trueInst, falseInst, ifInst.ILRange);
 			if (lifted != null) {
 				ifInst.ReplaceWith(lifted);
 				return true;
@@ -91,7 +96,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				return false;
 			ILInstruction trueInst = negativeCondition ? elseLeave.Value : thenLeave.Value;
 			ILInstruction falseInst = negativeCondition ? thenLeave.Value : elseLeave.Value;
-			var lifted = Lift(ifInst, trueInst, falseInst);
+			var lifted = Lift(trueInst, falseInst, ifInst.ILRange);
 			if (lifted != null) {
 				thenLeave.Value = lifted;
 				ifInst.ReplaceWith(thenLeave);
@@ -130,41 +135,73 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		#endregion
 
 		#region DoLift
-		ILInstruction Lift(IfInstruction ifInst, ILInstruction trueInst, ILInstruction falseInst)
+		ILInstruction Lift(ILInstruction trueInst, ILInstruction falseInst, Interval ilrange)
 		{
-			if (!MatchNullableCtor(trueInst, out var utype, out var exprToLift))
+			bool isNullCoalescingWithNonNullableFallback = false;
-				return null;
+			if (!MatchNullableCtor(trueInst, out var utype, out var exprToLift)) {
-			if (!MatchNull(falseInst, utype))
+				isNullCoalescingWithNonNullableFallback = true;
-				return null;
+				utype = context.TypeSystem.Compilation.FindType(trueInst.ResultType.ToKnownTypeCode());
+				exprToLift = trueInst;
+				if (nullableVars.Count == 1 && exprToLift.MatchLdLoc(nullableVars[0])) {
+					// v.HasValue ? ldloc v : fallback
+					// => v ?? fallback
+					context.Step("v.HasValue ? v : fallback => v ?? fallback", trueInst);
+					return new NullCoalescingInstruction(NullCoalescingKind.Nullable, trueInst, falseInst) {
+						UnderlyingResultType = NullableType.GetUnderlyingType(nullableVars[0].Type).GetStackType(),
+						ILRange = ilrange
+					};
+				}
+			}
 			ILInstruction lifted;
 			if (nullableVars.Count == 1 && MatchGetValueOrDefault(exprToLift, nullableVars[0])) {
-				// v != null ? call GetValueOrDefault(ldloca v) : null
+				// v.HasValue ? call GetValueOrDefault(ldloca v) : fallback
-				// => conv.nop.lifted(ldloc v)
+				// => conv.nop.lifted(ldloc v) ?? fallback
 				// This case is handled separately from DoLift() because
 				// that doesn't introduce nop-conversions.
-				context.Step("if => conv.nop.lifted", ifInst);
+				context.Step("v.HasValue ? v.GetValueOrDefault() : fallback => v ?? fallback", trueInst);
 				var inputUType = NullableType.GetUnderlyingType(nullableVars[0].Type);
-				lifted = new Conv(
+				lifted = new LdLoc(nullableVars[0]);
-					new LdLoc(nullableVars[0]),
+				if (!inputUType.Equals(utype) && utype.ToPrimitiveType() != PrimitiveType.None) {
-					inputUType.GetStackType(), inputUType.GetSign(), utype.ToPrimitiveType(),
+					// While the ILAst allows implicit conversions between short and int
-					checkForOverflow: false,
+					// (because both map to I4); it does not allow implicit conversions
-					isLifted: true
+					// between short? and int? (structs of different types).
-				) {
+					// So use 'conv.nop.lifted' to allow the conversion.
-					ILRange = ifInst.ILRange
+					lifted = new Conv(
-				};
+						lifted,
+						inputUType.GetStackType(), inputUType.GetSign(), utype.ToPrimitiveType(),
+						checkForOverflow: false,
+						isLifted: true
+					) {
+						ILRange = ilrange
+					};
+				}
 			} else {
-				context.Step("NullableLiftingTransform.DoLift", ifInst);
+				context.Step("NullableLiftingTransform.DoLift", trueInst);
 				BitSet bits;
 				(lifted, bits) = DoLift(exprToLift);
-				if (lifted != null && !bits.All(0, nullableVars.Count)) {
+				if (lifted == null) {
+					return null;
+				}
+				if (!bits.All(0, nullableVars.Count)) {
 					// don't lift if a nullableVar doesn't contribute to the result
-					lifted = null;
+					return null;
 				}
-			}
-			if (lifted != null) {
 				Debug.Assert(lifted is ILiftableInstruction liftable && liftable.IsLifted
 					&& liftable.UnderlyingResultType == exprToLift.ResultType);
 			}
+			if (isNullCoalescingWithNonNullableFallback) {
+				lifted = new NullCoalescingInstruction(NullCoalescingKind.NullableWithValueFallback, lifted, falseInst) {
+					UnderlyingResultType = exprToLift.ResultType,
+					ILRange = ilrange
+				};
+			} else if (!MatchNull(falseInst, utype)) {
+				// Normal lifting, but the falseInst isn't `default(utype?)`
+				// => use the `??` operator to provide the fallback value.
+				lifted = new NullCoalescingInstruction(NullCoalescingKind.Nullable, lifted, falseInst) {
+					UnderlyingResultType = exprToLift.ResultType,
+					ILRange = ilrange
+				};
+			}
 			return lifted;
 		}