Add transform to remove unconstrained generic reference type check.

Author: siegfriedpammer

ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@@ -17,7 +17,7 @@
     <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
 
     <NoWarn>1701;1702;1705,67,169,1058,728,1720,649,168,251,660,661,675;1998;162;8632;626;8618;8714;8602;8981</NoWarn>
-    <DefineConstants>ROSLYN;NET60;CS60;CS70;CS71;CS72;CS73;CS80;CS90;CS100;CS110;CS120</DefineConstants>
+    <DefineConstants>ROSLYN;ROSLYN2;ROSLYN3;ROSLYN4;NET60;CS60;CS70;CS71;CS72;CS73;CS80;CS90;CS100;CS110;CS120</DefineConstants>
 
     <GenerateAssemblyVersionAttribute>False</GenerateAssemblyVersionAttribute>
     <GenerateAssemblyFileVersionAttribute>False</GenerateAssemblyFileVersionAttribute>

ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs

@@ -84,6 +84,11 @@ public interface IInterface
 		{
 			void Method1<T>() where T : class;
 			void Method2<T>() where T : class;
+
+			void Method3(int a, string b, Type c);
+#if CS72
+			void Method4(in int a);
+#endif
 		}
 
 		public abstract class Base : IInterface
@@ -95,6 +100,16 @@ public abstract class Base : IInterface
 			void IInterface.Method2<T>()
 			{
 			}
+
+			void IInterface.Method3(int a, string b, Type c)
+			{
+			}
+
+#if CS72
+			void IInterface.Method4(in int a)
+			{
+			}
+#endif
 		}
 
 		public class Derived : Base
@@ -302,5 +317,38 @@ public static void ConstrainedCall<T>(T x, ref T y) where T : IDisposable
 			x.Dispose();
 			y.Dispose();
 		}
+
+		// prior to C# 7.0 UseRefLocalsForAccurateOrderOfEvaluation is disabled, so we will inline.
+		// Roslyn 4 generates the explicit ldobj.if.ref pattern, so we can also inline.
+		// The versions in between, we don't inline, so the code doesn't look pretty.
+#if ROSLYN4 || !CS70
+		public static int[] Issue3438<T>(T[] array)
+		{
+			List<int> list = new List<int>();
+			for (int i = 0; i < array.Length; i++)
+			{
+				if (!array[i].Equals(default(T)))
+				{
+					list.Add(i);
+				}
+			}
+			return list.ToArray();
+		}
+		public void Issue3438b<T>(T[] item1, T item2, int item3) where T : IInterface
+		{
+			item1[CastToInt(item2)].Method3(CastToInt(item2), CastToString(item2), TestTypeOf()[1]);
+		}
+		public void Issue3438c<T>(T item, T item2, int item3) where T : IInterface
+		{
+			CastFromInt<T>(item3).Method3(CastToInt(item2), CastToString(item2), TestTypeOf()[1]);
+		}
+		//#if CS72
+		// Disabled because ILInlining does not support inlining ldloca currently.
+		//		public void Issue3438d<T>(T[] item1, T item2, int item3) where T : IInterface
+		//		{
+		//			item1[CastToInt(item2)].Method4(CastToInt(item2));
+		//		}
+		//#endif
+#endif
 	}
 }

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -149,6 +149,7 @@ public static List<IILTransform> GetILTransforms()
 							new IndexRangeTransform(),
 							new DeconstructionTransform(),
 							new NamedArgumentTransform(),
+							new RemoveUnconstrainedGenericReferenceTypeCheck(),
 							new UserDefinedLogicTransform(),
 							new InterpolatedStringTransform()
 						),
@@ -1717,6 +1718,7 @@ void DecompileBody(IMethod method, EntityDeclaration entityDecl, DecompileRun de
 			{
 				var ilReader = new ILReader(typeSystem.MainModule) {
 					UseDebugSymbols = settings.UseDebugSymbols,
+					UseRefLocalsForAccurateOrderOfEvaluation = settings.UseRefLocalsForAccurateOrderOfEvaluation,
 					DebugInfo = DebugInfoProvider
 				};
 				var methodDef = metadata.GetMethodDefinition((MethodDefinitionHandle)method.MetadataToken);

ICSharpCode.Decompiler/DecompilerSettings.cs

@@ -92,7 +92,6 @@ public void SetLanguageVersion(CSharp.LanguageVersion languageVersion)
 				stringInterpolation = false;
 				dictionaryInitializers = false;
 				extensionMethodsInCollectionInitializers = false;
-				useRefLocalsForAccurateOrderOfEvaluation = false;
 				getterOnlyAutomaticProperties = false;
 			}
 			if (languageVersion < CSharp.LanguageVersion.CSharp7)
@@ -105,6 +104,7 @@ public void SetLanguageVersion(CSharp.LanguageVersion languageVersion)
 				localFunctions = false;
 				deconstruction = false;
 				patternMatching = false;
+				useRefLocalsForAccurateOrderOfEvaluation = false;
 			}
 			if (languageVersion < CSharp.LanguageVersion.CSharp7_2)
 			{
@@ -190,11 +190,11 @@ public CSharp.LanguageVersion GetMinimumRequiredVersion()
 				return CSharp.LanguageVersion.CSharp7_2;
 			// C# 7.1 missing
 			if (outVariables || throwExpressions || tupleTypes || tupleConversions
-				|| discards || localFunctions || deconstruction || patternMatching)
+				|| discards || localFunctions || deconstruction || patternMatching || useRefLocalsForAccurateOrderOfEvaluation)
 				return CSharp.LanguageVersion.CSharp7;
 			if (awaitInCatchFinally || useExpressionBodyForCalculatedGetterOnlyProperties || nullPropagation
 				|| stringInterpolation || dictionaryInitializers || extensionMethodsInCollectionInitializers
-				|| useRefLocalsForAccurateOrderOfEvaluation || getterOnlyAutomaticProperties)
+				|| getterOnlyAutomaticProperties)
 				return CSharp.LanguageVersion.CSharp6;
 			if (asyncAwait)
 				return CSharp.LanguageVersion.CSharp5;
@@ -1126,7 +1126,7 @@ public bool ExtensionMethodsInCollectionInitializers {
 		/// order of evaluation.
 		/// See https://github.com/icsharpcode/ILSpy/issues/2050
 		/// </summary>
-		[Category("C# 6.0 / VS 2015")]
+		[Category("C# 7.0 / VS 2017")]
 		[Description("DecompilerSettings.UseRefLocalsForAccurateOrderOfEvaluation")]
 		public bool UseRefLocalsForAccurateOrderOfEvaluation {
 			get { return useRefLocalsForAccurateOrderOfEvaluation; }

ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj

@@ -107,6 +107,7 @@
     <Compile Include="DecompilationProgress.cs" />
     <Compile Include="Disassembler\IEntityProcessor.cs" />
     <Compile Include="Disassembler\SortByNameProcessor.cs" />
+    <Compile Include="IL\Transforms\RemoveUnconstrainedGenericReferenceTypeCheck.cs" />
     <Compile Include="Metadata\MetadataFile.cs" />
     <Compile Include="Metadata\ModuleReferenceMetadata.cs" />
     <Compile Include="NRTAttributes.cs" />

ICSharpCode.Decompiler/IL/ILReader.cs

@@ -122,6 +122,7 @@ internal void ResetForReimport()
 		readonly MetadataReader metadata;
 
 		public bool UseDebugSymbols { get; set; }
+		public bool UseRefLocalsForAccurateOrderOfEvaluation { get; set; }
 		public DebugInfo.IDebugInfoProvider? DebugInfo { get; set; }
 		public List<string> Warnings { get; } = new List<string>();
 
@@ -1770,6 +1771,7 @@ ILInstruction[] PrepareArguments(bool firstArgumentIsStObjTarget)
 				StackType expectedStackType = CallInstruction.ExpectedTypeForThisPointer(method.DeclaringType, constrainedPrefix);
 				bool requiresLdObjIfRef = firstArgument == 1
 					&& !firstArgumentIsStObjTarget
+					&& UseRefLocalsForAccurateOrderOfEvaluation
 					&& expectedStackType == StackType.Ref && typeOfThis.IsReferenceType != false;
 				for (int i = method.Parameters.Count - 1; i >= 0; i--)
 				{

ICSharpCode.Decompiler/IL/Instructions/PatternMatching.cs

@@ -581,5 +581,17 @@ public virtual ILInstruction UnwrapConv(ConversionKind kind)
 		{
 			return this;
 		}
+
+		public bool MatchLdObj([NotNullWhen(true)] out ILInstruction? target, IType type)
+		{
+			var inst = this as LdObj;
+			if (inst != null && inst.Type.Equals(type))
+			{
+				target = inst.Target;
+				return true;
+			}
+			target = default(ILInstruction);
+			return false;
+		}
 	}
 }

ICSharpCode.Decompiler/IL/Transforms/ExpressionTransforms.cs

@@ -485,6 +485,16 @@ protected internal override void VisitLdObjIfRef(LdObjIfRef inst)
 				inst.ReplaceWith(inst.Target);
 				return;
 			}
+			if (inst.Target.MatchLdLoc(out var s) && s.IsSingleDefinition && s.LoadCount == 1
+				&& s.StoreInstructions.SingleOrDefault() is StLoc {
+					Value: LdLoca { Variable: { AddressCount: 1, StoreCount: 1 } }
+				})
+			{
+				context.Step("Single use of ldobj.if.ref(ldloc v) -> ldloc v", inst);
+				// there already is a temporary, so the ldobj.if.ref is a no-op in both cases
+				inst.ReplaceWith(inst.Target);
+				return;
+			}
 		}
 
 		protected internal override void VisitStObj(StObj inst)

ICSharpCode.Decompiler/IL/Transforms/IILTransform.cs

@@ -79,6 +79,7 @@ internal ILReader CreateILReader()
 		{
 			return new ILReader(TypeSystem.MainModule) {
 				UseDebugSymbols = Settings.UseDebugSymbols,
+				UseRefLocalsForAccurateOrderOfEvaluation = Settings.UseRefLocalsForAccurateOrderOfEvaluation,
 				DebugInfo = DebugInfo
 			};
 		}

ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@@ -170,14 +170,9 @@ public static bool InlineIfPossible(Block block, int pos, ILTransformContext con
 		public static bool InlineOneIfPossible(Block block, int pos, InliningOptions options, ILTransformContext context)
 		{
 			context.CancellationToken.ThrowIfCancellationRequested();
-			StLoc stloc = block.Instructions[pos] as StLoc;
-			if (stloc == null || stloc.Variable.Kind == VariableKind.PinnedLocal)
+			if (block.Instructions[pos] is not StLoc stloc)
 				return false;
-			ILVariable v = stloc.Variable;
-			// ensure the variable is accessed only a single time
-			if (v.StoreCount != 1)
-				return false;
-			if (v.LoadCount > 1 || v.LoadCount + v.AddressCount != 1)
+			if (!VariableCanBeUsedForInlining(stloc.Variable))
 				return false;
 			// TODO: inlining of small integer types might be semantically incorrect,
 			// but we can't avoid it this easily without breaking lots of tests.
@@ -186,6 +181,18 @@ public static bool InlineOneIfPossible(Block block, int pos, InliningOptions opt
 			return InlineOne(stloc, options, context);
 		}
 
+		public static bool VariableCanBeUsedForInlining(ILVariable v)
+		{
+			if (v.Kind == VariableKind.PinnedLocal)
+				return false;
+			// ensure the variable is accessed only a single time
+			if (v.StoreCount != 1)
+				return false;
+			if (v.LoadCount + v.AddressCount != 1)
+				return false;
+			return true;
+		}
+
 		/// <summary>
 		/// Inlines the stloc instruction at block.Instructions[pos] into the next instruction.
 		/// 
@@ -908,6 +915,30 @@ public static bool CanMoveInto(ILInstruction expressionBeingMoved, ILInstruction
 			return true;
 		}
 
+		/// <summary>
+		/// Gets whether 'expressionBeingMoved' can be moved from somewhere before 'stmt' to become the replacement of 'targetLoad'.
+		/// </summary>
+		public static bool CanMoveIntoCallVirt(ILInstruction expressionBeingMoved, ILInstruction stmt, CallVirt nestedCallVirt, ILInstruction targetLoad)
+		{
+			Debug.Assert(targetLoad.IsDescendantOf(stmt) && nestedCallVirt.IsDescendantOf(stmt));
+			ILInstruction thisArg = nestedCallVirt.Arguments[0];
+			Debug.Assert(thisArg is LdObjIfRef);
+			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 (predecessor != thisArg && !IsSafeForInlineOver(predecessor, expressionBeingMoved))
+						return false;
+				}
+			}
+			return true;
+		}
+
 		/// <summary>
 		/// Gets whether arg can be un-inlined out of stmt.
 		/// </summary>