From b033d11f00af7157586aa68217f9d489a8e8cccf Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 20 Jul 2024 11:58:00 +0200 Subject: [PATCH] Make NullableLiftingTransform handle the case where ExpressionTransforms.VisitComp already lifted a nullable comparison with constant. --- .../IL/Transforms/ExpressionTransforms.cs | 5 ++- .../IL/Transforms/NullableLiftingTransform.cs | 31 ++++++++++++++++--- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs index b9b99ad427..c3b464399e 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs @@ -106,7 +106,10 @@ protected internal override void VisitComp(Comp inst) inst.Left.AcceptVisitor(this); return; } - new NullableLiftingTransform(context).Run(inst); + if (context.Settings.LiftNullables) + { + new NullableLiftingTransform(context).Run(inst); + } base.VisitComp(inst); if (inst.IsLifted) diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs index 3d517e4633..111cac3ce5 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs @@ -225,14 +225,14 @@ ILInstruction Lift(ILInstruction ifInst, ILInstruction condition, ILInstruction return LiftCSharpEqualityComparison(comp, ComparisonKind.Inequality, trueInst) ?? LiftCSharpUserEqualityComparison(comp, ComparisonKind.Inequality, trueInst); } - else if (IsGenericNewPattern(comp.Left, comp.Right, trueInst, falseInst)) + else if (!comp.IsLifted && IsGenericNewPattern(comp.Left, comp.Right, trueInst, falseInst)) { // (default(T) == null) ? Activator.CreateInstance() : default(T) // => Activator.CreateInstance() return trueInst; } } - else + else if (!comp.IsLifted) { // Not (in)equality, but one of < <= > >=. // Returns false unless all HasValue bits are true. @@ -401,11 +401,12 @@ static bool MatchCompOrDecimal(ILInstruction inst, out CompOrDecimal result) { result = default(CompOrDecimal); result.Instruction = inst; - if (inst is Comp comp && !comp.IsLifted) + if (inst is Comp comp) { result.Kind = comp.Kind; result.Left = comp.Left; result.Right = comp.Right; + result.IsLifted = comp.IsLifted; return true; } else if (inst is Call call && call.Method.IsOperator && call.Arguments.Count == 2 && !call.IsLifted) @@ -449,6 +450,7 @@ struct CompOrDecimal public ComparisonKind Kind; public ILInstruction Left; public ILInstruction Right; + public bool IsLifted; public IType LeftExpectedType { get { @@ -528,6 +530,8 @@ ILInstruction LiftCSharpEqualityComparison(CompOrDecimal valueComp, ComparisonKi // The HasValue comparison must be the same operator as the Value comparison. if (hasValueTest is Comp hasValueComp) { + if (valueComp.IsLifted || hasValueComp.IsLifted) + return null; // Comparing two nullables: HasValue comparison must be the same operator as the Value comparison if ((hasValueTestNegated ? hasValueComp.Kind.Negate() : hasValueComp.Kind) != newComparisonKind) return null; @@ -576,6 +580,24 @@ ILInstruction LiftCSharpEqualityComparison(CompOrDecimal valueComp, ComparisonKi /// ILInstruction LiftCSharpComparison(CompOrDecimal comp, ComparisonKind newComparisonKind) { + if (comp.IsLifted) + { + // Happens when legacy csc generates 'num.GetValueOrDefault() == const && num.HasValue', + // checking HasValue is redundant here, modern Roslyn versions optimize it and our + // NullableLifting transform on Comp will already lift the lhs of the logic.and. + // Treat this case as if the transform had not undone the optimization yet. + if (nullableVars.Count != 1) + { + return null; + } + + if (comp.Left.MatchLdLoc(nullableVars[0]) || comp.Right.MatchLdLoc(nullableVars[0])) + { + return comp.MakeLifted(newComparisonKind, comp.Left.Clone(), comp.Right.Clone()); + } + + return null; + } var (left, right, bits) = DoLiftBinary(comp.Left, comp.Right, comp.LeftExpectedType, comp.RightExpectedType); // due to the restrictions on side effects, we only allow instructions that are pure after lifting. // (we can't check this before lifting due to the calls to GetValueOrDefault()) @@ -611,7 +633,8 @@ ILInstruction LiftCSharpUserEqualityComparison(CompOrDecimal hasValueComp, Compa // call op_Inequality(call GetValueOrDefault(ldloca nullable1), call GetValueOrDefault(ldloca nullable2)) // else // ldc.i4 0 - + if (hasValueComp.IsLifted) + return null; if (!MatchHasValueCall(hasValueComp.Left, out ILVariable nullable1)) return null; if (!MatchHasValueCall(hasValueComp.Right, out ILVariable nullable2))