Skip to content

Commit

Permalink
Make NullableLiftingTransform handle the case where ExpressionTransfo…
Browse files Browse the repository at this point in the history
…rms.VisitComp already lifted a nullable comparison with constant.
  • Loading branch information
siegfriedpammer authored and matt committed Jul 30, 2024
1 parent a631dc8 commit b033d11
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 5 deletions.
5 changes: 4 additions & 1 deletion ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
31 changes: 27 additions & 4 deletions ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>() : default(T)
// => Activator.CreateInstance<T>()
return trueInst;
}
}
else
else if (!comp.IsLifted)
{
// Not (in)equality, but one of < <= > >=.
// Returns false unless all HasValue bits are true.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -449,6 +450,7 @@ struct CompOrDecimal
public ComparisonKind Kind;
public ILInstruction Left;
public ILInstruction Right;
public bool IsLifted;

public IType LeftExpectedType {
get {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -576,6 +580,24 @@ ILInstruction LiftCSharpEqualityComparison(CompOrDecimal valueComp, ComparisonKi
/// </summary>
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())
Expand Down Expand Up @@ -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))
Expand Down

0 comments on commit b033d11

Please sign in to comment.