From 010abebcc979aea24f6991cd63cd8976b864085c Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 7 Mar 2020 23:00:19 +0100 Subject: [PATCH 1/8] Fix #1050: Implement TransformNullPropagationOnUnconstrainedGenericExpression --- .../IL/Transforms/NullPropagationTransform.cs | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs index 37f8ffcd3..d019f9b28 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs @@ -319,6 +319,152 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!context.Settings.NullPropagation) return; new NullPropagationTransform(context).RunStatements(block, pos); + if (TransformNullPropagationOnUnconstrainedGenericExpression(block, pos, out var target, out var value, out var rewrapPoint, out var endBlock)) { + context.Step("TransformNullPropagationOnUnconstrainedGenericExpression", block); + // A successful match was found: + // 1. The 'target' instruction, that is, the instruction where the actual 'null-propagating' call happens: + // .Call() is replaced with ?.Call() + // 2. The 'value' instruction, that is, the instruction that produces the value: + // It is inlined at the location of 'target'. + // 3. The 'rewrapPoint' instruction is an ancestor of the call that is used as location for the NullableRewrap instruction, + // if the expression does not yet contain a NullableRewrap. + + // First try to find a NullableRewrap instruction + bool needsRewrap = true; + var tmp = target; + while (needsRewrap && tmp != null) { + if (tmp is NullableRewrap) { + needsRewrap = false; + break; + } + tmp = tmp.Parent; + } + // Remove the fallback conditions and blocks + block.Instructions.RemoveRange(pos, 3); + // inline value and wrap it in a NullableUnwrap instruction to produce 'value?'. + target.ReplaceWith(new NullableUnwrap(value.ResultType, value, refInput: true)); + // if the ancestors do not yet have a NullableRewrap instruction + // insert it at the 'rewrapPoint'. + if (needsRewrap) { + var siblings = rewrapPoint.Parent.Children; + int index = rewrapPoint.ChildIndex; + siblings[index] = new NullableRewrap(rewrapPoint); + } + // If the endBlock is only reachable through the current block, + // combine both blocks. + if (endBlock.IncomingEdgeCount == 1) { + block.Instructions.AddRange(endBlock.Instructions); + block.Instructions.RemoveAt(pos + 1); + endBlock.Remove(); + } + } + } + + // stloc valueTemporary(valueExpression) + // stloc defaultTemporary(default.value type) + // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock { + // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) + // stloc valueTemporary(ldloca defaultTemporary) + // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock2 { + // stloc resultTemporary(ldnull) + // br endBlock + // } + // } + // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) + // br endBlock + // => + // stloc resultTemporary(nullable.rewrap(constrained[type].call_instruction(nullable.unwrap(valueExpression), ...))) + private bool TransformNullPropagationOnUnconstrainedGenericExpression(Block block, int pos, + out ILInstruction target, out ILInstruction value, out ILInstruction rewrapPoint, out Block endBlock) + { + target = null; + value = null; + rewrapPoint = null; + endBlock = null; + if (pos + 4 >= block.Instructions.Count) + return false; + // stloc valueTemporary(valueExpression) + if (!(block.Instructions[pos].MatchStLoc(out var valueTemporary, out value))) + return false; + if (!(valueTemporary.Kind == VariableKind.StackSlot && valueTemporary.LoadCount == 2 && valueTemporary.StoreCount == 2)) + return false; + // stloc defaultTemporary(default.value type) + if (!(block.Instructions[pos + 1].MatchStLoc(out var defaultTemporary, out var defaultExpression) && defaultExpression.MatchDefaultValue(out var type))) + return false; + // Some variables are reused by the compiler therefore we cannot check for absolute numbers. See for example ValueTuple`8.ToString + // LoadCount must be == StoreCount and LoadCount must be 2x AddressCount. + // Note: if successful, this transform removes exactly 2 store, 2 load and 1 addressof instruction. + if (!(defaultTemporary.Kind == VariableKind.Local && defaultTemporary.LoadCount == defaultTemporary.StoreCount && defaultTemporary.AddressCount * 2 == defaultTemporary.StoreCount)) + return false; + // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock + if (!(block.Instructions[pos + 2].MatchIfInstruction(out var condition, out var fallbackBlock1) && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) + return false; + // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) + if (!(block.Instructions[pos + 3].MatchStLoc(out var resultTemporary, out rewrapPoint))) + return false; + var loadInCall = FindLoadInExpression(valueTemporary, rewrapPoint); + if (!(loadInCall != null && loadInCall.Ancestors.OfType().FirstOrDefault() is CallInstruction call)) + return false; + if (!(call.Arguments.Count > 0 && loadInCall.IsDescendantOf(call.Arguments[0]))) + return false; + // br IL_0035 + if (!(block.Instructions[pos + 4].MatchBranch(out endBlock))) + return false; + // Analyze Block fallbackBlock + if (!(fallbackBlock1 is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, resultTemporary, endBlock))) + return false; + target = call.Arguments[0]; + return true; + + ILInstruction FindLoadInExpression(ILVariable variable, ILInstruction expression) + { + foreach (var load in variable.LoadInstructions) { + if (load.IsDescendantOf(expression)) + return load; + } + return null; + } + } + + // Block fallbackBlock { + // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) + // stloc valueTemporary(ldloca defaultTemporary) + // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock { + // stloc resultTemporary(ldnull) + // br endBlock + // } + // } + private bool IsFallbackBlock(Block block, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILVariable resultTemporary, Block endBlock) + { + if (!(block.Instructions.Count == 3)) + return false; + // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) + if (!(block.Instructions[0].MatchStLoc(defaultTemporary, out var valueExpression))) + return false; + if (!(valueExpression.MatchLdObj(out var target, out var t) && type.Equals(t) && target.MatchLdLoc(valueTemporary))) + return false; + // stloc valueTemporary(ldloca defaultTemporary) + if (!(block.Instructions[1].MatchStLoc(valueTemporary, out var defaultAddress) && defaultAddress.MatchLdLoca(defaultTemporary))) + return false; + // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock + if (!(block.Instructions[2].MatchIfInstruction(out var condition, out var tmp) && tmp is Block fallbackBlock && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) + return false; + // Block fallbackBlock { + // stloc resultTemporary(ldnull) + // br endBlock + // } + if (!(fallbackBlock.Instructions.Count == 2)) + return false; + if (!(fallbackBlock.Instructions[0].MatchStLoc(resultTemporary, out var defaultValue) && MatchDefaultValueOrLdNull(defaultValue))) + return false; + if (!fallbackBlock.Instructions[1].MatchBranch(endBlock)) + return false; + return true; + } + + private bool MatchDefaultValueOrLdNull(ILInstruction inst) + { + return inst.MatchLdNull() || inst.MatchDefaultValue(out _); } } } From a7d1d8fad7756ecad7eb5a87ba4eac8ecaccb002 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sat, 14 Mar 2020 21:27:52 +0100 Subject: [PATCH 2/8] TransformNullPropagationOnUnconstrainedGenericExpression: handle pattern that uses leave instructions instead of stloc into a temporary. --- .../TestCases/Pretty/NullPropagation.cs | 21 +++- .../IL/Transforms/NullPropagationTransform.cs | 110 ++++++++++++++---- 2 files changed, 103 insertions(+), 28 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs index 6d2191833..33c4617de 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs @@ -69,6 +69,17 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty } } + private struct GenericStruct + { + public T1 Field1; + public T2 Field2; + + public override string ToString() + { + return "(" + Field1?.ToString() + ", " + Field2?.ToString() + ")"; + } + } + public interface ITest { int Int(); @@ -243,12 +254,10 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty return t?.Int(); } - // See also: https://github.com/icsharpcode/ILSpy/issues/1050 - // The C# compiler generates pretty weird code in this case. - //private static int? GenericRefUnconstrainedInt(ref T t) where T : ITest - //{ - // return t?.Int(); - //} + private static int? GenericRefUnconstrainedInt(ref T t) where T : ITest + { + return t?.Int(); + } private static int? GenericRefClassConstraintInt(ref T t) where T : class, ITest { diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs index d019f9b28..98fcd8cf1 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs @@ -343,16 +343,21 @@ namespace ICSharpCode.Decompiler.IL.Transforms block.Instructions.RemoveRange(pos, 3); // inline value and wrap it in a NullableUnwrap instruction to produce 'value?'. target.ReplaceWith(new NullableUnwrap(value.ResultType, value, refInput: true)); + + var siblings = rewrapPoint.Parent.Children; + int index = rewrapPoint.ChildIndex; + // remove Nullable-ctor, if necessary + if (NullableLiftingTransform.MatchNullableCtor(rewrapPoint, out var utype, out var arg) && arg.InferType(context.TypeSystem).Equals(utype)) { + rewrapPoint = arg; + } // if the ancestors do not yet have a NullableRewrap instruction // insert it at the 'rewrapPoint'. if (needsRewrap) { - var siblings = rewrapPoint.Parent.Children; - int index = rewrapPoint.ChildIndex; siblings[index] = new NullableRewrap(rewrapPoint); } - // If the endBlock is only reachable through the current block, + // if the endBlock is only reachable through the current block, // combine both blocks. - if (endBlock.IncomingEdgeCount == 1) { + if (endBlock?.IncomingEdgeCount == 1) { block.Instructions.AddRange(endBlock.Instructions); block.Instructions.RemoveAt(pos + 1); endBlock.Remove(); @@ -374,6 +379,21 @@ namespace ICSharpCode.Decompiler.IL.Transforms // br endBlock // => // stloc resultTemporary(nullable.rewrap(constrained[type].call_instruction(nullable.unwrap(valueExpression), ...))) + // + // -or- + // + // stloc valueTemporary(valueExpression) + // stloc defaultTemporary(default.value type) + // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock { + // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) + // stloc valueTemporary(ldloca defaultTemporary) + // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock2 { + // leave(ldnull) + // } + // } + // leave (constrained[type].call_instruction(ldloc valueTemporary, ...)) + // => + // leave (nullable.rewrap(constrained[type].call_instruction(nullable.unwrap(valueExpression), ...))) private bool TransformNullPropagationOnUnconstrainedGenericExpression(Block block, int pos, out ILInstruction target, out ILInstruction value, out ILInstruction rewrapPoint, out Block endBlock) { @@ -381,7 +401,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms value = null; rewrapPoint = null; endBlock = null; - if (pos + 4 >= block.Instructions.Count) + if (pos + 3 >= block.Instructions.Count) return false; // stloc valueTemporary(valueExpression) if (!(block.Instructions[pos].MatchStLoc(out var valueTemporary, out value))) @@ -399,13 +419,30 @@ namespace ICSharpCode.Decompiler.IL.Transforms // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock if (!(block.Instructions[pos + 2].MatchIfInstruction(out var condition, out var fallbackBlock1) && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) return false; + if (!MatchStLocResultTemporary(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out rewrapPoint, out var call, out endBlock) && !MatchLeaveResult(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out rewrapPoint, out call)) + return false; + target = call.Arguments[0]; + return true; + } + + // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) + // br IL_0035 + private bool MatchStLocResultTemporary(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock1, out ILInstruction rewrapPoint, out CallInstruction call, out Block endBlock) + { + call = null; + endBlock = null; + rewrapPoint = null; + + if (pos + 4 >= block.Instructions.Count) + return false; + // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) if (!(block.Instructions[pos + 3].MatchStLoc(out var resultTemporary, out rewrapPoint))) return false; var loadInCall = FindLoadInExpression(valueTemporary, rewrapPoint); - if (!(loadInCall != null && loadInCall.Ancestors.OfType().FirstOrDefault() is CallInstruction call)) + if (!(loadInCall != null && loadInCall.Ancestors.OfType().FirstOrDefault() is CallInstruction c)) return false; - if (!(call.Arguments.Count > 0 && loadInCall.IsDescendantOf(call.Arguments[0]))) + if (!(c.Arguments.Count > 0 && loadInCall.IsDescendantOf(c.Arguments[0]))) return false; // br IL_0035 if (!(block.Instructions[pos + 4].MatchBranch(out endBlock))) @@ -413,17 +450,40 @@ namespace ICSharpCode.Decompiler.IL.Transforms // Analyze Block fallbackBlock if (!(fallbackBlock1 is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, resultTemporary, endBlock))) return false; - target = call.Arguments[0]; + + call = c; return true; + } - ILInstruction FindLoadInExpression(ILVariable variable, ILInstruction expression) - { - foreach (var load in variable.LoadInstructions) { - if (load.IsDescendantOf(expression)) - return load; - } - return null; + private bool MatchLeaveResult(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction rewrapPoint, out CallInstruction call) + { + call = null; + rewrapPoint = null; + + // leave (constrained[type].call_instruction(ldloc valueTemporary, ...)) + if (!(block.Instructions[pos + 3] is Leave leave && leave.IsLeavingFunction)) + return false; + rewrapPoint = leave.Value; + var loadInCall = FindLoadInExpression(valueTemporary, rewrapPoint); + if (!(loadInCall != null && loadInCall.Ancestors.OfType().FirstOrDefault() is CallInstruction c)) + return false; + if (!(c.Arguments.Count > 0 && loadInCall.IsDescendantOf(c.Arguments[0]))) + return false; + // Analyze Block fallbackBlock + if (!(fallbackBlock is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, null, leave.TargetContainer))) + return false; + + call = c; + return true; + } + + private ILInstruction FindLoadInExpression(ILVariable variable, ILInstruction expression) + { + foreach (var load in variable.LoadInstructions) { + if (load.IsDescendantOf(expression)) + return load; } + return null; } // Block fallbackBlock { @@ -434,7 +494,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms // br endBlock // } // } - private bool IsFallbackBlock(Block block, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILVariable resultTemporary, Block endBlock) + private bool IsFallbackBlock(Block block, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILVariable resultTemporary, ILInstruction endBlockOrLeaveContainer) { if (!(block.Instructions.Count == 3)) return false; @@ -447,18 +507,24 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!(block.Instructions[1].MatchStLoc(valueTemporary, out var defaultAddress) && defaultAddress.MatchLdLoca(defaultTemporary))) return false; // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock - if (!(block.Instructions[2].MatchIfInstruction(out var condition, out var tmp) && tmp is Block fallbackBlock && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) + if (!(block.Instructions[2].MatchIfInstruction(out var condition, out var tmp) && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) return false; // Block fallbackBlock { // stloc resultTemporary(ldnull) // br endBlock // } - if (!(fallbackBlock.Instructions.Count == 2)) - return false; - if (!(fallbackBlock.Instructions[0].MatchStLoc(resultTemporary, out var defaultValue) && MatchDefaultValueOrLdNull(defaultValue))) - return false; - if (!fallbackBlock.Instructions[1].MatchBranch(endBlock)) + var fallbackInst = Block.Unwrap(tmp); + if (fallbackInst is Block fallbackBlock && endBlockOrLeaveContainer is Block endBlock) { + if (!(fallbackBlock.Instructions.Count == 2)) + return false; + if (!(fallbackBlock.Instructions[0].MatchStLoc(resultTemporary, out var defaultValue) && MatchDefaultValueOrLdNull(defaultValue))) + return false; + if (!fallbackBlock.Instructions[1].MatchBranch(endBlock)) + return false; + } else if (!(fallbackInst is Leave fallbackLeave && endBlockOrLeaveContainer is BlockContainer leaveContainer + && fallbackLeave.TargetContainer == leaveContainer && MatchDefaultValueOrLdNull(fallbackLeave.Value))) return false; + return true; } From 199d38b85a2fec29623fb71cc1eaa4a0997629ea Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Wed, 8 Apr 2020 17:32:06 +0200 Subject: [PATCH 3/8] Add more test cases --- .../TestCases/Pretty/NullPropagation.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs index 33c4617de..efb9ec366 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs @@ -68,16 +68,42 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty { } } + + private class Container + { + public GenericStruct Other; + } private struct GenericStruct { public T1 Field1; public T2 Field2; + public Container Other; public override string ToString() { return "(" + Field1?.ToString() + ", " + Field2?.ToString() + ")"; } + + public int? GetTextLength() + { + return Field1?.ToString().Length + Field2?.ToString().Length + 4; + } + + public string Chain1() + { + return Other?.Other.Other?.Other.Field1?.ToString(); + } + + public string Chain2() + { + return Other?.Other.Other?.Other.Field1?.ToString()?.GetType().Name; + } + + public int? GetTextLengthNRE() + { + return (Field1?.ToString()).Length; + } } public interface ITest From 1303c540868b6749c5e676415604747776aa532c Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 26 Apr 2020 12:04:02 +0200 Subject: [PATCH 4/8] Exactly match load, store and address counts of defaultTemporary. --- .../IL/Transforms/NullPropagationTransform.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs index 98fcd8cf1..63b3c5752 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs @@ -411,10 +411,8 @@ namespace ICSharpCode.Decompiler.IL.Transforms // stloc defaultTemporary(default.value type) if (!(block.Instructions[pos + 1].MatchStLoc(out var defaultTemporary, out var defaultExpression) && defaultExpression.MatchDefaultValue(out var type))) return false; - // Some variables are reused by the compiler therefore we cannot check for absolute numbers. See for example ValueTuple`8.ToString - // LoadCount must be == StoreCount and LoadCount must be 2x AddressCount. - // Note: if successful, this transform removes exactly 2 store, 2 load and 1 addressof instruction. - if (!(defaultTemporary.Kind == VariableKind.Local && defaultTemporary.LoadCount == defaultTemporary.StoreCount && defaultTemporary.AddressCount * 2 == defaultTemporary.StoreCount)) + // In the above pattern the defaultTemporary variable is used two times in stloc and ldloc instructions and once in a ldloca instruction + if (!(defaultTemporary.Kind == VariableKind.Local && defaultTemporary.LoadCount == 2 && defaultTemporary.StoreCount == 2 && defaultTemporary.AddressCount == 1)) return false; // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock if (!(block.Instructions[pos + 2].MatchIfInstruction(out var condition, out var fallbackBlock1) && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) From effc49c479e3cb9058cc31b1643df4a46b7c3e00 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 26 Apr 2020 14:29:11 +0200 Subject: [PATCH 5/8] Move TransformNullPropagationOnUnconstrainedGenericExpression into NullPropagationTransform and make use of IsValidAccessChain --- .../IL/Transforms/NullPropagationTransform.cs | 161 +++++++----------- 1 file changed, 64 insertions(+), 97 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs index 63b3c5752..5c4660016 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs @@ -39,7 +39,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms } readonly ILTransformContext context; - + public NullPropagationTransform(ILTransformContext context) { this.context = context; @@ -142,17 +142,46 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// internal void RunStatements(Block block, int pos) { - var ifInst = block.Instructions[pos] as IfInstruction; - if (ifInst == null || !ifInst.FalseInst.MatchNop()) - return; - if (ifInst.Condition is Comp comp && comp.Kind == ComparisonKind.Inequality - && comp.Left.MatchLdLoc(out var testedVar) && comp.Right.MatchLdNull()) { - TryNullPropForVoidCall(testedVar, Mode.ReferenceType, ifInst.TrueInst as Block, ifInst); - } else if (NullableLiftingTransform.MatchHasValueCall(ifInst.Condition, out ILInstruction arg)) { - if (arg.MatchLdLoca(out testedVar)) { - TryNullPropForVoidCall(testedVar, Mode.NullableByValue, ifInst.TrueInst as Block, ifInst); - } else if (arg.MatchLdLoc(out testedVar)) { - TryNullPropForVoidCall(testedVar, Mode.NullableByReference, ifInst.TrueInst as Block, ifInst); + if (block.Instructions[pos] is IfInstruction ifInst && ifInst.FalseInst.MatchNop()) { + if (ifInst.Condition is Comp comp && comp.Kind == ComparisonKind.Inequality + && comp.Left.MatchLdLoc(out var testedVar) && comp.Right.MatchLdNull()) { + TryNullPropForVoidCall(testedVar, Mode.ReferenceType, ifInst.TrueInst as Block, ifInst); + } else if (NullableLiftingTransform.MatchHasValueCall(ifInst.Condition, out ILInstruction arg)) { + if (arg.MatchLdLoca(out testedVar)) { + TryNullPropForVoidCall(testedVar, Mode.NullableByValue, ifInst.TrueInst as Block, ifInst); + } else if (arg.MatchLdLoc(out testedVar)) { + TryNullPropForVoidCall(testedVar, Mode.NullableByReference, ifInst.TrueInst as Block, ifInst); + } + } + } + if (TransformNullPropagationOnUnconstrainedGenericExpression(block, pos, out var target, out var value, out var rewrapPoint, out var endBlock)) { + context.Step("TransformNullPropagationOnUnconstrainedGenericExpression", block); + // A successful match was found: + // 1. The 'target' instruction, that is, the instruction where the actual 'null-propagating' call happens: + // .Call() is replaced with ?.Call() + // 2. The 'value' instruction, that is, the instruction that produces the value: + // It is inlined at the location of 'target'. + // 3. The 'rewrapPoint' instruction is an ancestor of the call that is used as location for the NullableRewrap instruction + + // Remove the fallback conditions and blocks + block.Instructions.RemoveRange(pos, 3); + // inline value and wrap it in a NullableUnwrap instruction to produce 'value?'. + target.ReplaceWith(new NullableUnwrap(value.ResultType, value, refInput: true)); + + var siblings = rewrapPoint.Parent.Children; + int index = rewrapPoint.ChildIndex; + // remove Nullable-ctor, if necessary + if (NullableLiftingTransform.MatchNullableCtor(rewrapPoint, out var utype, out var arg) && arg.InferType(context.TypeSystem).Equals(utype)) { + rewrapPoint = arg; + } + // insert a NullableRewrap instruction at the 'rewrapPoint' + siblings[index] = new NullableRewrap(rewrapPoint); + // if the endBlock is only reachable through the current block, + // combine both blocks. + if (endBlock?.IncomingEdgeCount == 1) { + block.Instructions.AddRange(endBlock.Instructions); + block.Instructions.RemoveAt(pos + 1); + endBlock.Remove(); } } } @@ -310,60 +339,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms } oldParentChildren[oldChildIndex] = replacement; } - } - - class NullPropagationStatementTransform : IStatementTransform - { - public void Run(Block block, int pos, StatementTransformContext context) - { - if (!context.Settings.NullPropagation) - return; - new NullPropagationTransform(context).RunStatements(block, pos); - if (TransformNullPropagationOnUnconstrainedGenericExpression(block, pos, out var target, out var value, out var rewrapPoint, out var endBlock)) { - context.Step("TransformNullPropagationOnUnconstrainedGenericExpression", block); - // A successful match was found: - // 1. The 'target' instruction, that is, the instruction where the actual 'null-propagating' call happens: - // .Call() is replaced with ?.Call() - // 2. The 'value' instruction, that is, the instruction that produces the value: - // It is inlined at the location of 'target'. - // 3. The 'rewrapPoint' instruction is an ancestor of the call that is used as location for the NullableRewrap instruction, - // if the expression does not yet contain a NullableRewrap. - - // First try to find a NullableRewrap instruction - bool needsRewrap = true; - var tmp = target; - while (needsRewrap && tmp != null) { - if (tmp is NullableRewrap) { - needsRewrap = false; - break; - } - tmp = tmp.Parent; - } - // Remove the fallback conditions and blocks - block.Instructions.RemoveRange(pos, 3); - // inline value and wrap it in a NullableUnwrap instruction to produce 'value?'. - target.ReplaceWith(new NullableUnwrap(value.ResultType, value, refInput: true)); - - var siblings = rewrapPoint.Parent.Children; - int index = rewrapPoint.ChildIndex; - // remove Nullable-ctor, if necessary - if (NullableLiftingTransform.MatchNullableCtor(rewrapPoint, out var utype, out var arg) && arg.InferType(context.TypeSystem).Equals(utype)) { - rewrapPoint = arg; - } - // if the ancestors do not yet have a NullableRewrap instruction - // insert it at the 'rewrapPoint'. - if (needsRewrap) { - siblings[index] = new NullableRewrap(rewrapPoint); - } - // if the endBlock is only reachable through the current block, - // combine both blocks. - if (endBlock?.IncomingEdgeCount == 1) { - block.Instructions.AddRange(endBlock.Instructions); - block.Instructions.RemoveAt(pos + 1); - endBlock.Remove(); - } - } - } // stloc valueTemporary(valueExpression) // stloc defaultTemporary(default.value type) @@ -395,11 +370,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms // => // leave (nullable.rewrap(constrained[type].call_instruction(nullable.unwrap(valueExpression), ...))) private bool TransformNullPropagationOnUnconstrainedGenericExpression(Block block, int pos, - out ILInstruction target, out ILInstruction value, out ILInstruction rewrapPoint, out Block endBlock) + out ILInstruction target, out ILInstruction value, out ILInstruction nonNullInst, out Block endBlock) { target = null; value = null; - rewrapPoint = null; + nonNullInst = null; endBlock = null; if (pos + 3 >= block.Instructions.Count) return false; @@ -417,30 +392,27 @@ namespace ICSharpCode.Decompiler.IL.Transforms // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock if (!(block.Instructions[pos + 2].MatchIfInstruction(out var condition, out var fallbackBlock1) && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) return false; - if (!MatchStLocResultTemporary(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out rewrapPoint, out var call, out endBlock) && !MatchLeaveResult(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out rewrapPoint, out call)) + if (!MatchStLocResultTemporary(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out nonNullInst, out target, out endBlock) + && !MatchLeaveResult(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out nonNullInst, out target)) return false; - target = call.Arguments[0]; return true; } // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) // br IL_0035 - private bool MatchStLocResultTemporary(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock1, out ILInstruction rewrapPoint, out CallInstruction call, out Block endBlock) + private bool MatchStLocResultTemporary(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock1, out ILInstruction nonNullInst, out ILInstruction finalLoad, out Block endBlock) { - call = null; endBlock = null; - rewrapPoint = null; + nonNullInst = null; + finalLoad = null; if (pos + 4 >= block.Instructions.Count) return false; // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) - if (!(block.Instructions[pos + 3].MatchStLoc(out var resultTemporary, out rewrapPoint))) + if (!(block.Instructions[pos + 3].MatchStLoc(out var resultTemporary, out nonNullInst))) return false; - var loadInCall = FindLoadInExpression(valueTemporary, rewrapPoint); - if (!(loadInCall != null && loadInCall.Ancestors.OfType().FirstOrDefault() is CallInstruction c)) - return false; - if (!(c.Arguments.Count > 0 && loadInCall.IsDescendantOf(c.Arguments[0]))) + if (!IsValidAccessChain(valueTemporary, Mode.ReferenceType, nonNullInst, out finalLoad)) return false; // br IL_0035 if (!(block.Instructions[pos + 4].MatchBranch(out endBlock))) @@ -449,41 +421,26 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!(fallbackBlock1 is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, resultTemporary, endBlock))) return false; - call = c; return true; } - private bool MatchLeaveResult(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction rewrapPoint, out CallInstruction call) + private bool MatchLeaveResult(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction rewrapPoint, out ILInstruction finalLoad) { - call = null; rewrapPoint = null; + finalLoad = null; // leave (constrained[type].call_instruction(ldloc valueTemporary, ...)) if (!(block.Instructions[pos + 3] is Leave leave && leave.IsLeavingFunction)) return false; rewrapPoint = leave.Value; - var loadInCall = FindLoadInExpression(valueTemporary, rewrapPoint); - if (!(loadInCall != null && loadInCall.Ancestors.OfType().FirstOrDefault() is CallInstruction c)) - return false; - if (!(c.Arguments.Count > 0 && loadInCall.IsDescendantOf(c.Arguments[0]))) + if (!IsValidAccessChain(valueTemporary, Mode.ReferenceType, rewrapPoint, out finalLoad)) return false; // Analyze Block fallbackBlock if (!(fallbackBlock is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, null, leave.TargetContainer))) return false; - - call = c; return true; } - private ILInstruction FindLoadInExpression(ILVariable variable, ILInstruction expression) - { - foreach (var load in variable.LoadInstructions) { - if (load.IsDescendantOf(expression)) - return load; - } - return null; - } - // Block fallbackBlock { // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) // stloc valueTemporary(ldloca defaultTemporary) @@ -522,7 +479,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms } else if (!(fallbackInst is Leave fallbackLeave && endBlockOrLeaveContainer is BlockContainer leaveContainer && fallbackLeave.TargetContainer == leaveContainer && MatchDefaultValueOrLdNull(fallbackLeave.Value))) return false; - + return true; } @@ -531,4 +488,14 @@ namespace ICSharpCode.Decompiler.IL.Transforms return inst.MatchLdNull() || inst.MatchDefaultValue(out _); } } + + class NullPropagationStatementTransform : IStatementTransform + { + public void Run(Block block, int pos, StatementTransformContext context) + { + if (!context.Settings.NullPropagation) + return; + new NullPropagationTransform(context).RunStatements(block, pos); + } + } } From b114734128eb4da79b0321cea24d77c71b221564 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Tue, 28 Apr 2020 18:38:07 +0200 Subject: [PATCH 6/8] Add Mode.UnconstrainedType and implement TransformNullPropagationOnUnconstrainedGenericExpression using TryNullPropagation. --- .../TestCases/Pretty/NullPropagation.cs | 5 + .../IL/Transforms/NullPropagationTransform.cs | 110 ++++++++++-------- 2 files changed, 64 insertions(+), 51 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs index efb9ec366..6e927db63 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/NullPropagation.cs @@ -100,6 +100,11 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty return Other?.Other.Other?.Other.Field1?.ToString()?.GetType().Name; } + public int? Test2() + { + return Field1?.ToString().Length ?? 42; + } + public int? GetTextLengthNRE() { return (Field1?.ToString()).Length; diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs index 5c4660016..19a10b6b9 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs @@ -59,6 +59,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms /// nullable type, used by reference (comparison is 'call get_HasValue(ldloc(testedVar))') /// NullableByReference, + /// + /// unconstrained generic type (see the pattern described in TransformNullPropagationOnUnconstrainedGenericExpression) + /// + UnconstrainedType, } /// @@ -154,35 +158,34 @@ namespace ICSharpCode.Decompiler.IL.Transforms } } } - if (TransformNullPropagationOnUnconstrainedGenericExpression(block, pos, out var target, out var value, out var rewrapPoint, out var endBlock)) { + if (TransformNullPropagationOnUnconstrainedGenericExpression(block, pos, out var testedVariable, out var nonNullInst, out var nullInst, out var endBlock)) { + var parentInstruction = nonNullInst.Parent; + var replacement = TryNullPropagation(testedVariable, nonNullInst, nullInst, Mode.UnconstrainedType); + if (replacement == null) + return; context.Step("TransformNullPropagationOnUnconstrainedGenericExpression", block); - // A successful match was found: - // 1. The 'target' instruction, that is, the instruction where the actual 'null-propagating' call happens: - // .Call() is replaced with ?.Call() - // 2. The 'value' instruction, that is, the instruction that produces the value: - // It is inlined at the location of 'target'. - // 3. The 'rewrapPoint' instruction is an ancestor of the call that is used as location for the NullableRewrap instruction - - // Remove the fallback conditions and blocks - block.Instructions.RemoveRange(pos, 3); - // inline value and wrap it in a NullableUnwrap instruction to produce 'value?'. - target.ReplaceWith(new NullableUnwrap(value.ResultType, value, refInput: true)); - - var siblings = rewrapPoint.Parent.Children; - int index = rewrapPoint.ChildIndex; - // remove Nullable-ctor, if necessary - if (NullableLiftingTransform.MatchNullableCtor(rewrapPoint, out var utype, out var arg) && arg.InferType(context.TypeSystem).Equals(utype)) { - rewrapPoint = arg; + switch (parentInstruction) { + case StLoc stloc: + stloc.Value = replacement; + break; + case Leave leave: + leave.Value = replacement; + break; + default: + // if this ever happens, the pattern checked by TransformNullPropagationOnUnconstrainedGenericExpression + // has changed, but this part of the code was not properly adjusted. + throw new NotSupportedException(); } - // insert a NullableRewrap instruction at the 'rewrapPoint' - siblings[index] = new NullableRewrap(rewrapPoint); + // Remove the fallback conditions and blocks + block.Instructions.RemoveRange(pos + 1, 2); // if the endBlock is only reachable through the current block, // combine both blocks. if (endBlock?.IncomingEdgeCount == 1) { block.Instructions.AddRange(endBlock.Instructions); - block.Instructions.RemoveAt(pos + 1); + block.Instructions.RemoveAt(pos + 2); endBlock.Remove(); } + ILInlining.InlineIfPossible(block, pos, context); } } @@ -290,6 +293,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms case Mode.NullableByReference: return NullableLiftingTransform.MatchGetValueOrDefault(inst, out ILInstruction arg) && arg.MatchLdLoc(testedVar); + case Mode.UnconstrainedType: + // unconstrained generic type (expect: ldloc(testedVar)) + return inst.MatchLdLoc(testedVar); default: throw new ArgumentOutOfRangeException(nameof(mode)); } @@ -334,6 +340,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms refInput: true ).WithILRange(varLoad); break; + case Mode.UnconstrainedType: + // Wrap varLoad in nullable.unwrap: + replacement = new NullableUnwrap(varLoad.ResultType, varLoad, refInput: varLoad.ResultType == StackType.Ref); + break; default: throw new ArgumentOutOfRangeException(nameof(mode)); } @@ -346,7 +356,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) // stloc valueTemporary(ldloca defaultTemporary) // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock2 { - // stloc resultTemporary(ldnull) + // stloc resultTemporary(nullInst) // br endBlock // } // } @@ -363,23 +373,23 @@ namespace ICSharpCode.Decompiler.IL.Transforms // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) // stloc valueTemporary(ldloca defaultTemporary) // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock2 { - // leave(ldnull) + // leave(nullInst) // } // } // leave (constrained[type].call_instruction(ldloc valueTemporary, ...)) // => // leave (nullable.rewrap(constrained[type].call_instruction(nullable.unwrap(valueExpression), ...))) private bool TransformNullPropagationOnUnconstrainedGenericExpression(Block block, int pos, - out ILInstruction target, out ILInstruction value, out ILInstruction nonNullInst, out Block endBlock) + out ILVariable valueTemporary, out ILInstruction nonNullInst, out ILInstruction nullInst, out Block endBlock) { - target = null; - value = null; + valueTemporary = null; nonNullInst = null; + nullInst = null; endBlock = null; if (pos + 3 >= block.Instructions.Count) return false; // stloc valueTemporary(valueExpression) - if (!(block.Instructions[pos].MatchStLoc(out var valueTemporary, out value))) + if (!block.Instructions[pos].MatchStLoc(out valueTemporary, out var value)) return false; if (!(valueTemporary.Kind == VariableKind.StackSlot && valueTemporary.LoadCount == 2 && valueTemporary.StoreCount == 2)) return false; @@ -392,51 +402,46 @@ namespace ICSharpCode.Decompiler.IL.Transforms // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock if (!(block.Instructions[pos + 2].MatchIfInstruction(out var condition, out var fallbackBlock1) && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) return false; - if (!MatchStLocResultTemporary(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out nonNullInst, out target, out endBlock) - && !MatchLeaveResult(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out nonNullInst, out target)) + if (!MatchStLocResultTemporary(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out nonNullInst, out nullInst, out endBlock) + && !MatchLeaveResult(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out nonNullInst, out nullInst)) return false; return true; } // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) - // br IL_0035 - private bool MatchStLocResultTemporary(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock1, out ILInstruction nonNullInst, out ILInstruction finalLoad, out Block endBlock) + // br endBlock + private bool MatchStLocResultTemporary(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction nonNullInst, out ILInstruction nullInst, out Block endBlock) { endBlock = null; nonNullInst = null; - finalLoad = null; + nullInst = null; if (pos + 4 >= block.Instructions.Count) return false; - // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) if (!(block.Instructions[pos + 3].MatchStLoc(out var resultTemporary, out nonNullInst))) return false; - if (!IsValidAccessChain(valueTemporary, Mode.ReferenceType, nonNullInst, out finalLoad)) - return false; - // br IL_0035 + // br endBlock if (!(block.Instructions[pos + 4].MatchBranch(out endBlock))) return false; // Analyze Block fallbackBlock - if (!(fallbackBlock1 is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, resultTemporary, endBlock))) + if (!(fallbackBlock is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, resultTemporary, endBlock, out nullInst))) return false; return true; } - private bool MatchLeaveResult(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction rewrapPoint, out ILInstruction finalLoad) + private bool MatchLeaveResult(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction nonNullInst, out ILInstruction nullInst) { - rewrapPoint = null; - finalLoad = null; + nonNullInst = null; + nullInst = null; // leave (constrained[type].call_instruction(ldloc valueTemporary, ...)) if (!(block.Instructions[pos + 3] is Leave leave && leave.IsLeavingFunction)) return false; - rewrapPoint = leave.Value; - if (!IsValidAccessChain(valueTemporary, Mode.ReferenceType, rewrapPoint, out finalLoad)) - return false; + nonNullInst = leave.Value; // Analyze Block fallbackBlock - if (!(fallbackBlock is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, null, leave.TargetContainer))) + if (!(fallbackBlock is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, null, leave.TargetContainer, out nullInst))) return false; return true; } @@ -449,8 +454,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms // br endBlock // } // } - private bool IsFallbackBlock(Block block, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILVariable resultTemporary, ILInstruction endBlockOrLeaveContainer) + private bool IsFallbackBlock(Block block, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILVariable resultTemporary, ILInstruction endBlockOrLeaveContainer, out ILInstruction nullInst) { + nullInst = null; if (!(block.Instructions.Count == 3)) return false; // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) @@ -465,21 +471,23 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (!(block.Instructions[2].MatchIfInstruction(out var condition, out var tmp) && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) return false; // Block fallbackBlock { - // stloc resultTemporary(ldnull) + // stloc resultTemporary(nullInst) // br endBlock // } var fallbackInst = Block.Unwrap(tmp); if (fallbackInst is Block fallbackBlock && endBlockOrLeaveContainer is Block endBlock) { if (!(fallbackBlock.Instructions.Count == 2)) return false; - if (!(fallbackBlock.Instructions[0].MatchStLoc(resultTemporary, out var defaultValue) && MatchDefaultValueOrLdNull(defaultValue))) + if (!fallbackBlock.Instructions[0].MatchStLoc(resultTemporary, out nullInst)) return false; if (!fallbackBlock.Instructions[1].MatchBranch(endBlock)) return false; - } else if (!(fallbackInst is Leave fallbackLeave && endBlockOrLeaveContainer is BlockContainer leaveContainer - && fallbackLeave.TargetContainer == leaveContainer && MatchDefaultValueOrLdNull(fallbackLeave.Value))) - return false; - + } else { + if (!(fallbackInst is Leave fallbackLeave && endBlockOrLeaveContainer is BlockContainer leaveContainer + && fallbackLeave.TargetContainer == leaveContainer && !fallbackLeave.Value.MatchNop())) + return false; + nullInst = fallbackLeave.Value; + } return true; } From 7d8b9fee1e3bf9f2a5f1659c849063baceb6fd78 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Fri, 1 May 2020 16:33:03 +0200 Subject: [PATCH 7/8] Remove redundant code. --- .../IL/Transforms/NullPropagationTransform.cs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs index 19a10b6b9..79c969332 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs @@ -323,6 +323,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms ILInstruction replacement; switch (mode) { case Mode.ReferenceType: + case Mode.UnconstrainedType: // Wrap varLoad in nullable.unwrap: replacement = new NullableUnwrap(varLoad.ResultType, varLoad, refInput: varLoad.ResultType == StackType.Ref); break; @@ -340,10 +341,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms refInput: true ).WithILRange(varLoad); break; - case Mode.UnconstrainedType: - // Wrap varLoad in nullable.unwrap: - replacement = new NullableUnwrap(varLoad.ResultType, varLoad, refInput: varLoad.ResultType == StackType.Ref); - break; default: throw new ArgumentOutOfRangeException(nameof(mode)); } @@ -490,11 +487,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms } return true; } - - private bool MatchDefaultValueOrLdNull(ILInstruction inst) - { - return inst.MatchLdNull() || inst.MatchDefaultValue(out _); - } } class NullPropagationStatementTransform : IStatementTransform From 571118583229c87cecb616295667416d461b5587 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Fri, 1 May 2020 17:37:22 +0200 Subject: [PATCH 8/8] Make sure that the code and the pattern described in the comment above are in sync. --- .../IL/Transforms/NullPropagationTransform.cs | 64 +++++++++---------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs index 79c969332..93d7f90f5 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullPropagationTransform.cs @@ -347,48 +347,48 @@ namespace ICSharpCode.Decompiler.IL.Transforms oldParentChildren[oldChildIndex] = replacement; } - // stloc valueTemporary(valueExpression) + // stloc target(targetInst) // stloc defaultTemporary(default.value type) // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock { - // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) - // stloc valueTemporary(ldloca defaultTemporary) + // stloc defaultTemporary(ldobj type(ldloc target)) + // stloc target(ldloca defaultTemporary) // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock2 { // stloc resultTemporary(nullInst) // br endBlock // } // } - // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) + // stloc resultTemporary(constrained[type].call_instruction(ldloc target, ...)) // br endBlock // => - // stloc resultTemporary(nullable.rewrap(constrained[type].call_instruction(nullable.unwrap(valueExpression), ...))) + // stloc resultTemporary(nullable.rewrap(constrained[type].call_instruction(nullable.unwrap(targetInst), ...))) // // -or- // - // stloc valueTemporary(valueExpression) + // stloc target(targetInst) // stloc defaultTemporary(default.value type) // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock { - // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) - // stloc valueTemporary(ldloca defaultTemporary) + // stloc defaultTemporary(ldobj type(ldloc target)) + // stloc target(ldloca defaultTemporary) // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock2 { // leave(nullInst) // } // } - // leave (constrained[type].call_instruction(ldloc valueTemporary, ...)) + // leave (constrained[type].call_instruction(ldloc target, ...)) // => - // leave (nullable.rewrap(constrained[type].call_instruction(nullable.unwrap(valueExpression), ...))) + // leave (nullable.rewrap(constrained[type].call_instruction(nullable.unwrap(targetInst), ...))) private bool TransformNullPropagationOnUnconstrainedGenericExpression(Block block, int pos, - out ILVariable valueTemporary, out ILInstruction nonNullInst, out ILInstruction nullInst, out Block endBlock) + out ILVariable target, out ILInstruction nonNullInst, out ILInstruction nullInst, out Block endBlock) { - valueTemporary = null; + target = null; nonNullInst = null; nullInst = null; endBlock = null; if (pos + 3 >= block.Instructions.Count) return false; - // stloc valueTemporary(valueExpression) - if (!block.Instructions[pos].MatchStLoc(out valueTemporary, out var value)) + // stloc target(...) + if (!block.Instructions[pos].MatchStLoc(out target, out _)) return false; - if (!(valueTemporary.Kind == VariableKind.StackSlot && valueTemporary.LoadCount == 2 && valueTemporary.StoreCount == 2)) + if (!(target.Kind == VariableKind.StackSlot && target.LoadCount == 2 && target.StoreCount == 2)) return false; // stloc defaultTemporary(default.value type) if (!(block.Instructions[pos + 1].MatchStLoc(out var defaultTemporary, out var defaultExpression) && defaultExpression.MatchDefaultValue(out var type))) @@ -399,15 +399,15 @@ namespace ICSharpCode.Decompiler.IL.Transforms // if (logic.not(comp.o(box `0(ldloc defaultTemporary) != ldnull))) Block fallbackBlock if (!(block.Instructions[pos + 2].MatchIfInstruction(out var condition, out var fallbackBlock1) && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary))) return false; - if (!MatchStLocResultTemporary(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out nonNullInst, out nullInst, out endBlock) - && !MatchLeaveResult(block, pos, type, valueTemporary, defaultTemporary, fallbackBlock1, out nonNullInst, out nullInst)) + if (!MatchStLocResultTemporary(block, pos, type, target, defaultTemporary, fallbackBlock1, out nonNullInst, out nullInst, out endBlock) + && !MatchLeaveResult(block, pos, type, target, defaultTemporary, fallbackBlock1, out nonNullInst, out nullInst)) return false; return true; } - // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) + // stloc resultTemporary(constrained[type].call_instruction(ldloc target, ...)) // br endBlock - private bool MatchStLocResultTemporary(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction nonNullInst, out ILInstruction nullInst, out Block endBlock) + private bool MatchStLocResultTemporary(Block block, int pos, IType type, ILVariable target, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction nonNullInst, out ILInstruction nullInst, out Block endBlock) { endBlock = null; nonNullInst = null; @@ -415,54 +415,54 @@ namespace ICSharpCode.Decompiler.IL.Transforms if (pos + 4 >= block.Instructions.Count) return false; - // stloc resultTemporary(constrained[type].call_instruction(ldloc valueTemporary, ...)) + // stloc resultTemporary(constrained[type].call_instruction(ldloc target, ...)) if (!(block.Instructions[pos + 3].MatchStLoc(out var resultTemporary, out nonNullInst))) return false; // br endBlock if (!(block.Instructions[pos + 4].MatchBranch(out endBlock))) return false; // Analyze Block fallbackBlock - if (!(fallbackBlock is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, resultTemporary, endBlock, out nullInst))) + if (!(fallbackBlock is Block b && IsFallbackBlock(b, type, target, defaultTemporary, resultTemporary, endBlock, out nullInst))) return false; return true; } - private bool MatchLeaveResult(Block block, int pos, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction nonNullInst, out ILInstruction nullInst) + private bool MatchLeaveResult(Block block, int pos, IType type, ILVariable target, ILVariable defaultTemporary, ILInstruction fallbackBlock, out ILInstruction nonNullInst, out ILInstruction nullInst) { nonNullInst = null; nullInst = null; - // leave (constrained[type].call_instruction(ldloc valueTemporary, ...)) + // leave (constrained[type].call_instruction(ldloc target, ...)) if (!(block.Instructions[pos + 3] is Leave leave && leave.IsLeavingFunction)) return false; nonNullInst = leave.Value; // Analyze Block fallbackBlock - if (!(fallbackBlock is Block b && IsFallbackBlock(b, type, valueTemporary, defaultTemporary, null, leave.TargetContainer, out nullInst))) + if (!(fallbackBlock is Block b && IsFallbackBlock(b, type, target, defaultTemporary, null, leave.TargetContainer, out nullInst))) return false; return true; } // Block fallbackBlock { - // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) - // stloc valueTemporary(ldloca defaultTemporary) + // stloc defaultTemporary(ldobj type(ldloc target)) + // stloc target(ldloca defaultTemporary) // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock { // stloc resultTemporary(ldnull) // br endBlock // } // } - private bool IsFallbackBlock(Block block, IType type, ILVariable valueTemporary, ILVariable defaultTemporary, ILVariable resultTemporary, ILInstruction endBlockOrLeaveContainer, out ILInstruction nullInst) + private bool IsFallbackBlock(Block block, IType type, ILVariable target, ILVariable defaultTemporary, ILVariable resultTemporary, ILInstruction endBlockOrLeaveContainer, out ILInstruction nullInst) { nullInst = null; if (!(block.Instructions.Count == 3)) return false; - // stloc defaultTemporary(ldobj type(ldloc valueTemporary)) - if (!(block.Instructions[0].MatchStLoc(defaultTemporary, out var valueExpression))) + // stloc defaultTemporary(ldobj type(ldloc target)) + if (!(block.Instructions[0].MatchStLoc(defaultTemporary, out var value))) return false; - if (!(valueExpression.MatchLdObj(out var target, out var t) && type.Equals(t) && target.MatchLdLoc(valueTemporary))) + if (!(value.MatchLdObj(out var inst, out var t) && type.Equals(t) && inst.MatchLdLoc(target))) return false; - // stloc valueTemporary(ldloca defaultTemporary) - if (!(block.Instructions[1].MatchStLoc(valueTemporary, out var defaultAddress) && defaultAddress.MatchLdLoca(defaultTemporary))) + // stloc target(ldloca defaultTemporary) + if (!(block.Instructions[1].MatchStLoc(target, out var defaultAddress) && defaultAddress.MatchLdLoca(defaultTemporary))) return false; // if (comp.o(ldloc defaultTemporary == ldnull)) Block fallbackBlock if (!(block.Instructions[2].MatchIfInstruction(out var condition, out var tmp) && condition.MatchCompEqualsNull(out var arg) && arg.MatchLdLoc(defaultTemporary)))