Merge pull request #2069 from icsharpcode/remove-redundant-statement-transform

Remove redundant StatementTransform pass.

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -120,18 +120,12 @@ namespace ICSharpCode.Decompiler.CSharp
 				new DetectExitPoints(canIntroduceExitForReturn: true),
 				new BlockILTransform { // per-block transforms
 					PostOrderTransforms = {
-						//new UseExitPoints(),
 						new ConditionDetection(),
 						new LockTransform(),
 						new UsingTransform(),
 						// CachedDelegateInitialization must run after ConditionDetection and before/in LoopingBlockTransform
 						// and must run before NullCoalescingTransform
 						new CachedDelegateInitialization(),
-						// Run the assignment transform both before and after copy propagation.
-						// Before is necessary because inline assignments of constants are otherwise
-						// copy-propated (turned into two separate assignments of the constant).
-						// After is necessary because the assigned value might involve null coalescing/etc.
-						new StatementTransform(new ILInlining(), new TransformAssignment()),
 						new StatementTransform(
 							// per-block transforms that depend on each other, and thus need to
 							// run interleaved (statement by statement).

ICSharpCode.Decompiler/IL/Instructions/BlockContainer.cs

@@ -20,6 +20,7 @@ using System;
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
+using ICSharpCode.Decompiler.IL.Transforms;
 using ICSharpCode.Decompiler.Util;
 
 namespace ICSharpCode.Decompiler.IL
@@ -239,6 +240,19 @@ namespace ICSharpCode.Decompiler.IL
 			}
 		}
 
+		internal override bool CanInlineIntoSlot(int childIndex, ILInstruction expressionBeingMoved)
+		{
+			// Inlining into the entry-point is allowed as long as we're not moving code into a loop.
+			// This is used to inline into the switch expression.
+			return childIndex == 0 && this.EntryPoint.IncomingEdgeCount == 1;
+		}
+
+		internal override bool PrepareExtract(int childIndex, ExtractionContext ctx)
+		{
+			// Un-inlining from the entry-point is allowed as long as we're not moving code out of a loop
+			return childIndex == 0 && this.EntryPoint.IncomingEdgeCount == 1;
+		}
+
 		/// <summary>
 		/// Topologically sort the blocks.
 		/// The new order is returned without modifying the BlockContainer.

ICSharpCode.Decompiler/IL/Instructions/ILFunction.cs

@@ -358,7 +358,16 @@ namespace ICSharpCode.Decompiler.IL
 				return InstructionFlags.MayThrow | InstructionFlags.ControlFlow;
 			}
 		}
-		
+
+		internal override bool CanInlineIntoSlot(int childIndex, ILInstruction expressionBeingMoved)
+		{
+			// With expression trees, we occasionally need to inline constants into an existing expression tree.
+			// Only allow this for completely pure constants; a MayReadLocals effect would already be problematic
+			// because we're essentially delaying evaluation of the expression until the ILFunction is called.
+			Debug.Assert(childIndex == 0);
+			return kind == ILFunctionKind.ExpressionTree && expressionBeingMoved.Flags == InstructionFlags.None;
+		}
+
 		/// <summary>
 		/// Apply a list of transforms to this function.
 		/// </summary>

ICSharpCode.Decompiler/IL/Instructions/ILInstruction.cs

@@ -802,6 +802,15 @@ namespace ICSharpCode.Decompiler.IL
 			}
 			return true;
 		}
+
+		/// <summary>
+		/// 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!
+		/// </summary>
+		internal virtual bool CanInlineIntoSlot(int childIndex, ILInstruction expressionBeingMoved)
+		{
+			return GetChildSlot(childIndex).CanInlineInto;
+		}
 	}
 	
 	public interface IInstructionWithTypeOperand

ICSharpCode.Decompiler/IL/Instructions/NullableInstructions.cs

@@ -63,7 +63,7 @@ namespace ICSharpCode.Decompiler.IL
 		/// </summary>
 		public bool RefOutput { get => ResultType == StackType.Ref; }
 
-		public NullableUnwrap(StackType unwrappedType, ILInstruction argument, bool refInput=false)
+		public NullableUnwrap(StackType unwrappedType, ILInstruction argument, bool refInput = false)
 			: base(OpCode.NullableUnwrap, argument)
 		{
 			this.ResultType = unwrappedType;
@@ -132,5 +132,13 @@ namespace ICSharpCode.Decompiler.IL
 			return base.PrepareExtract(childIndex, ctx)
 				&& (ctx.FlagsBeingMoved & InstructionFlags.MayUnwrapNull) == 0;
 		}
+
+		internal override bool CanInlineIntoSlot(int childIndex, ILInstruction expressionBeingMoved)
+		{
+			// Inlining into nullable.rewrap is OK unless the expression being inlined
+			// contains a nullable.wrap that isn't being re-wrapped within the expression being inlined.
+			return base.CanInlineIntoSlot(childIndex, expressionBeingMoved)
+				&& !expressionBeingMoved.HasFlag(InstructionFlags.MayUnwrapNull);
+		}
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -589,6 +589,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 			} else if (expr is Block block) {
 				// Inlining into inline-blocks?
 				switch (block.Kind) {
+					case BlockKind.ControlFlow when block.Parent is BlockContainer:
 					case BlockKind.ArrayInitializer:
 					case BlockKind.CollectionInitializer:
 					case BlockKind.ObjectInitializer:
@@ -604,19 +605,9 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					default:
 						return FindResult.Stop;
 				}
-			} else if (expr is BlockContainer container && container.EntryPoint.IncomingEdgeCount == 1) {
-				// Possibly a switch-container, allow inlining into the switch instruction:
-				return NoContinue(FindLoadInNext(container.EntryPoint.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 blockcontainer.
-			} else if (expr is NullableRewrap) {
-				// Inlining into nullable.rewrap is OK unless the expression being inlined
-				// contains a nullable.wrap that isn't being re-wrapped within the expression being inlined.
-				if (expressionBeingMoved.HasFlag(InstructionFlags.MayUnwrapNull))
-					return FindResult.Stop;
 			}
 			foreach (var child in expr.Children) {
-				if (!child.SlotInfo.CanInlineInto)
+				if (!expr.CanInlineIntoSlot(child.ChildIndex, expressionBeingMoved))
 					return FindResult.Stop;
 				
 				// Recursively try to find the load instruction
@@ -666,24 +657,33 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 		}
 
 		/// <summary>
-		/// Gets whether arg can be un-inlined out of stmt.
+		/// Gets whether 'expressionBeingMoved' can be moved from somewhere before 'stmt' to become the replacement of 'targetLoad'.
 		/// </summary>
-		/// <seealso cref="ILInstruction.Extract"/>
-		internal static bool CanUninline(ILInstruction arg, ILInstruction stmt)
+		public static bool CanMoveInto(ILInstruction expressionBeingMoved, ILInstruction stmt, ILInstruction targetLoad)
 		{
-			Debug.Assert(arg.IsDescendantOf(stmt));
-			for (ILInstruction inst = arg; inst != stmt; inst = inst.Parent) {
-				if (!inst.SlotInfo.CanInlineInto)
+			Debug.Assert(targetLoad.IsDescendantOf(stmt));
+			for (ILInstruction inst = targetLoad; inst != stmt; inst = inst.Parent) {
+				if (!inst.Parent.CanInlineIntoSlot(inst.ChildIndex, expressionBeingMoved))
 					return false;
 				// Check whether re-ordering with predecessors is valid:
 				int childIndex = inst.ChildIndex;
 				for (int i = 0; i < childIndex; ++i) {
 					ILInstruction predecessor = inst.Parent.Children[i];
-					if (!SemanticHelper.MayReorder(arg, predecessor))
+					if (!IsSafeForInlineOver(predecessor, expressionBeingMoved))
 						return false;
 				}
 			}
 			return true;
 		}
+
+		/// <summary>
+		/// Gets whether arg can be un-inlined out of stmt.
+		/// </summary>
+		/// <seealso cref="ILInstruction.Extract"/>
+		internal static bool CanUninline(ILInstruction arg, ILInstruction stmt)
+		{
+			// moving into and moving out-of are equivalent
+			return CanMoveInto(arg, stmt, arg);
+		}
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/IndexRangeTransform.cs

@@ -271,10 +271,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 					return;
 				}
 				if (rangeVar != null) {
-					if (!MatchIndexFromRange(startIndexKind, startIndexLoad, rangeVar, "get_Start"))
-						return;
-					if (!MatchIndexFromRange(endIndexKind, endIndexLoad, rangeVar, "get_End"))
-						return;
+					return; // this should only ever happen in the second step (ExtendSlicing)
 				}
 				if (!(sliceLengthVar.LoadInstructions.Single().Parent is CallInstruction call))
 					return;
@@ -291,9 +288,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				} else {
 					if (!call.Arguments[1].MatchLdLoc(startOffsetVar))
 						return;
+					if (!ILInlining.CanMoveInto(startOffsetVarInit, block.Instructions[pos], call.Arguments[1]))
+						return;
 				}
 				if (!call.Arguments[2].MatchLdLoc(sliceLengthVar))
 					return;
+				if (!ILInlining.CanMoveInto(sliceLengthVarInit, block.Instructions[pos], call.Arguments[2]))
+					return;
 				if (!CSharpWillGenerateIndexer(call.Method.DeclaringType, slicing: true))
 					return;
 				var specialMethods = new IndexMethods(context.TypeSystem);
@@ -385,13 +386,17 @@ namespace ICSharpCode.Decompiler.IL.Transforms
 				// holds because we've used containerLengthVar at least once
 				Debug.Assert(startIndexKind != IndexKind.FromStart || endIndexKind != IndexKind.FromStart);
 				if (rangeVar != null) {
+					if (!ILInlining.CanMoveInto(rangeVarInit, block.Instructions[pos], startIndexLoad))
+						return;
 					if (!MatchIndexFromRange(startIndexKind, startIndexLoad, rangeVar, "get_Start"))
 						return;
 					if (!MatchIndexFromRange(endIndexKind, endIndexLoad, rangeVar, "get_End"))
 						return;
 				}
-				context.Step("Merge containerLengthVar into slicing", slicingCall);
 				var specialMethods = new IndexMethods(context.TypeSystem);
+				if (!specialMethods.IsValid)
+					return;
+				context.Step("Merge containerLengthVar into slicing", slicingCall);
 				rangeCtorCall.ReplaceWith(MakeRange(startIndexKind, startIndexLoad, endIndexKind, endIndexLoad, specialMethods));
 				for (int i = startPos; i < pos; i++) {
 					slicingCall.AddILRange(block.Instructions[i]);