diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index 903c59586..65849d73d 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -158,7 +158,6 @@ namespace ICSharpCode.Decompiler.CSharp }, new ProxyCallReplacer(), new FixRemainingIncrements(), - new FixLoneIsInst(), new CopyPropagation(), new DelegateConstruction(), new LocalFunctionDecompiler(), diff --git a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs index 3f5427a12..91010db44 100644 --- a/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs @@ -398,10 +398,12 @@ namespace ICSharpCode.Decompiler.CSharp // unbox.any T(isinst T(expr)) ==> "expr as T" for nullable value types and class-constrained generic types // comp(isinst T(expr) != null) ==> "expr is T" // on block level (StatementBuilder.VisitIsInst) => "expr is T" - if (SemanticHelper.IsPure(inst.Argument.Flags)) + if (SemanticHelper.IsPure(inst.Argument.Flags) || (inst.Argument is Box box && SemanticHelper.IsPure(box.Argument.Flags))) { // We can emulate isinst using // expr is T ? expr : null + // (doubling the boxing side-effect is harmless because the "expr is T" part won't observe object identity, + // and we need to support this because Roslyn pattern matching sometimes generates such code.) return new ConditionalExpression( new IsExpression(arg, ConvertType(inst.Type)).WithILInstruction(inst), arg.Expression.Clone(), @@ -3185,16 +3187,16 @@ namespace ICSharpCode.Decompiler.CSharp return input.ConvertTo(targetType, this); } - internal static bool IsUnboxAnyWithIsInst(UnboxAny unboxAny, IsInst isInst) + internal static bool IsUnboxAnyWithIsInst(UnboxAny unboxAny, IType isInstType) { - return unboxAny.Type.Equals(isInst.Type) - && (unboxAny.Type.IsKnownType(KnownTypeCode.NullableOfT) || isInst.Type.IsReferenceType == true); + return unboxAny.Type.Equals(isInstType) + && (unboxAny.Type.IsKnownType(KnownTypeCode.NullableOfT) || isInstType.IsReferenceType == true); } protected internal override TranslatedExpression VisitUnboxAny(UnboxAny inst, TranslationContext context) { TranslatedExpression arg; - if (inst.Argument is IsInst isInst && IsUnboxAnyWithIsInst(inst, isInst)) + if (inst.Argument is IsInst isInst && IsUnboxAnyWithIsInst(inst, isInst.Type)) { // unbox.any T(isinst T(expr)) ==> expr as T // This is used for generic types and nullable value types diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index 3a0cb5d01..00cb53e3c 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -136,7 +136,6 @@ - @@ -413,6 +412,7 @@ + diff --git a/ICSharpCode.Decompiler/IL/ILReader.cs b/ICSharpCode.Decompiler/IL/ILReader.cs index 272a8ed91..7f070479a 100644 --- a/ICSharpCode.Decompiler/IL/ILReader.cs +++ b/ICSharpCode.Decompiler/IL/ILReader.cs @@ -1138,7 +1138,14 @@ namespace ICSharpCode.Decompiler.IL case ILOpCode.Initobj: return InitObj(PopStObjTarget(), ReadAndDecodeTypeReference()); case ILOpCode.Isinst: - return Push(new IsInst(Pop(StackType.O), ReadAndDecodeTypeReference())); + { + var type = ReadAndDecodeTypeReference(); + if (type.IsReferenceType != true) + { + FlushExpressionStack(); // value-type isinst has inlining restrictions + } + return Push(new IsInst(Pop(StackType.O), type)); + } case ILOpCode.Ldelem: return LdElem(ReadAndDecodeTypeReference()); case ILOpCode.Ldelem_i1: diff --git a/ICSharpCode.Decompiler/IL/Instructions/Block.cs b/ICSharpCode.Decompiler/IL/Instructions/Block.cs index 06ea213f0..3f8bf1d06 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Block.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Block.cs @@ -217,6 +217,22 @@ namespace ICSharpCode.Decompiler.IL } } + internal override bool CanInlineIntoSlot(int childIndex, ILInstruction expressionBeingMoved) + { + switch (Kind) + { + case BlockKind.ControlFlow when Parent is BlockContainer: + case BlockKind.ArrayInitializer: + case BlockKind.CollectionInitializer: + case BlockKind.ObjectInitializer: + case BlockKind.CallInlineAssign: + // Allow inlining into the first instruction of the block + return childIndex == 0; + default: + return false; + } + } + /// /// Gets the name of this block. /// diff --git a/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs b/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs index ef03c4e10..d3f32df2e 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs @@ -941,13 +941,28 @@ namespace ICSharpCode.Decompiler.IL } /// - /// Gets whether the specified instruction may be inlined into the specified slot. - /// Note: this does not check whether reordering with the previous slots is valid; only wheter the target slot supports inlining at all! + /// Gets whether the expressionBeingMoved, which previously executes prior to `this`, + /// may be moved into a descendant of the specified slot. + /// (i.e. this controls whether FindLoadInNext may descent into the specified slot) + /// + /// Note: this does not check whether reordering with the previous slots is valid; only whether the target slot supports inlining at all! /// internal virtual bool CanInlineIntoSlot(int childIndex, ILInstruction expressionBeingMoved) { return GetChildSlot(childIndex).CanInlineInto; } + + /// + /// Some slots in the ILAst have restrictions on which instructions can appear in them. + /// Used to suppress inlining if the new child does not satisfy the restrictions. + /// Unlike `CanInlineIntoSlot`, this is not about descendants of the slot, only about + /// whether SetChild(childIndex, newChild) is valid. + /// (i.e. this controls whether FindLoadInNext may return the specified slot as a final result) + /// + internal virtual bool SatisfiesSlotRestrictionForInlining(int childIndex, ILInstruction newChild) + { + return true; + } } public interface IInstructionWithTypeOperand diff --git a/ICSharpCode.Decompiler/IL/Instructions/IsInst.cs b/ICSharpCode.Decompiler/IL/Instructions/IsInst.cs new file mode 100644 index 000000000..714bfefbf --- /dev/null +++ b/ICSharpCode.Decompiler/IL/Instructions/IsInst.cs @@ -0,0 +1,62 @@ +#nullable enable +// Copyright (c) 2025 Daniel Grunwald +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System.Diagnostics; + +using ICSharpCode.Decompiler.CSharp; + +namespace ICSharpCode.Decompiler.IL; + +partial class IsInst +{ + internal override bool SatisfiesSlotRestrictionForInlining(int childIndex, ILInstruction newChild) + { + Debug.Assert(childIndex == 0); + Debug.Assert(base.SatisfiesSlotRestrictionForInlining(childIndex, newChild)); + if (this.Type.IsReferenceType == true) + { + return true; // reference-type isinst is always supported + } + if (SemanticHelper.IsPure(newChild.Flags)) + { + return true; // emulated via "expr is T ? (T)expr : null" + } + else if (newChild is Box box && SemanticHelper.IsPure(box.Argument.Flags)) + { + // Also emulated via "expr is T ? (T)expr : null". + // This duplicates the boxing side-effect, but that's harmless as one of the boxes is only + // used in the `expr is T` type test where the object identity can never be observed. + // This appears as part of C# pattern matching, inlining early makes those code patterns easier to detect. + return true; + } + if (this.Parent is UnboxAny unboxAny && ExpressionBuilder.IsUnboxAnyWithIsInst(unboxAny, this.Type)) + { + return true; // supported pattern "expr as T?" + } + if (this.Parent != null && (this.Parent.MatchCompEqualsNull(out _) || this.Parent.MatchCompNotEqualsNull(out _))) + { + return true; // supported pattern "expr is T" + } + if (this.Parent is Block { Kind: BlockKind.ControlFlow }) + { + return true; // supported via StatementBuilder.VisitIsInst + } + return false; + } +} \ No newline at end of file diff --git a/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs b/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs index ff35a007c..2f91962f6 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs @@ -48,28 +48,35 @@ namespace ICSharpCode.Decompiler.IL public sealed partial class StObj { - /// - /// For a store to a field or array element, C# will only throw NullReferenceException/IndexOfBoundsException - /// after the value-to-be-stored has been computed. - /// This means a LdFlda/LdElema used as target for StObj must have DelayExceptions==true to allow a translation to C# - /// without changing the program semantics. See https://github.com/icsharpcode/ILSpy/issues/2050 - /// - public bool CanInlineIntoTargetSlot(ILInstruction inst) + internal override bool SatisfiesSlotRestrictionForInlining(int childIndex, ILInstruction newChild) { - switch (inst.OpCode) + if (childIndex == 0) // target slot { - case OpCode.LdElema: - case OpCode.LdFlda: - Debug.Assert(inst.HasDirectFlag(InstructionFlags.MayThrow)); - // If the ldelema/ldflda may throw a non-delayed exception, inlining will cause it - // to turn into a delayed exception after the translation to C#. - // This is only valid if the value computation doesn't involve any side effects. - return SemanticHelper.IsPure(this.Value.Flags); - // Note that after inlining such a ldelema/ldflda, the normal inlining rules will - // prevent us from inlining an effectful instruction into the value slot. - default: - return true; + Debug.Assert(GetChildSlot(childIndex) == TargetSlot); + // For a store to a field or array element, C# will only throw NullReferenceException/IndexOfBoundsException + // after the value-to-be-stored has been computed. + // This means a LdFlda/LdElema used as target for StObj must have DelayExceptions==true to allow a translation to C# + // without changing the program semantics. See https://github.com/icsharpcode/ILSpy/issues/2050 + switch (newChild.OpCode) + { + case OpCode.LdElema: + case OpCode.LdFlda: + { + Debug.Assert(newChild.HasDirectFlag(InstructionFlags.MayThrow)); + // If the ldelema/ldflda may throw a non-delayed exception, inlining will cause it + // to turn into a delayed exception after the translation to C#. + // This is only valid if the value computation doesn't involve any side effects. + if (!SemanticHelper.IsPure(this.Value.Flags)) + { + return false; + } + // Note that after inlining such a ldelema/ldflda, the normal inlining rules will + // prevent us from inlining an effectful instruction into the value slot. + break; + } + } } + return base.SatisfiesSlotRestrictionForInlining(childIndex, newChild); } /// diff --git a/ICSharpCode.Decompiler/IL/Transforms/FixLoneIsInst.cs b/ICSharpCode.Decompiler/IL/Transforms/FixLoneIsInst.cs deleted file mode 100644 index 50ddb43b0..000000000 --- a/ICSharpCode.Decompiler/IL/Transforms/FixLoneIsInst.cs +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright (c) 2020 Daniel Grunwald -// -// Permission is hereby granted, free of charge, to any person obtaining a copy of this -// software and associated documentation files (the "Software"), to deal in the Software -// without restriction, including without limitation the rights to use, copy, modify, merge, -// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons -// to whom the Software is furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all copies or -// substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, -// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR -// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE -// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER -// DEALINGS IN THE SOFTWARE. - -using System.Collections.Generic; -using System.Linq; - -using ICSharpCode.Decompiler.CSharp; - -namespace ICSharpCode.Decompiler.IL.Transforms -{ - /// - /// C# cannot represent `isinst T` directly for value-types. - /// This transform un-inlines the argument of `isinst` instructions that can't be directly translated to C#, - /// thus allowing the emulation via "expr is T ? (T)expr : null". - /// - public class FixLoneIsInst : IILTransform - { - void IILTransform.Run(ILFunction function, ILTransformContext context) - { - var instructionsToFix = new List(); - foreach (var isInst in function.Descendants.OfType()) - { - if (isInst.Type.IsReferenceType == true) - { - continue; // reference-type isinst is always supported - } - if (SemanticHelper.IsPure(isInst.Argument.Flags)) - { - continue; // emulated via "expr is T ? (T)expr : null" - } - if (isInst.Parent is UnboxAny unboxAny && ExpressionBuilder.IsUnboxAnyWithIsInst(unboxAny, isInst)) - { - continue; // supported pattern "expr as T?" - } - if (isInst.Parent.MatchCompEqualsNull(out _) || isInst.Parent.MatchCompNotEqualsNull(out _)) - { - continue; // supported pattern "expr is T" - } - if (isInst.Parent is Block { Kind: BlockKind.ControlFlow }) - { - continue; // supported via StatementBuilder.VisitIsInst - } - instructionsToFix.Add(isInst); - } - // Need to delay fixing until we're done with iteration, because Extract() modifies parents - foreach (var isInst in instructionsToFix) - { - // Use extraction to turn isInst.Argument into a pure instruction, thus making the emulation possible - context.Step("FixLoneIsInst", isInst); - isInst.Argument.Extract(context); - } - } - } -} diff --git a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs index 06565aebe..e66b993db 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs @@ -821,46 +821,27 @@ namespace ICSharpCode.Decompiler.IL.Transforms return FindResult.Stop; if (expr.MatchLdLoc(v) || expr.MatchLdLoca(v)) { - // Match found, we can inline - if (expr.SlotInfo == StObj.TargetSlot && !((StObj)expr.Parent).CanInlineIntoTargetSlot(expressionBeingMoved)) + // Match found, we can inline unless there are slot restrictions. + if (expr.Parent.SatisfiesSlotRestrictionForInlining(expr.ChildIndex, expressionBeingMoved)) { - if ((options & InliningOptions.AllowChangingOrderOfEvaluationForExceptions) != 0) - { - // Intentionally change code semantics so that we can avoid a ref local - if (expressionBeingMoved is LdFlda ldflda) - ldflda.DelayExceptions = true; - else if (expressionBeingMoved is LdElema ldelema) - ldelema.DelayExceptions = true; - } - else - { - // special case: the StObj.TargetSlot does not accept some kinds of expressions - return FindResult.Stop; - } + return FindResult.Found(expr); } - return FindResult.Found(expr); - } - else if (expr is Block block) - { - // Inlining into inline-blocks? - switch (block.Kind) + // We cannot inline because the targets slot restrictions are not satisfied + if ((options & InliningOptions.AllowChangingOrderOfEvaluationForExceptions) != 0 && expr.SlotInfo == StObj.TargetSlot) { - case BlockKind.ControlFlow when block.Parent is BlockContainer: - case BlockKind.ArrayInitializer: - case BlockKind.CollectionInitializer: - case BlockKind.ObjectInitializer: - case BlockKind.CallInlineAssign: - // Allow inlining into the first instruction of the block - if (block.Instructions.Count == 0) - return FindResult.Stop; - return NoContinue(FindLoadInNext(block.Instructions[0], v, expressionBeingMoved, options)); - // If FindLoadInNext() returns null, we still can't continue searching - // because we can't inline over the remainder of the block. - case BlockKind.CallWithNamedArgs: - return NamedArgumentTransform.CanExtendNamedArgument(block, v, expressionBeingMoved); - default: - return FindResult.Stop; + // Special case: inlining will change code semantics, + // but we accept that so that we can avoid a ref local. + if (expressionBeingMoved is LdFlda ldflda) + ldflda.DelayExceptions = true; + else if (expressionBeingMoved is LdElema ldelema) + ldelema.DelayExceptions = true; + return FindResult.Found(expr); } + return FindResult.Stop; + } + else if (expr is Block { Kind: BlockKind.CallWithNamedArgs } block) + { + return NamedArgumentTransform.CanExtendNamedArgument(block, v, expressionBeingMoved); } else if (options.HasFlag(InliningOptions.FindDeconstruction) && expr is DeconstructInstruction di) { @@ -886,14 +867,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms return FindResult.Stop; // abort, inlining not possible } - private static FindResult NoContinue(FindResult findResult) - { - if (findResult.Type == FindResultType.Continue) - return FindResult.Stop; - else - return findResult; - } - /// /// Determines whether it is safe to move 'expressionBeingMoved' past 'expr' ///