#2050: Allow inlining into the StObj target slot when this is possible without changing the program semantics.

Author: dgrunwald

ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs

@@ -139,7 +139,7 @@ public void Loops([ValueSource("defaultOptions")] CompilerOptions options)
 		[Test]
 		public void NullableTests([ValueSource("defaultOptions")] CompilerOptions options)
 		{
-			RunCS(options: options);
+			RunCS(options: options, forceRoslynRecompile: true);
 		}
 
 		[Test]
@@ -301,7 +301,7 @@ public void MiniJSON([ValueSource("defaultOptions")] CompilerOptions options)
 			RunCS(options: options);
 		}
 
-		void RunCS([CallerMemberName] string testName = null, CompilerOptions options = CompilerOptions.UseDebug)
+		void RunCS([CallerMemberName] string testName = null, CompilerOptions options = CompilerOptions.UseDebug, bool forceRoslynRecompile = false)
 		{
 			string testFileName = testName + ".cs";
 			string testOutputFileName = testName + Tester.GetSuffix(options) + ".exe";
@@ -311,7 +311,7 @@ void RunCS([CallerMemberName] string testName = null, CompilerOptions options =
 				outputFile = Tester.CompileCSharp(Path.Combine(TestCasePath, testFileName), options,
 					outputFileName: Path.Combine(TestCasePath, testOutputFileName));
 				string decompiledCodeFile = Tester.DecompileCSharp(outputFile.PathToAssembly, Tester.GetSettings(options));
-				if (options.HasFlag(CompilerOptions.UseMcs)) {
+				if (forceRoslynRecompile || options.HasFlag(CompilerOptions.UseMcs)) {
 					// For second pass, use roslyn instead of mcs.
 					// mcs has some compiler bugs that cause it to not accept ILSpy-generated code,
 					// for example when there's unreachable code due to other compiler bugs in the first mcs run.

ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs

@@ -27,6 +27,7 @@ static void Main()
 			AvoidLifting();
 			BitNot();
 			FieldAccessOrderOfEvaluation(null);
+			ArrayAccessOrderOfEvaluation();
 		}
 
 		static void AvoidLifting()
@@ -85,10 +86,10 @@ static void Assert(bool b)
 		}
 
 
-		static int GetInt()
+		static T GetValue<T>()
 		{
-			Console.WriteLine("got int");
-			return 42;
+			Console.WriteLine("GetValue");
+			return default(T);
 		}
 
 		int intField;
@@ -97,19 +98,59 @@ static void FieldAccessOrderOfEvaluation(NullableTests c)
 		{
 			Console.WriteLine("GetInt, then NRE:");
 			try {
-				c.intField = GetInt();
+				c.intField = GetValue<int>();
 			} catch (Exception ex) {
 				Console.WriteLine(ex.Message);
 			}
 			Console.WriteLine("NRE before GetInt:");
 			try {
 #if CS60
 				ref int i = ref c.intField;
-				i = GetInt();
+				i = GetValue<int>();
 #endif
 			} catch (Exception ex) {
 				Console.WriteLine(ex.Message);
 			}
 		}
+
+		static T[] GetArray<T>()
+		{
+			Console.WriteLine("GetArray");
+			return null;
+		}
+
+		static int GetIndex()
+		{
+			Console.WriteLine("GetIndex");
+			return 0;
+		}
+
+		static void ArrayAccessOrderOfEvaluation()
+		{
+			Console.WriteLine("GetArray direct:");
+			try {
+				GetArray<int>()[GetIndex()] = GetValue<int>();
+			} catch (Exception ex) {
+				Console.WriteLine(ex.Message);
+			}
+			Console.WriteLine("GetArray with ref:");
+			try {
+#if CS60
+				ref int elem = ref GetArray<int>()[GetIndex()];
+				elem = GetValue<int>();
+#endif
+			} catch (Exception ex) {
+				Console.WriteLine(ex.Message);
+			}
+			Console.WriteLine("GetArray direct with value-type:");
+			try {
+				// This line is mis-compiled by legacy csc:
+				// with the legacy compiler the NRE is thrown before the GetValue call;
+				// with Roslyn the NRE is thrown after the GetValue call.
+				GetArray<TimeSpan>()[GetIndex()] = GetValue<TimeSpan>();
+			} catch (Exception ex) {
+				Console.WriteLine(ex.Message);
+			}
+		}
 	}
 }

ICSharpCode.Decompiler/IL/Instructions.cs

@@ -4117,7 +4117,6 @@ internal override void CheckInvariant(ILPhase phase)
 			base.CheckInvariant(phase);
 			Debug.Assert(target.ResultType == StackType.Ref || target.ResultType == StackType.I);
 			Debug.Assert(value.ResultType == type.GetStackType());
-			Debug.Assert(IsValidTarget(target));
 		}
 	}
 }

ICSharpCode.Decompiler/IL/Instructions.tt

@@ -253,8 +253,7 @@
 		new OpCode("stobj", "Indirect store (store to ref/pointer)." + Environment.NewLine
 				+ "Evaluates to the value that was stored (when using type byte/short: evaluates to the truncated value, sign/zero extended back to I4 based on type.GetSign())",
 			CustomClassName("StObj"), CustomArguments(("target", new[] { "Ref", "I" }), ("value", new[] { "type.GetStackType()" })), HasTypeOperand, MemoryAccess, CustomWriteToButKeepOriginal,
-			SupportsVolatilePrefix, SupportsUnalignedPrefix, MayThrow, ResultType("type.GetStackType()"),
-			CustomInvariant("Debug.Assert(IsValidTarget(target));")),
+			SupportsVolatilePrefix, SupportsUnalignedPrefix, MayThrow, ResultType("type.GetStackType()")),
 
 		new OpCode("box", "Boxes a value.",
 			Unary, HasTypeOperand, MemoryAccess, MayThrow, ResultType("O")),

ICSharpCode.Decompiler/IL/Instructions/LdFlda.cs

@@ -52,13 +52,18 @@ public sealed partial class StObj
 		/// 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
 		/// </summary>
-		/// <remarks>This is part of the StObj invariant.</remarks>
-		public static bool IsValidTarget(ILInstruction inst)
+		public bool CanInlineIntoTargetSlot(ILInstruction inst)
 		{
 			switch (inst.OpCode) {
 				case OpCode.LdElema:
 				case OpCode.LdFlda:
-					return !inst.HasDirectFlag(InstructionFlags.MayThrow);
+					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;
 			}

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -581,7 +581,7 @@ internal static FindResult FindLoadInNext(ILInstruction expr, ILVariable v, ILIn
 				return FindResult.Stop;
 			if (expr.MatchLdLoc(v) || expr.MatchLdLoca(v)) {
 				// Match found, we can inline
-				if (expr.SlotInfo == StObj.TargetSlot && !StObj.IsValidTarget(expressionBeingMoved)) {
+				if (expr.SlotInfo == StObj.TargetSlot && !((StObj)expr.Parent).CanInlineIntoTargetSlot(expressionBeingMoved)) {
 					// special case: the StObj.TargetSlot does not accept some kinds of expressions
 					return FindResult.Stop;
 				}

ICSharpCode.Decompiler/IL/Transforms/StatementTransform.cs

@@ -45,6 +45,10 @@ public interface IStatementTransform
 		/// <remarks>
 		/// Instructions prior to block.Instructions[pos] must not be modified.
 		/// It is valid to read such instructions, but not recommended as those have not been transformed yet.
+		/// 
+		/// This function is only called on control-flow blocks with unreachable end-point.
+		/// Thus, the last instruction in the block always must have the EndPointUnreachable flag.
+		/// ==> Instructions with reachable end can't be last. Some transforms use this to save some bounds checks.
 		/// </remarks>
 		void Run(Block block, int pos, StatementTransformContext context);
 	}
@@ -122,6 +126,7 @@ public void Run(Block block, BlockTransformContext context)
 					ctx.rerunPosition = null;
 				}
 				foreach (var transform in children) {
+					Debug.Assert(block.HasFlag(InstructionFlags.EndPointUnreachable));
 					transform.Run(block, pos, ctx);
 #if DEBUG
 					block.Instructions[pos].CheckInvariant(ILPhase.Normal);

ILSpy/TextView/ILAsm-Mode.xshd

@@ -149,6 +149,7 @@
 			<Word>newarr</Word>
 			<Word>ldlen</Word>
 			<Word>ldelema</Word>
+			<Word>ldelem</Word>
 			<Word>ldelem.i1</Word>
 			<Word>ldelem.u1</Word>
 			<Word>ldelem.i2</Word>
@@ -160,6 +161,7 @@
 			<Word>ldelem.r4</Word>
 			<Word>ldelem.r8</Word>
 			<Word>ldelem.ref</Word>
+			<Word>stelem</Word>
 			<Word>stelem.i</Word>
 			<Word>stelem.i1</Word>
 			<Word>stelem.i2</Word>