From eece44d36133797d895da5a6d7e19a8573da91e8 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 5 Nov 2017 00:38:12 +0100 Subject: [PATCH 1/2] Avoid unnecessary (object) cast when calling myEnum.ToString() --- .gitignore | 1 + .../TestCases/Correctness/ValueTypeCall.cs | 15 +++++ ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 57 ++++++++++++++----- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index 86be8d458..e5dcba17b 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ _ReSharper*/ .vs/ /ILSpy.AddIn/Packages/* /ICSharpCode.Decompiler.Tests/TestCases/Correctness/*.exe +/ICSharpCode.Decompiler/ICSharpCode.Decompiler.nuspec diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/ValueTypeCall.cs b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/ValueTypeCall.cs index 5a8ad0bd6..0a8250c92 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Correctness/ValueTypeCall.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Correctness/ValueTypeCall.cs @@ -17,6 +17,11 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness { Console.WriteLine("MutValueType disposed on {0}", val); } + + public override string ToString() + { + return "MutValueType.ToString() " + (++val); + } } public struct GenericValueType @@ -56,6 +61,7 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness ValueParameter(m); Field(); Box(); + BoxToStringCalls(); Using(); var gvt = new GenericValueType("Test"); gvt.Call(ref gvt); @@ -109,6 +115,15 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness ((MutValueType)o).Dispose(); } + static void BoxToStringCalls() + { + Console.WriteLine("BoxToStringCalls:"); + MutValueType m = new MutValueType { val = 400 }; + Console.WriteLine(m.ToString()); + Console.WriteLine(((object)m).ToString()); + Console.WriteLine(m.ToString()); + } + MutValueType instanceField; ValueTypeWithReadOnlyMember mutableInstanceFieldWithReadOnlyMember; diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index 33dfb7845..6b3f735a1 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -32,6 +32,12 @@ namespace ICSharpCode.Decompiler.CSharp { struct CallBuilder { + struct ExpectedTargetDetails + { + public OpCode CallOpCode; + public bool NeedsBoxingConversion; + } + readonly DecompilerSettings settings; readonly ExpressionBuilder expressionBuilder; readonly CSharpResolver resolver; @@ -50,17 +56,34 @@ namespace ICSharpCode.Decompiler.CSharp if (inst is NewObj newobj && IL.Transforms.DelegateConstruction.IsDelegateConstruction(newobj, true)) { return HandleDelegateConstruction(newobj); } - return Build(inst.OpCode, inst.Method, inst.Arguments).WithILInstruction(inst); + return Build(inst.OpCode, inst.Method, inst.Arguments, inst.ConstrainedTo).WithILInstruction(inst); } - public ExpressionWithResolveResult Build(OpCode callOpCode, IMethod method, IReadOnlyList callArguments) + public ExpressionWithResolveResult Build(OpCode callOpCode, IMethod method, IReadOnlyList callArguments, + IType constrainedTo = null) { // Used for Call, CallVirt and NewObj + var expectedTargetDetails = new ExpectedTargetDetails { + CallOpCode = callOpCode + }; TranslatedExpression target; if (callOpCode == OpCode.NewObj) { target = default(TranslatedExpression); // no target } else { target = expressionBuilder.TranslateTarget(method, callArguments.FirstOrDefault(), callOpCode == OpCode.Call); + if (callOpCode == OpCode.CallVirt + && constrainedTo == null + && target.Expression is CastExpression cast + && target.ResolveResult is ConversionResolveResult conversion + && target.Type.IsKnownType(KnownTypeCode.Object) + && conversion.Conversion.IsBoxingConversion) + { + // boxing conversion on call target? + // let's see if we can make that implicit: + target = target.UnwrapChild(cast.Expression); + // we'll need to make sure the boxing effect is preserved + expectedTargetDetails.NeedsBoxingConversion = true; + } } int firstParamIndex = (method.IsStatic || callOpCode == OpCode.NewObj) ? 0 : 1; @@ -94,7 +117,7 @@ namespace ICSharpCode.Decompiler.CSharp expandedArguments.Add(expressionBuilder.GetDefaultValueExpression(elementType).WithoutILInstruction()); } } - if (IsUnambiguousCall(callOpCode, target, method, Array.Empty(), expandedArguments) == OverloadResolutionErrors.None) { + if (IsUnambiguousCall(expectedTargetDetails, method, target, Array.Empty(), expandedArguments) == OverloadResolutionErrors.None) { isExpandedForm = true; expectedParameters = expandedParameters; arguments = expandedArguments.SelectList(a => new TranslatedExpression(a.Expression.Detach())); @@ -147,7 +170,7 @@ namespace ICSharpCode.Decompiler.CSharp .WithRR(rr); } else { - if (IsUnambiguousCall(callOpCode, target, method, Array.Empty(), arguments) != OverloadResolutionErrors.None) { + if (IsUnambiguousCall(expectedTargetDetails, method, target, Array.Empty(), arguments) != OverloadResolutionErrors.None) { for (int i = 0; i < arguments.Count; i++) { if (settings.AnonymousTypes && expectedParameters[i].Type.ContainsAnonymousType()) { if (arguments[i].Expression is LambdaExpression lambda) { @@ -164,7 +187,7 @@ namespace ICSharpCode.Decompiler.CSharp } else { int allowedParamCount = (method.ReturnType.IsKnownType(KnownTypeCode.Void) ? 1 : 0); if (method.IsAccessor && (method.AccessorOwner.SymbolKind == SymbolKind.Indexer || expectedParameters.Count == allowedParamCount)) { - return HandleAccessorCall(callOpCode == OpCode.CallVirt, target, method, arguments.ToList()); + return HandleAccessorCall(expectedTargetDetails, method, target, arguments.ToList()); } else if (method.Name == "Invoke" && method.DeclaringType.Kind == TypeKind.Delegate) { return new InvocationExpression(target, arguments.Select(arg => arg.Expression)).WithRR(rr); } else if (IsDelegateEqualityComparison(method, arguments)) { @@ -179,7 +202,7 @@ namespace ICSharpCode.Decompiler.CSharp IType[] typeArguments = Array.Empty(); OverloadResolutionErrors errors; - while ((errors = IsUnambiguousCall(callOpCode, target, method, typeArguments, arguments)) != OverloadResolutionErrors.None) { + while ((errors = IsUnambiguousCall(expectedTargetDetails, method, target, typeArguments, arguments)) != OverloadResolutionErrors.None) { switch (errors) { case OverloadResolutionErrors.TypeInferenceFailed: case OverloadResolutionErrors.WrongNumberOfTypeArguments: @@ -288,11 +311,12 @@ namespace ICSharpCode.Decompiler.CSharp .WithRR(new ConversionResolveResult(targetType, argument.ResolveResult, conv)); } - OverloadResolutionErrors IsUnambiguousCall(OpCode callOpCode, TranslatedExpression target, IMethod method, IType[] typeArguments, IList arguments) + OverloadResolutionErrors IsUnambiguousCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, + TranslatedExpression target, IType[] typeArguments, IList arguments) { var lookup = new MemberLookup(resolver.CurrentTypeDefinition, resolver.CurrentTypeDefinition.ParentAssembly); var or = new OverloadResolution(resolver.Compilation, arguments.SelectArray(a => a.ResolveResult), typeArguments: typeArguments); - if (callOpCode == OpCode.NewObj) { + if (expectedTargetDetails.CallOpCode == OpCode.NewObj) { foreach (IMethod ctor in method.DeclaringType.GetConstructors()) { if (lookup.IsAccessible(ctor, allowProtectedAccess: resolver.CurrentTypeDefinition == method.DeclaringTypeDefinition)) { or.AddCandidate(ctor); @@ -306,7 +330,7 @@ namespace ICSharpCode.Decompiler.CSharp } if (or.BestCandidateErrors != OverloadResolutionErrors.None) return or.BestCandidateErrors; - if (!IsAppropriateCallTarget(method, or.GetBestCandidateWithSubstitutedTypeArguments(), callOpCode == OpCode.CallVirt)) + if (!IsAppropriateCallTarget(expectedTargetDetails, method, or.GetBestCandidateWithSubstitutedTypeArguments())) return OverloadResolutionErrors.AmbiguousMatch; return OverloadResolutionErrors.None; } @@ -329,12 +353,12 @@ namespace ICSharpCode.Decompiler.CSharp return true; } - ExpressionWithResolveResult HandleAccessorCall(bool isVirtCall, TranslatedExpression target, IMethod method, IList arguments) + ExpressionWithResolveResult HandleAccessorCall(ExpectedTargetDetails expectedTargetDetails, IMethod method, TranslatedExpression target, IList arguments) { var lookup = new MemberLookup(resolver.CurrentTypeDefinition, resolver.CurrentTypeDefinition.ParentAssembly); var result = lookup.Lookup(target.ResolveResult, method.AccessorOwner.Name, EmptyList.Instance, isInvocation: false); - if (result.IsError || (result is MemberResolveResult && !IsAppropriateCallTarget(method.AccessorOwner, ((MemberResolveResult)result).Member, isVirtCall))) + if (result.IsError || (result is MemberResolveResult && !IsAppropriateCallTarget(expectedTargetDetails, method.AccessorOwner, ((MemberResolveResult)result).Member))) target = target.ConvertTo(method.AccessorOwner.DeclaringType, expressionBuilder); var rr = new MemberResolveResult(target.ResolveResult, method.AccessorOwner); @@ -367,12 +391,14 @@ namespace ICSharpCode.Decompiler.CSharp } } - bool IsAppropriateCallTarget(IMember expectedTarget, IMember actualTarget, bool isVirtCall) + bool IsAppropriateCallTarget(ExpectedTargetDetails expectedTargetDetails, IMember expectedTarget, IMember actualTarget) { if (expectedTarget.Equals(actualTarget)) return true; - if (isVirtCall && actualTarget.IsOverride) { + if (expectedTargetDetails.CallOpCode == OpCode.CallVirt && actualTarget.IsOverride) { + if (expectedTargetDetails.NeedsBoxingConversion && actualTarget.DeclaringType.IsReferenceType != true) + return false; foreach (var possibleTarget in InheritanceHelper.GetBaseMembers(actualTarget, false)) { if (expectedTarget.Equals(possibleTarget)) return true; @@ -418,7 +444,10 @@ namespace ICSharpCode.Decompiler.CSharp bool needsCast = true; if (result is MethodGroupResolveResult mgrr) { or.AddMethodLists(mgrr.MethodsGroupedByDeclaringType.ToArray()); - needsCast = (or.BestCandidateErrors != OverloadResolutionErrors.None || !IsAppropriateCallTarget(method, or.BestCandidate, func.OpCode == OpCode.LdVirtFtn)); + var expectedTargetDetails = new ExpectedTargetDetails { + CallOpCode = inst.OpCode + }; + needsCast = (or.BestCandidateErrors != OverloadResolutionErrors.None || !IsAppropriateCallTarget(expectedTargetDetails, method, or.BestCandidate)); } if (needsCast) { target = target.ConvertTo(targetType, expressionBuilder); From 7af15d27cddcd6c4ff8ab8db48d8f0bb4fee7689 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Sun, 5 Nov 2017 00:44:32 +0100 Subject: [PATCH 2/2] Fix #905: improve exit point detection for foreach loops. --- .../TestCases/Pretty/Loops.cs | 10 +++ .../TestCases/Pretty/Loops.il | 83 ++++++++++++++++++- .../TestCases/Pretty/Loops.opt.il | 65 ++++++++++++++- .../TestCases/Pretty/Loops.opt.roslyn.il | 61 +++++++++++++- .../TestCases/Pretty/Loops.roslyn.il | 70 +++++++++++++++- .../IL/ControlFlow/LoopDetection.cs | 56 ++++++++++++- ICSharpCode.Decompiler/IL/Instructions.cs | 2 +- ICSharpCode.Decompiler/IL/Instructions.tt | 2 +- .../IL/Instructions/Block.cs | 4 - .../IL/Transforms/NullableLiftingTransform.cs | 11 +-- .../Util/ExtensionMethods.cs | 9 +- 11 files changed, 343 insertions(+), 30 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.cs index e67507cb6..5134c7e42 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.cs @@ -430,6 +430,16 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty Console.WriteLine(new IntPtr(value)); } } + + public void ForEachBreakWhenFound(string name, ref StringComparison output) + { + foreach (StringComparison value in Enum.GetValues(typeof(StringComparison))) { + if (value.ToString() == name) { + output = value; + break; + } + } + } #endregion public void ForOverArray(string[] array) diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.il b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.il index 90c44f2f8..f976c663a 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.il @@ -10,7 +10,7 @@ .publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4.. .ver 4:0:0:0 } -.assembly vsvybxtk +.assembly n3qdq2uj { .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilationRelaxationsAttribute::.ctor(int32) = ( 01 00 08 00 00 00 00 00 ) .custom instance void [mscorlib]System.Runtime.CompilerServices.RuntimeCompatibilityAttribute::.ctor() = ( 01 00 01 00 54 02 16 57 72 61 70 4E 6F 6E 45 78 // ....T..WrapNonEx @@ -20,15 +20,15 @@ .hash algorithm 0x00008004 .ver 0:0:0:0 } -.module vsvybxtk.dll -// MVID: {6C5236E6-826F-4D2D-AF03-574254796A89} +.module n3qdq2uj.dll +// MVID: {98B4F05B-5E30-48EB-AAF5-A90EA42A94A2} .custom instance void [mscorlib]System.Security.UnverifiableCodeAttribute::.ctor() = ( 01 00 00 00 ) .imagebase 0x10000000 .file alignment 0x00000200 .stackreserve 0x00100000 .subsystem 0x0003 // WINDOWS_CUI .corflags 0x00000001 // ILONLY -// Image base: 0x02F80000 +// Image base: 0x02AD0000 // =============== CLASS MEMBERS DECLARATION =================== @@ -1594,6 +1594,81 @@ IL_003e: ret } // end of method Loops::ForEachOverArrayOfPointers + .method public hidebysig instance void + ForEachBreakWhenFound(string name, + valuetype [mscorlib]System.StringComparison& output) cil managed + { + // Code size 106 (0x6a) + .maxstack 2 + .locals init (valuetype [mscorlib]System.StringComparison V_0, + class [mscorlib]System.Collections.IEnumerator V_1, + bool V_2, + class [mscorlib]System.IDisposable V_3) + IL_0000: nop + IL_0001: nop + IL_0002: ldtoken [mscorlib]System.StringComparison + IL_0007: call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle) + IL_000c: call class [mscorlib]System.Array [mscorlib]System.Enum::GetValues(class [mscorlib]System.Type) + IL_0011: callvirt instance class [mscorlib]System.Collections.IEnumerator [mscorlib]System.Array::GetEnumerator() + IL_0016: stloc.1 + .try + { + IL_0017: br.s IL_0045 + + IL_0019: ldloc.1 + IL_001a: callvirt instance object [mscorlib]System.Collections.IEnumerator::get_Current() + IL_001f: unbox.any [mscorlib]System.StringComparison + IL_0024: stloc.0 + IL_0025: nop + IL_0026: ldloc.0 + IL_0027: box [mscorlib]System.StringComparison + IL_002c: callvirt instance string [mscorlib]System.Object::ToString() + IL_0031: ldarg.1 + IL_0032: call bool [mscorlib]System.String::op_Equality(string, + string) + IL_0037: ldc.i4.0 + IL_0038: ceq + IL_003a: stloc.2 + IL_003b: ldloc.2 + IL_003c: brtrue.s IL_0044 + + IL_003e: nop + IL_003f: ldarg.2 + IL_0040: ldloc.0 + IL_0041: stind.i4 + IL_0042: br.s IL_004f + + IL_0044: nop + IL_0045: ldloc.1 + IL_0046: callvirt instance bool [mscorlib]System.Collections.IEnumerator::MoveNext() + IL_004b: stloc.2 + IL_004c: ldloc.2 + IL_004d: brtrue.s IL_0019 + + IL_004f: leave.s IL_0068 + + } // end .try + finally + { + IL_0051: ldloc.1 + IL_0052: isinst [mscorlib]System.IDisposable + IL_0057: stloc.3 + IL_0058: ldloc.3 + IL_0059: ldnull + IL_005a: ceq + IL_005c: stloc.2 + IL_005d: ldloc.2 + IL_005e: brtrue.s IL_0067 + + IL_0060: ldloc.3 + IL_0061: callvirt instance void [mscorlib]System.IDisposable::Dispose() + IL_0066: nop + IL_0067: endfinally + } // end handler + IL_0068: nop + IL_0069: ret + } // end of method Loops::ForEachBreakWhenFound + .method public hidebysig instance void ForOverArray(string[] 'array') cil managed { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.opt.il b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.opt.il index a16562df5..7e47d77b2 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.opt.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.opt.il @@ -10,7 +10,7 @@ .publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4.. .ver 4:0:0:0 } -.assembly hrizu0iy +.assembly spdzj2hg { .custom instance void [mscorlib]System.Runtime.CompilerServices.CompilationRelaxationsAttribute::.ctor(int32) = ( 01 00 08 00 00 00 00 00 ) .custom instance void [mscorlib]System.Runtime.CompilerServices.RuntimeCompatibilityAttribute::.ctor() = ( 01 00 01 00 54 02 16 57 72 61 70 4E 6F 6E 45 78 // ....T..WrapNonEx @@ -20,15 +20,15 @@ .hash algorithm 0x00008004 .ver 0:0:0:0 } -.module hrizu0iy.dll -// MVID: {8FC80EBC-E149-454A-A769-13F3C07EB46F} +.module spdzj2hg.dll +// MVID: {D8CD3A33-FD06-4791-A4E6-AC836DA34D06} .custom instance void [mscorlib]System.Security.UnverifiableCodeAttribute::.ctor() = ( 01 00 00 00 ) .imagebase 0x10000000 .file alignment 0x00000200 .stackreserve 0x00100000 .subsystem 0x0003 // WINDOWS_CUI .corflags 0x00000001 // ILONLY -// Image base: 0x02CE0000 +// Image base: 0x002F0000 // =============== CLASS MEMBERS DECLARATION =================== @@ -1279,6 +1279,63 @@ IL_0034: ret } // end of method Loops::ForEachOverArrayOfPointers + .method public hidebysig instance void + ForEachBreakWhenFound(string name, + valuetype [mscorlib]System.StringComparison& output) cil managed + { + // Code size 87 (0x57) + .maxstack 2 + .locals init (valuetype [mscorlib]System.StringComparison V_0, + class [mscorlib]System.Collections.IEnumerator V_1, + class [mscorlib]System.IDisposable V_2) + IL_0000: ldtoken [mscorlib]System.StringComparison + IL_0005: call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle) + IL_000a: call class [mscorlib]System.Array [mscorlib]System.Enum::GetValues(class [mscorlib]System.Type) + IL_000f: callvirt instance class [mscorlib]System.Collections.IEnumerator [mscorlib]System.Array::GetEnumerator() + IL_0014: stloc.1 + .try + { + IL_0015: br.s IL_003b + + IL_0017: ldloc.1 + IL_0018: callvirt instance object [mscorlib]System.Collections.IEnumerator::get_Current() + IL_001d: unbox.any [mscorlib]System.StringComparison + IL_0022: stloc.0 + IL_0023: ldloc.0 + IL_0024: box [mscorlib]System.StringComparison + IL_0029: callvirt instance string [mscorlib]System.Object::ToString() + IL_002e: ldarg.1 + IL_002f: call bool [mscorlib]System.String::op_Equality(string, + string) + IL_0034: brfalse.s IL_003b + + IL_0036: ldarg.2 + IL_0037: ldloc.0 + IL_0038: stind.i4 + IL_0039: br.s IL_0043 + + IL_003b: ldloc.1 + IL_003c: callvirt instance bool [mscorlib]System.Collections.IEnumerator::MoveNext() + IL_0041: brtrue.s IL_0017 + + IL_0043: leave.s IL_0056 + + } // end .try + finally + { + IL_0045: ldloc.1 + IL_0046: isinst [mscorlib]System.IDisposable + IL_004b: stloc.2 + IL_004c: ldloc.2 + IL_004d: brfalse.s IL_0055 + + IL_004f: ldloc.2 + IL_0050: callvirt instance void [mscorlib]System.IDisposable::Dispose() + IL_0055: endfinally + } // end handler + IL_0056: ret + } // end of method Loops::ForEachBreakWhenFound + .method public hidebysig instance void ForOverArray(string[] 'array') cil managed { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.opt.roslyn.il b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.opt.roslyn.il index 4ab40094f..eb172c6d5 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.opt.roslyn.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.opt.roslyn.il @@ -25,14 +25,14 @@ .ver 0:0:0:0 } .module Loops.dll -// MVID: {30F4646B-D900-420C-89F2-283C178D2EAC} +// MVID: {B3F85A6A-79B4-41C0-9146-2088985BFC34} .custom instance void [mscorlib]System.Security.UnverifiableCodeAttribute::.ctor() = ( 01 00 00 00 ) .imagebase 0x10000000 .file alignment 0x00000200 .stackreserve 0x00100000 .subsystem 0x0003 // WINDOWS_CUI .corflags 0x00000001 // ILONLY -// Image base: 0x00E90000 +// Image base: 0x02550000 // =============== CLASS MEMBERS DECLARATION =================== @@ -1238,6 +1238,63 @@ IL_0032: ret } // end of method Loops::ForEachOverArrayOfPointers + .method public hidebysig instance void + ForEachBreakWhenFound(string name, + valuetype [mscorlib]System.StringComparison& output) cil managed + { + // Code size 89 (0x59) + .maxstack 2 + .locals init (class [mscorlib]System.Collections.IEnumerator V_0, + valuetype [mscorlib]System.StringComparison V_1, + class [mscorlib]System.IDisposable V_2) + IL_0000: ldtoken [mscorlib]System.StringComparison + IL_0005: call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle) + IL_000a: call class [mscorlib]System.Array [mscorlib]System.Enum::GetValues(class [mscorlib]System.Type) + IL_000f: callvirt instance class [mscorlib]System.Collections.IEnumerator [mscorlib]System.Array::GetEnumerator() + IL_0014: stloc.0 + .try + { + IL_0015: br.s IL_003d + + IL_0017: ldloc.0 + IL_0018: callvirt instance object [mscorlib]System.Collections.IEnumerator::get_Current() + IL_001d: unbox.any [mscorlib]System.StringComparison + IL_0022: stloc.1 + IL_0023: ldloca.s V_1 + IL_0025: constrained. [mscorlib]System.StringComparison + IL_002b: callvirt instance string [mscorlib]System.Object::ToString() + IL_0030: ldarg.1 + IL_0031: call bool [mscorlib]System.String::op_Equality(string, + string) + IL_0036: brfalse.s IL_003d + + IL_0038: ldarg.2 + IL_0039: ldloc.1 + IL_003a: stind.i4 + IL_003b: leave.s IL_0058 + + IL_003d: ldloc.0 + IL_003e: callvirt instance bool [mscorlib]System.Collections.IEnumerator::MoveNext() + IL_0043: brtrue.s IL_0017 + + IL_0045: leave.s IL_0058 + + } // end .try + finally + { + IL_0047: ldloc.0 + IL_0048: isinst [mscorlib]System.IDisposable + IL_004d: stloc.2 + IL_004e: ldloc.2 + IL_004f: brfalse.s IL_0057 + + IL_0051: ldloc.2 + IL_0052: callvirt instance void [mscorlib]System.IDisposable::Dispose() + IL_0057: endfinally + } // end handler + IL_0058: ret + } // end of method Loops::ForEachBreakWhenFound + .method public hidebysig instance void ForOverArray(string[] 'array') cil managed { diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.roslyn.il b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.roslyn.il index 3a9841da1..570e97dad 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.roslyn.il +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Loops.roslyn.il @@ -25,14 +25,14 @@ .ver 0:0:0:0 } .module Loops.dll -// MVID: {721D369B-DC6E-425B-AE0E-46C242DEFDF5} +// MVID: {8261F08F-23F6-43C1-9650-C5B6A10A0F30} .custom instance void [mscorlib]System.Security.UnverifiableCodeAttribute::.ctor() = ( 01 00 00 00 ) .imagebase 0x10000000 .file alignment 0x00000200 .stackreserve 0x00100000 .subsystem 0x0003 // WINDOWS_CUI .corflags 0x00000001 // ILONLY -// Image base: 0x00480000 +// Image base: 0x00720000 // =============== CLASS MEMBERS DECLARATION =================== @@ -1488,6 +1488,72 @@ IL_003a: ret } // end of method Loops::ForEachOverArrayOfPointers + .method public hidebysig instance void + ForEachBreakWhenFound(string name, + valuetype [mscorlib]System.StringComparison& output) cil managed + { + // Code size 97 (0x61) + .maxstack 2 + .locals init (class [mscorlib]System.Collections.IEnumerator V_0, + valuetype [mscorlib]System.StringComparison V_1, + bool V_2, + class [mscorlib]System.IDisposable V_3) + IL_0000: nop + IL_0001: nop + IL_0002: ldtoken [mscorlib]System.StringComparison + IL_0007: call class [mscorlib]System.Type [mscorlib]System.Type::GetTypeFromHandle(valuetype [mscorlib]System.RuntimeTypeHandle) + IL_000c: call class [mscorlib]System.Array [mscorlib]System.Enum::GetValues(class [mscorlib]System.Type) + IL_0011: callvirt instance class [mscorlib]System.Collections.IEnumerator [mscorlib]System.Array::GetEnumerator() + IL_0016: stloc.0 + .try + { + IL_0017: br.s IL_0044 + + IL_0019: ldloc.0 + IL_001a: callvirt instance object [mscorlib]System.Collections.IEnumerator::get_Current() + IL_001f: unbox.any [mscorlib]System.StringComparison + IL_0024: stloc.1 + IL_0025: nop + IL_0026: ldloca.s V_1 + IL_0028: constrained. [mscorlib]System.StringComparison + IL_002e: callvirt instance string [mscorlib]System.Object::ToString() + IL_0033: ldarg.1 + IL_0034: call bool [mscorlib]System.String::op_Equality(string, + string) + IL_0039: stloc.2 + IL_003a: ldloc.2 + IL_003b: brfalse.s IL_0043 + + IL_003d: nop + IL_003e: ldarg.2 + IL_003f: ldloc.1 + IL_0040: stind.i4 + IL_0041: br.s IL_004c + + IL_0043: nop + IL_0044: ldloc.0 + IL_0045: callvirt instance bool [mscorlib]System.Collections.IEnumerator::MoveNext() + IL_004a: brtrue.s IL_0019 + + IL_004c: leave.s IL_0060 + + } // end .try + finally + { + IL_004e: ldloc.0 + IL_004f: isinst [mscorlib]System.IDisposable + IL_0054: stloc.3 + IL_0055: ldloc.3 + IL_0056: brfalse.s IL_005f + + IL_0058: ldloc.3 + IL_0059: callvirt instance void [mscorlib]System.IDisposable::Dispose() + IL_005e: nop + IL_005f: endfinally + } // end handler + IL_0060: ret + } // end of method Loops::ForEachBreakWhenFound + .method public hidebysig instance void ForOverArray(string[] 'array') cil managed { diff --git a/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs b/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs index ce3d0f43d..ee7294583 100644 --- a/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs +++ b/ICSharpCode.Decompiler/IL/ControlFlow/LoopDetection.cs @@ -215,6 +215,7 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow if (exitPoint != null) { // Either we are in case 1 and just picked an exit that maximizes the amount of code // outside the loop, or we are in case 2 and found an exit point via post-dominance. + // Note that if exitPoint == NoExitPoint, we end up adding all dominated blocks to the loop. var ep = exitPoint; foreach (var node in TreeTraversal.PreOrder(loopHead, n => (n != ep) ? n.DominatorTreeChildren : null)) { if (node != exitPoint && !node.Visited) { @@ -228,10 +229,21 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow ExtendLoopHeuristic(loopHead, loop, loopHead); } } - + + /// + /// Special control flow node (not part of any graph) that signifies that we want to construct a loop + /// without any exit point. + /// + static readonly ControlFlowNode NoExitPoint = new ControlFlowNode(); + /// /// Finds a suitable single exit point for the specified loop. /// + /// + /// 1) If a suitable exit point was found: the control flow block that should be reached when breaking from the loop + /// 2) If the loop should not have any exit point (extend by all dominated blocks): NoExitPoint + /// 3) otherwise (exit point unknown, heuristically extend loop): null + /// /// This method must not write to the Visited flags on the CFG. ControlFlowNode FindExitPoint(ControlFlowNode loopHead, IReadOnlyList naturalLoop, bool treatBackEdgesAsExits) { @@ -245,6 +257,16 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow // Case 1: // There are no nodes n so that loopHead dominates a predecessor of n but not n itself // -> we could build a loop with zero exit points. + if (IsPossibleForeachLoop((Block)loopHead.UserData, out var exitBranch)) { + if (exitBranch != null) { + // let's see if the target of the exit branch is a suitable exit point + var cfgNode = loopHead.Successors.FirstOrDefault(n => n.UserData == exitBranch.TargetBlock); + if (cfgNode != null && loopHead.Dominates(cfgNode) && !context.ControlFlowGraph.HasReachableExit(cfgNode)) { + return cfgNode; + } + } + return NoExitPoint; + } ControlFlowNode exitPoint = null; int exitPointILOffset = -1; foreach (var node in loopHead.DominatorTreeChildren) { @@ -388,8 +410,38 @@ namespace ICSharpCode.Decompiler.IL.ControlFlow Dominance.ComputeDominance(exitNode, context.CancellationToken); return rev; } + + static bool IsPossibleForeachLoop(Block loopHead, out Branch exitBranch) + { + exitBranch = null; + var container = (BlockContainer)loopHead.Parent; + if (!(container.SlotInfo == TryInstruction.TryBlockSlot && container.Parent is TryFinally)) + return false; + if (loopHead.Instructions.Count != 2) + return false; + if (!loopHead.Instructions[0].MatchIfInstruction(out var condition, out var trueInst)) + return false; + var falseInst = loopHead.Instructions[1]; + while (condition.MatchLogicNot(out var arg)) { + condition = arg; + ExtensionMethods.Swap(ref trueInst, ref falseInst); + } + if (!(condition is CallInstruction call && call.Method.Name == "MoveNext")) + return false; + if (!(call.Arguments.Count == 1 && call.Arguments[0].MatchLdLocRef(out var enumeratorVar))) + return false; + exitBranch = falseInst as Branch; + + // Check that loopHead is entry-point of try-block: + Block entryPoint = container.EntryPoint; + while (entryPoint.IncomingEdgeCount == 1 && entryPoint.Instructions.Count == 1 && entryPoint.Instructions[0].MatchBranch(out var targetBlock)) { + // skip blocks that only branch to another block + entryPoint = targetBlock; + } + return entryPoint == loopHead; + } #endregion - + #region ExtendLoop (fall-back heuristic) /// /// This function implements a heuristic algorithm that tries to reduce the number of exit diff --git a/ICSharpCode.Decompiler/IL/Instructions.cs b/ICSharpCode.Decompiler/IL/Instructions.cs index 6748f7b84..8d3db22de 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.cs +++ b/ICSharpCode.Decompiler/IL/Instructions.cs @@ -27,7 +27,7 @@ namespace ICSharpCode.Decompiler.IL /// /// Enum representing the type of an . /// - public enum OpCode + public enum OpCode : byte { /// Represents invalid IL. Semantically, this instruction is considered to throw some kind of exception. InvalidBranch, diff --git a/ICSharpCode.Decompiler/IL/Instructions.tt b/ICSharpCode.Decompiler/IL/Instructions.tt index 9d01fcf8c..d29ea9d68 100644 --- a/ICSharpCode.Decompiler/IL/Instructions.tt +++ b/ICSharpCode.Decompiler/IL/Instructions.tt @@ -271,7 +271,7 @@ namespace ICSharpCode.Decompiler.IL /// /// Enum representing the type of an . /// - public enum OpCode + public enum OpCode : byte { <# foreach (OpCode opCode in opCodes) { #> /// <#=opCode.Description.Replace("\n", "\n\t\t/// ")#> diff --git a/ICSharpCode.Decompiler/IL/Instructions/Block.cs b/ICSharpCode.Decompiler/IL/Instructions/Block.cs index 480851e59..6c6fc813f 100644 --- a/ICSharpCode.Decompiler/IL/Instructions/Block.cs +++ b/ICSharpCode.Decompiler/IL/Instructions/Block.cs @@ -37,10 +37,6 @@ namespace ICSharpCode.Decompiler.IL /// 1) Blocks in block containers. Used as targets for Branch instructions. /// 2) Blocks to group a bunch of statements, e.g. the TrueInst of an IfInstruction. /// 3) Inline blocks that evaluate to a value, e.g. for array initializers. - /// - /// TODO: consider splitting inline blocks (with FinalInstruction) from those - /// used in containers for control flow purposes -- these are very different things - /// which should not share a class. /// partial class Block : ILInstruction { diff --git a/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs b/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs index 20c63b6c8..7498f857a 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs @@ -126,7 +126,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms ILInstruction condition = ifInst.Condition; while (condition.MatchLogicNot(out var arg)) { condition = arg; - Swap(ref trueInst, ref falseInst); + ExtensionMethods.Swap(ref trueInst, ref falseInst); } if (AnalyzeCondition(condition)) { // (v1 != null && ... && vn != null) ? trueInst : falseInst @@ -140,7 +140,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms // for equality/inequality, the HasValue bits must also compare equal/inequal if (comp.Kind == ComparisonKind.Inequality) { // handle inequality by swapping one last time - Swap(ref trueInst, ref falseInst); + ExtensionMethods.Swap(ref trueInst, ref falseInst); } if (falseInst.MatchLdcI4(0)) { // (a.GetValueOrDefault() == b.GetValueOrDefault()) ? (a.HasValue == b.HasValue) : false @@ -282,13 +282,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms return false; return MatchHasValueCall(arg, nullable1); } - - static void Swap(ref T a, ref T b) - { - T tmp = a; - a = b; - b = tmp; - } #endregion #region CSharpComp diff --git a/ICSharpCode.Decompiler/Util/ExtensionMethods.cs b/ICSharpCode.Decompiler/Util/ExtensionMethods.cs index a5e754140..57c97199a 100644 --- a/ICSharpCode.Decompiler/Util/ExtensionMethods.cs +++ b/ICSharpCode.Decompiler/Util/ExtensionMethods.cs @@ -22,7 +22,7 @@ using System.Collections.Generic; namespace ICSharpCode.Decompiler.Util { /// - /// Contains extension methods for use within NRefactory. + /// Contains extension methods for internal use within the decompiler. /// static class ExtensionMethods { @@ -34,5 +34,12 @@ namespace ICSharpCode.Decompiler.Util return filter1; return m => filter1(m) && filter2(m); } + + public static void Swap(ref T a, ref T b) + { + T tmp = a; + a = b; + b = tmp; + } } }