Merge remote-tracking branch 'origin/master' into avalondock

ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs

@@ -280,6 +280,12 @@ namespace ICSharpCode.Decompiler.Tests
 			RunCS(options: options);
 		}
 
+		[Test]
+		public void StringConcat([ValueSource("defaultOptions")] CompilerOptions options)
+		{
+			RunCS(options: options);
+		}
+
 		[Test]
 		public void MiniJSON([ValueSource("defaultOptions")] CompilerOptions options)
 		{

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

@@ -79,6 +79,7 @@
 
   <ItemGroup>
     <Compile Include="DisassemblerPrettyTestRunner.cs" />
+    <Compile Include="TestCases\Correctness\StringConcat.cs" />
     <Compile Include="TestCases\ILPretty\ConstantBlobs.cs" />
     <Compile Include="TestCases\Pretty\OutVariables.cs" />
     <Compile Include="TestCases\Pretty\CustomTaskType.cs" />

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

@@ -0,0 +1,57 @@
+using System;
+
+namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
+{
+	class StringConcat
+	{
+		private class C
+		{
+			readonly int i;
+
+			public C(int i)
+			{
+				Console.WriteLine("  new C(" + i + ")");
+				this.i = i;
+			}
+
+			public override string ToString()
+			{
+				Console.WriteLine("  C(" + i + ").ToString()");
+				return i.ToString();
+			}
+		}
+
+		static string Space()
+		{
+			Console.WriteLine("  Space()");
+			return " ";
+		}
+
+		static void Main()
+		{
+			Console.WriteLine("string + C:");
+			Console.WriteLine(Space() + new C(1));
+
+			Console.WriteLine("C + string:");
+			Console.WriteLine(new C(2) + Space());
+
+			Console.WriteLine("C + string + C:");
+			Console.WriteLine(new C(3) + Space() + new C(4));
+
+			Console.WriteLine("string + C + C:");
+			Console.WriteLine(Space() + new C(5) + new C(6));
+
+			Console.WriteLine("string.Concat(C, string, C):");
+			Console.WriteLine(string.Concat(new C(10), Space(), new C(11)));
+
+			Console.WriteLine("string.Concat(string.Concat(C, string), C):");
+			Console.WriteLine(string.Concat(string.Concat(new C(15), Space()), new C(16)));
+
+			Console.WriteLine("string.Concat(C, string.Concat(string, C)):");
+			Console.WriteLine(string.Concat(new C(20), string.Concat(Space(), new C(21))));
+
+			Console.WriteLine("string.Concat(C, string) + C:");
+			Console.WriteLine(string.Concat(new C(30), Space()) + new C(31));
+		}
+	}
+}

ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoExtensionMethods.Expected.cs

@@ -19,5 +19,15 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly
 		{
 			return value;
 		}
+
+		internal static Func<int, int> ExtensionMethodAsStaticFunc()
+		{
+			return Return;
+		}
+
+		internal static Func<object> ExtensionMethodBoundToNull()
+		{
+			return new Func<object>(null, __ldftn(Return));
+		}
 	}
 }

ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoExtensionMethods.cs

@@ -13,5 +13,15 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Ugly
 		{
 			return value;
 		}
+
+		internal static Func<int, int> ExtensionMethodAsStaticFunc()
+		{
+			return Return;
+		}
+
+		internal static Func<object> ExtensionMethodBoundToNull()
+		{
+			return ((object)null).Return;
+		}
 	}
 }

ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoExtensionMethods.opt.roslyn.il

@@ -61,6 +61,30 @@
     IL_0001:  ret
   } // end of method NoExtensionMethods::Return
 
+  .method assembly hidebysig static class [mscorlib]System.Func`2<int32,int32> 
+          ExtensionMethodAsStaticFunc() cil managed
+  {
+    // Code size       13 (0xd)
+    .maxstack  8
+    IL_0000:  ldnull
+    IL_0001:  ldftn      !!0 ICSharpCode.Decompiler.Tests.TestCases.Ugly.NoExtensionMethods::Return<int32>(!!0)
+    IL_0007:  newobj     instance void class [mscorlib]System.Func`2<int32,int32>::.ctor(object,
+                                                                                         native int)
+    IL_000c:  ret
+  } // end of method NoExtensionMethods::ExtensionMethodAsStaticFunc
+
+  .method assembly hidebysig static class [mscorlib]System.Func`1<object> 
+          ExtensionMethodBoundToNull() cil managed
+  {
+    // Code size       13 (0xd)
+    .maxstack  8
+    IL_0000:  ldnull
+    IL_0001:  ldftn      !!0 ICSharpCode.Decompiler.Tests.TestCases.Ugly.NoExtensionMethods::Return<object>(!!0)
+    IL_0007:  newobj     instance void class [mscorlib]System.Func`1<object>::.ctor(object,
+                                                                                    native int)
+    IL_000c:  ret
+  } // end of method NoExtensionMethods::ExtensionMethodBoundToNull
+
 } // end of class ICSharpCode.Decompiler.Tests.TestCases.Ugly.NoExtensionMethods
 
 

ICSharpCode.Decompiler.Tests/TestCases/Ugly/NoExtensionMethods.roslyn.il

@@ -73,6 +73,42 @@
     IL_0006:  ret
   } // end of method NoExtensionMethods::Return
 
+  .method assembly hidebysig static class [mscorlib]System.Func`2<int32,int32> 
+          ExtensionMethodAsStaticFunc() cil managed
+  {
+    // Code size       18 (0x12)
+    .maxstack  2
+    .locals init (class [mscorlib]System.Func`2<int32,int32> V_0)
+    IL_0000:  nop
+    IL_0001:  ldnull
+    IL_0002:  ldftn      !!0 ICSharpCode.Decompiler.Tests.TestCases.Ugly.NoExtensionMethods::Return<int32>(!!0)
+    IL_0008:  newobj     instance void class [mscorlib]System.Func`2<int32,int32>::.ctor(object,
+                                                                                         native int)
+    IL_000d:  stloc.0
+    IL_000e:  br.s       IL_0010
+
+    IL_0010:  ldloc.0
+    IL_0011:  ret
+  } // end of method NoExtensionMethods::ExtensionMethodAsStaticFunc
+
+  .method assembly hidebysig static class [mscorlib]System.Func`1<object> 
+          ExtensionMethodBoundToNull() cil managed
+  {
+    // Code size       18 (0x12)
+    .maxstack  2
+    .locals init (class [mscorlib]System.Func`1<object> V_0)
+    IL_0000:  nop
+    IL_0001:  ldnull
+    IL_0002:  ldftn      !!0 ICSharpCode.Decompiler.Tests.TestCases.Ugly.NoExtensionMethods::Return<object>(!!0)
+    IL_0008:  newobj     instance void class [mscorlib]System.Func`1<object>::.ctor(object,
+                                                                                    native int)
+    IL_000d:  stloc.0
+    IL_000e:  br.s       IL_0010
+
+    IL_0010:  ldloc.0
+    IL_0011:  ret
+  } // end of method NoExtensionMethods::ExtensionMethodBoundToNull
+
 } // end of class ICSharpCode.Decompiler.Tests.TestCases.Ugly.NoExtensionMethods
 
 

ICSharpCode.Decompiler/CSharp/CallBuilder.cs

@@ -1235,12 +1235,38 @@ namespace ICSharpCode.Decompiler.CSharp
 				default:
 					throw new ArgumentException($"Unknown instruction type: {func.OpCode}");
 			}
-			if (method.IsStatic  && !method.IsExtensionMethod) {
+			if (CanUseDelegateConstruction(method, thisArg, inst.Method.DeclaringType.GetDelegateInvokeMethod())) {
+				return HandleDelegateConstruction(inst.Method.DeclaringType, method, expectedTargetDetails, thisArg, inst);
+			} else {
 				var argumentList = BuildArgumentList(expectedTargetDetails, null, inst.Method,
 					0, inst.Arguments, null);
 				return HandleConstructorCall(new ExpectedTargetDetails { CallOpCode = OpCode.NewObj }, null, inst.Method, argumentList).WithILInstruction(inst);
 			}
-			return HandleDelegateConstruction(inst.Method.DeclaringType, method, expectedTargetDetails, thisArg, inst);
+		}
+
+		private bool CanUseDelegateConstruction(IMethod targetMethod, ILInstruction thisArg, IMethod invokeMethod)
+		{
+			if (targetMethod.IsStatic) {
+				// If the invoke method is known, we can compare the parameter counts to figure out whether the
+				// delegate is static or binds the first argument
+				if (invokeMethod != null) {
+					if (invokeMethod.Parameters.Count == targetMethod.Parameters.Count) {
+						return thisArg.MatchLdNull();
+					} else if (targetMethod.IsExtensionMethod && invokeMethod.Parameters.Count == targetMethod.Parameters.Count - 1) {
+						return true;
+					} else {
+						return false;
+					}
+				} else {
+					// delegate type unknown:
+					return thisArg.MatchLdNull() || targetMethod.IsExtensionMethod;
+				}
+			} else {
+				// targetMethod is instance method
+				if (invokeMethod != null && invokeMethod.Parameters.Count != targetMethod.Parameters.Count)
+					return false;
+				return true;
+			}
 		}
 
 		internal TranslatedExpression Build(LdVirtDelegate inst)

ICSharpCode.Decompiler/CSharp/Transforms/ReplaceMethodCallsWithOperators.cs

@@ -23,7 +23,9 @@ using System.Reflection;
 using System.Text;
 using ICSharpCode.Decompiler.CSharp.Syntax;
 using ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching;
+using ICSharpCode.Decompiler.Semantics;
 using ICSharpCode.Decompiler.TypeSystem;
+using ICSharpCode.Decompiler.Util;
 
 namespace ICSharpCode.Decompiler.CSharp.Transforms
 {
@@ -56,7 +58,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 			var arguments = invocationExpression.Arguments.ToArray();
 
 			// Reduce "String.Concat(a, b)" to "a + b"
-			if (method.Name == "Concat" && method.DeclaringType.FullName == "System.String" && CheckArgumentsForStringConcat(arguments)) {
+			if (IsStringConcat(method) && CheckArgumentsForStringConcat(arguments)) {
 				invocationExpression.Arguments.Clear(); // detach arguments from invocationExpression
 				Expression expr = arguments[0];
 				for (int i = 1; i < arguments.Length; i++) {
@@ -174,9 +176,77 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 			if (arguments.Length < 2)
 				return false;
 
-			return !arguments.Any(arg => arg is NamedArgumentExpression) &&
-				(arguments[0].GetResolveResult().Type.IsKnownType(KnownTypeCode.String) ||
-				arguments[1].GetResolveResult().Type.IsKnownType(KnownTypeCode.String));
+			if (arguments.Any(arg => arg is NamedArgumentExpression))
+				return false;
+
+			// The evaluation order when the object.ToString() calls happen is a mess:
+			// The C# spec says the evaluation for order for each individual string + should be:
+			//   * evaluate left argument
+			//   * evaluate right argument
+			//   * call ToString() on object argument
+			// What actually happens pre-VS2019.3:
+			//   * evaluate all arguments in chain of + operators from left to right
+			//   * call ToString() on all object arguments from left to right
+			// What happens in VS2019.3:
+			//   * for each argument in chain of + operators fom left to right:
+			//       * evaluate argument
+			//       * call ToString() on object argument
+			// See https://github.com/dotnet/roslyn/issues/38641 for details.
+			// To ensure the decompiled code's behavior matches the original IL behavior,
+			// no matter which compiler is used to recompile it, we require that all
+			// implicit ToString() calls except for the last are free of side effects.
+			foreach (var arg in arguments.SkipLast(1)) {
+				if (!ToStringIsKnownEffectFree(arg.GetResolveResult().Type)) {
+					return false;
+				}
+			}
+			foreach (var arg in arguments) {
+				if (arg.GetResolveResult() is InvocationResolveResult rr && IsStringConcat(rr.Member)) {
+					// Roslyn + mcs also flatten nested string.Concat() invocations within a operator+ use,
+					// which causes it to use the incorrect evaluation order despite the code using an
+					// explicit string.Concat() call.
+					// This problem is avoided if the outer call remains string.Concat() as well.
+					return false;
+				}
+			}
+
+			// One of the first two arguments must be string, otherwise the + operator
+			// won't resolve to a string concatenation.
+			return arguments[0].GetResolveResult().Type.IsKnownType(KnownTypeCode.String)
+				|| arguments[1].GetResolveResult().Type.IsKnownType(KnownTypeCode.String);
+		}
+
+		private bool IsStringConcat(IParameterizedMember member)
+		{
+			return member is IMethod method
+				&& method.Name == "Concat"
+				&& method.DeclaringType.IsKnownType(KnownTypeCode.String);
+		}
+
+		static bool ToStringIsKnownEffectFree(IType type)
+		{
+			type = NullableType.GetUnderlyingType(type);
+			switch (type.GetDefinition()?.KnownTypeCode) {
+				case KnownTypeCode.Boolean:
+				case KnownTypeCode.Char:
+				case KnownTypeCode.SByte:
+				case KnownTypeCode.Byte:
+				case KnownTypeCode.Int16:
+				case KnownTypeCode.UInt16:
+				case KnownTypeCode.Int32:
+				case KnownTypeCode.UInt32:
+				case KnownTypeCode.Int64:
+				case KnownTypeCode.UInt64:
+				case KnownTypeCode.Single:
+				case KnownTypeCode.Double:
+				case KnownTypeCode.Decimal:
+				case KnownTypeCode.IntPtr:
+				case KnownTypeCode.UIntPtr:
+				case KnownTypeCode.String:
+					return true;
+				default:
+					return false;
+			}
 		}
 
 		static BinaryOperatorType? GetBinaryOperatorTypeFromMetadataName(string name)

ILSpy.AddIn/ILSpyInstance.cs

@@ -42,7 +42,8 @@ namespace ICSharpCode.ILSpy.AddIn
 			var process = new Process() {
 				StartInfo = new ProcessStartInfo() {
 					FileName = GetILSpyPath(),
-					UseShellExecute = false
+					UseShellExecute = false,
+					Arguments = "/navigateTo:none"
 				}
 			};
 			process.Start();

azure-pipelines.yml

@@ -39,7 +39,7 @@ jobs:
 
   - task: DotNetCoreInstaller@0
     inputs:
-      version: '3.0.100-preview9-014004'
+      version: '3.0.100'
 
   - powershell: .\BuildTools\pipelines-install.ps1
     displayName: Install