diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LiftedOperators.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LiftedOperators.cs index 7e71bc397..b903c1f5b 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/LiftedOperators.cs +++ b/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 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); } } } diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 9b23742f6..8c36a5ee9 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/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); - fallback = fallback.ConvertTo(targetType, this); + if (inst.Kind != NullCoalescingKind.Ref) { + 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) diff --git a/ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs b/ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs index 43b1bc7d1..0e4068958 100644 --- a/ICSharpCode.Decompiler/CSharp/OutputVisitor/InsertParenthesesVisitor.cs +++ b/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); diff --git a/ICSharpCode.Decompiler/IL/Instructions.cs b/ICSharpCode.Decompiler/IL/Instructions.cs index 25a4e8f03..825fcafdd 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.cs +++ b/ICSharpCode.Decompiler/IL/Instructions.cs @@ -1240,7 +1240,7 @@ namespace ICSharpCode.Decompiler.IL /// Null coalescing operator expression. if.notnull(valueInst, fallbackInst) 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; } diff --git a/ICSharpCode.Decompiler/IL/Instructions.tt b/ICSharpCode.Decompiler/IL/Instructions.tt index 2ff5d34df..bc4a5b695 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.tt +++ b/ICSharpCode.Decompiler/IL/Instructions.tt @@ -90,7 +90,7 @@ new OpCode("if.notnull", "Null coalescing operator expression. if.notnull(valueInst, fallbackInst)", CustomClassName("NullCoalescingInstruction"), CustomChildren(new []{ - new ChildInfo("valueInst"), + new ChildInfo("valueInst") { CanInlineInto = true }, new ChildInfo("fallbackInst"), }), CustomConstructor, CustomComputeFlags, CustomWriteTo), new OpCode("switch", "Switch statement", diff --git a/ICSharpCode.Decompiler/IL/Instructions/Conv.cs b/ICSharpCode.Decompiler/IL/Instructions/Conv.cs index 52c9e6f6a..cce1c8b5e 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Conv.cs +++ b/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; } diff --git a/ICSharpCode.Decompiler/IL/Instructions/NullCoalescingInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/NullCoalescingInstruction.cs index ec670fc74..e71e69594 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/NullCoalescingInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/NullCoalescingInstruction.cs @@ -20,23 +20,44 @@ using System.Diagnostics; namespace ICSharpCode.Decompiler.IL { - /// Null coalescing operator expression. if.notnull(valueInst, fallbackInst) - /// - /// 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. - /// + /// + /// Kind of null-coalescing operator. + /// ILAst: if.notnull(valueInst, fallbackInst) + /// C#: value ?? fallback + /// + public enum NullCoalescingKind + { + /// + /// Both ValueInst and FallbackInst are of reference type. + /// + /// Semantics: equivalent to "valueInst != null ? valueInst : fallbackInst", + /// except that valueInst is evaluated only once. + /// + Ref, + /// + /// Both ValueInst and FallbackInst are of type Nullable{T}. + /// + /// Semantics: equivalent to "valueInst.HasValue ? valueInst : fallbackInst", + /// except that valueInst is evaluated only once. + /// + Nullable, + /// + /// 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. + /// + 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 { diff --git a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs index 834e8258e..942e387f9 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs +++ b/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) diff --git a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs index 52de04973..9dc8ec508 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs +++ b/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: diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullCoalescingTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullCoalescingTransform.cs index cd70c0302..078e1b34b 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullCoalescingTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullCoalescingTransform.cs @@ -24,6 +24,12 @@ using System.Threading.Tasks; namespace ICSharpCode.Decompiler.IL.Transforms { + /// + /// 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. + /// 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 } /// + /// 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)) /// - 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); - stloc.Value = new NullCoalescingInstruction(stloc.Value, fallbackValue); + context.Step("NullCoalescingTransform (reference types)", stloc); + stloc.Value = new NullCoalescingInstruction(NullCoalescingKind.Ref, stloc.Value, fallbackValue); return true; // returning true removes the if instruction } return false; diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs index db6c07dd6..1c942d80d 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs @@ -26,9 +26,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms { /// /// 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 /// 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)) - return null; - if (!MatchNull(falseInst, utype)) - return null; + bool isNullCoalescingWithNonNullableFallback = false; + if (!MatchNullableCtor(trueInst, out var utype, out var exprToLift)) { + isNullCoalescingWithNonNullableFallback = true; + 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 - // => conv.nop.lifted(ldloc v) + // v.HasValue ? call GetValueOrDefault(ldloca v) : fallback + // => 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( - new LdLoc(nullableVars[0]), - inputUType.GetStackType(), inputUType.GetSign(), utype.ToPrimitiveType(), - checkForOverflow: false, - isLifted: true - ) { - ILRange = ifInst.ILRange - }; + lifted = new LdLoc(nullableVars[0]); + if (!inputUType.Equals(utype) && utype.ToPrimitiveType() != PrimitiveType.None) { + // While the ILAst allows implicit conversions between short and int + // (because both map to I4); it does not allow implicit conversions + // between short? and int? (structs of different types). + // So use 'conv.nop.lifted' to allow the conversion. + 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; }