Skip to content

Commit dba836c

Browse files
siegfriedpammersiegfriedpammer
authored
Merge pull request #3440 from icsharpcode/bugfix/constrained-call-targets
Fix changes due to eval order fix
2 parents 05d30f1 + 6c72d1c commit dba836c

27 files changed

+677
-47
lines changed

‎.github/workflows/build-ilspy.yml‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ jobs:
6060
run: msbuild ILSpy.sln /p:Configuration=${{ matrix.configuration }} /p:Platform=$env:BuildPlatform /m
6161

6262
- name: Execute unit tests
63-
run: dotnet test --logger "junit;LogFileName=${{ matrix.configuration }}.xml" --results-directory test-results $env:Tests1 $env:Tests2 $env:Tests3
63+
run: dotnet test --logger "trx;LogFileName=${{ matrix.configuration }}.trx" --results-directory test-results $env:Tests1 $env:Tests2 $env:Tests3
6464
env:
6565
Tests1: ICSharpCode.Decompiler.Tests\bin\${{ matrix.configuration }}\net8.0-windows\win-x64\ICSharpCode.Decompiler.Tests.dll
6666
Tests2: ILSpy.Tests\bin\${{ matrix.configuration }}\net8.0-windows\ILSpy.Tests.dll
@@ -71,13 +71,14 @@ jobs:
7171
if: success() || failure()
7272
with:
7373
name: test-results-${{ matrix.configuration }}
74-
path: 'test-results/${{ matrix.configuration }}.xml'
74+
path: 'test-results/${{ matrix.configuration }}.trx'
7575

7676
- name: Create Test Report
77-
uses: test-summary/action@v2
77+
uses: icsharpcode/test-summary-action@dist
7878
if: always()
7979
with:
80-
paths: "test-results/${{ matrix.configuration }}.xml"
80+
paths: "test-results/${{ matrix.configuration }}.trx"
81+
folded: true
8182

8283
- name: Format check
8384
run: dotnet-format whitespace --verify-no-changes --verbosity detailed ILSpy.sln

‎ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
1717
// DEALINGS IN THE SOFTWARE.
1818

19-
using System;
20-
using System.CodeDom.Compiler;
2119
using System.IO;
2220
using System.Linq;
2321
using System.Runtime.CompilerServices;
@@ -313,6 +311,12 @@ public async Task Jmp()
313311
await RunIL("Jmp.il");
314312
}
315313

314+
[Test]
315+
public async Task NonGenericConstrainedCallVirt()
316+
{
317+
await RunIL("NonGenericConstrainedCallVirt.il", CompilerOptions.UseRoslynLatest);
318+
}
319+
316320
[Test]
317321
public async Task StackTests()
318322
{

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
<AllowUnsafeBlocks>True</AllowUnsafeBlocks>
1818

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

2222
<GenerateAssemblyVersionAttribute>False</GenerateAssemblyVersionAttribute>
2323
<GenerateAssemblyFileVersionAttribute>False</GenerateAssemblyFileVersionAttribute>
@@ -95,6 +95,7 @@
9595
<None Include="TestCases\ILPretty\Issue2260SwitchString.il" />
9696
<None Include="TestCases\ILPretty\Issue3442.il" />
9797
<None Include="TestCases\ILPretty\MonoFixed.il" />
98+
<None Include="TestCases\Correctness\NonGenericConstrainedCallVirt.il" />
9899
<None Include="TestCases\ILPretty\UnknownTypes.cs" />
99100
<None Include="TestCases\ILPretty\UnknownTypes.il" />
100101
<None Include="TestCases\ILPretty\EvalOrder.cs" />

‎ICSharpCode.Decompiler.Tests/TestCases/Correctness/NonGenericConstrainedCallVirt.il‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
.assembly extern mscorlib
2+
{
3+
.publickeytoken = (B7 7A 5C 56 19 34 E0 89 ) // .z\V.4..
4+
.ver 4:0:0:0
5+
}
6+
7+
.assembly _
8+
{
9+
.hash algorithm 0x00008004 // SHA1
10+
.ver 0:0:0:0
11+
}
12+
13+
.class public auto ansi beforefieldinit NonGenericConstrainedCallVirt
14+
extends [mscorlib]System.Object
15+
{
16+
// Methods
17+
.method public hidebysig static
18+
void Main () cil managed
19+
{
20+
// Method begins at RVA 0x2050
21+
// Code size 16 (0x10)
22+
.entrypoint
23+
.maxstack 8
24+
25+
IL_0000: ldstr "A"
26+
IL_0005: newobj instance void C::.ctor(string)
27+
IL_000a: call void NonGenericConstrainedCallVirt::Foo(class C)
28+
IL_000f: ret
29+
} // end of method NonGenericConstrainedCallVirt::Main
30+
31+
.method private hidebysig static
32+
void Foo (
33+
class C arg
34+
) cil managed
35+
{
36+
// Method begins at RVA 0x2064
37+
// Code size 25 (0x19)
38+
.maxstack 8
39+
40+
IL_0000: ldarga arg
41+
IL_0004: ldarga arg
42+
IL_0008: call int32 NonGenericConstrainedCallVirt::Bar(class C&)
43+
IL_000d: constrained. C
44+
IL_0013: callvirt instance void C::Baz(int32)
45+
IL_0018: ret
46+
} // end of method NonGenericConstrainedCallVirt::Foo
47+
48+
.method private hidebysig static
49+
int32 Bar (
50+
class C& o
51+
) cil managed
52+
{
53+
// Method begins at RVA 0x2080
54+
// Code size 14 (0xe)
55+
.maxstack 8
56+
57+
IL_0000: ldarg.0
58+
IL_0001: ldstr "B"
59+
IL_0006: newobj instance void C::.ctor(string)
60+
IL_000b: stind.ref
61+
IL_000c: ldc.i4.0
62+
IL_000d: ret
63+
} // end of method NonGenericConstrainedCallVirt::Bar
64+
65+
.method public hidebysig specialname rtspecialname
66+
instance void .ctor () cil managed
67+
{
68+
// Method begins at RVA 0x2090
69+
// Code size 7 (0x7)
70+
.maxstack 8
71+
72+
IL_0000: ldarg.0
73+
IL_0001: call instance void [mscorlib]System.Object::.ctor()
74+
IL_0006: ret
75+
} // end of method NonGenericConstrainedCallVirt::.ctor
76+
77+
} // end of class NonGenericConstrainedCallVirt
78+
79+
.class public auto ansi beforefieldinit C
80+
extends [mscorlib]System.Object
81+
{
82+
// Fields
83+
.field private initonly string '<Name>k__BackingField'
84+
.custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = (
85+
01 00 00 00
86+
)
87+
88+
// Methods
89+
.method public hidebysig specialname rtspecialname
90+
instance void .ctor (
91+
string n
92+
) cil managed
93+
{
94+
// Method begins at RVA 0x2098
95+
// Code size 14 (0xe)
96+
.maxstack 8
97+
98+
IL_0000: ldarg.0
99+
IL_0001: call instance void [mscorlib]System.Object::.ctor()
100+
IL_0006: ldarg.0
101+
IL_0007: ldarg.1
102+
IL_0008: stfld string C::'<Name>k__BackingField'
103+
IL_000d: ret
104+
} // end of method C::.ctor
105+
106+
.method public hidebysig
107+
instance void Baz (
108+
int32 arg
109+
) cil managed
110+
{
111+
// Method begins at RVA 0x20a8
112+
// Code size 12 (0xc)
113+
.maxstack 8
114+
115+
IL_0000: ldarg.0
116+
IL_0001: call instance string C::get_Name()
117+
IL_0006: call void [mscorlib]System.Console::WriteLine(string)
118+
IL_000b: ret
119+
} // end of method C::Baz
120+
121+
.method public hidebysig specialname
122+
instance string get_Name () cil managed
123+
{
124+
.custom instance void [mscorlib]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = (
125+
01 00 00 00
126+
)
127+
// Method begins at RVA 0x20b8
128+
// Code size 7 (0x7)
129+
.maxstack 8
130+
131+
IL_0000: ldarg.0
132+
IL_0001: ldfld string C::'<Name>k__BackingField'
133+
IL_0006: ret
134+
} // end of method C::get_Name
135+
136+
// Properties
137+
.property instance string Name()
138+
{
139+
.get instance string C::get_Name()
140+
}
141+
142+
} // end of class C
143+

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ public interface IInterface
8484
{
8585
void Method1<T>() where T : class;
8686
void Method2<T>() where T : class;
87+
88+
void Method3(int a, string b, Type c);
89+
#if CS72
90+
void Method4(in int a);
91+
#endif
8792
}
8893

8994
public abstract class Base : IInterface
@@ -95,6 +100,16 @@ public abstract class Base : IInterface
95100
void IInterface.Method2<T>()
96101
{
97102
}
103+
104+
void IInterface.Method3(int a, string b, Type c)
105+
{
106+
}
107+
108+
#if CS72
109+
void IInterface.Method4(in int a)
110+
{
111+
}
112+
#endif
98113
}
99114

100115
public class Derived : Base
@@ -296,5 +311,44 @@ public static string Issue2231b<T>()
296311
{
297312
return default(T).ToString();
298313
}
314+
315+
public static void ConstrainedCall<T>(T x, ref T y) where T : IDisposable
316+
{
317+
x.Dispose();
318+
y.Dispose();
319+
}
320+
321+
// prior to C# 7.0 UseRefLocalsForAccurateOrderOfEvaluation is disabled, so we will inline.
322+
// Roslyn 4 generates the explicit ldobj.if.ref pattern, so we can also inline.
323+
// The versions in between, we don't inline, so the code doesn't look pretty.
324+
#if ROSLYN4 || !CS70
325+
public static int[] Issue3438<T>(T[] array)
326+
{
327+
List<int> list = new List<int>();
328+
for (int i = 0; i < array.Length; i++)
329+
{
330+
if (!array[i].Equals(default(T)))
331+
{
332+
list.Add(i);
333+
}
334+
}
335+
return list.ToArray();
336+
}
337+
public void Issue3438b<T>(T[] item1, T item2, int item3) where T : IInterface
338+
{
339+
item1[CastToInt(item2)].Method3(CastToInt(item2), CastToString(item2), TestTypeOf()[1]);
340+
}
341+
public void Issue3438c<T>(T item, T item2, int item3) where T : IInterface
342+
{
343+
CastFromInt<T>(item3).Method3(CastToInt(item2), CastToString(item2), TestTypeOf()[1]);
344+
}
345+
//#if CS72
346+
// Disabled because ILInlining does not support inlining ldloca currently.
347+
// public void Issue3438d<T>(T[] item1, T item2, int item3) where T : IInterface
348+
// {
349+
// item1[CastToInt(item2)].Method4(CastToInt(item2));
350+
// }
351+
//#endif
352+
#endif
299353
}
300354
}

‎ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ public static List<IILTransform> GetILTransforms()
149149
new IndexRangeTransform(),
150150
new DeconstructionTransform(),
151151
new NamedArgumentTransform(),
152+
new RemoveUnconstrainedGenericReferenceTypeCheck(),
152153
new UserDefinedLogicTransform(),
153154
new InterpolatedStringTransform()
154155
),
@@ -1717,6 +1718,7 @@ void DecompileBody(IMethod method, EntityDeclaration entityDecl, DecompileRun de
17171718
{
17181719
var ilReader = new ILReader(typeSystem.MainModule) {
17191720
UseDebugSymbols = settings.UseDebugSymbols,
1721+
UseRefLocalsForAccurateOrderOfEvaluation = settings.UseRefLocalsForAccurateOrderOfEvaluation,
17201722
DebugInfo = DebugInfoProvider
17211723
};
17221724
var methodDef = metadata.GetMethodDefinition((MethodDefinitionHandle)method.MetadataToken);

‎ICSharpCode.Decompiler/CSharp/CallBuilder.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,11 +356,18 @@ public ExpressionWithResolveResult Build(OpCode callOpCode, IMethod method,
356356
}
357357
else
358358
{
359+
var thisArg = callArguments.FirstOrDefault();
360+
if (thisArg is LdObjIfRef ldObjIfRef)
361+
{
362+
Debug.Assert(constrainedTo != null);
363+
thisArg = ldObjIfRef.Target;
364+
}
359365
target = expressionBuilder.TranslateTarget(
360-
callArguments.FirstOrDefault(),
366+
thisArg,
361367
nonVirtualInvocation: callOpCode == OpCode.Call || method.IsConstructor,
362368
memberStatic: method.IsStatic,
363-
memberDeclaringType: constrainedTo ?? method.DeclaringType);
369+
memberDeclaringType: method.DeclaringType,
370+
constrainedTo: constrainedTo);
364371
if (constrainedTo == null
365372
&& target.Expression is CastExpression cast
366373
&& target.ResolveResult is ConversionResolveResult conversion

‎ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2614,7 +2614,7 @@ protected internal override TranslatedExpression VisitBlockContainer(BlockContai
26142614
}
26152615

26162616
internal TranslatedExpression TranslateTarget(ILInstruction target, bool nonVirtualInvocation,
2617-
bool memberStatic, IType memberDeclaringType)
2617+
bool memberStatic, IType memberDeclaringType, IType constrainedTo = null)
26182618
{
26192619
// If references are missing member.IsStatic might not be set correctly.
26202620
// Additionally check target for null, in order to avoid a crash.
@@ -2630,8 +2630,8 @@ internal TranslatedExpression TranslateTarget(ILInstruction target, bool nonVirt
26302630
}
26312631
else
26322632
{
2633-
IType targetTypeHint = memberDeclaringType;
2634-
if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType) == StackType.Ref)
2633+
IType targetTypeHint = constrainedTo ?? memberDeclaringType;
2634+
if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType, constrainedTo) == StackType.Ref)
26352635
{
26362636
if (target.ResultType == StackType.Ref)
26372637
{
@@ -2643,13 +2643,13 @@ internal TranslatedExpression TranslateTarget(ILInstruction target, bool nonVirt
26432643
}
26442644
}
26452645
var translatedTarget = Translate(target, targetTypeHint);
2646-
if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType) == StackType.Ref)
2646+
if (CallInstruction.ExpectedTypeForThisPointer(memberDeclaringType, constrainedTo) == StackType.Ref)
26472647
{
26482648
// When accessing members on value types, ensure we use a reference of the correct type,
26492649
// and not a pointer or a reference to a different type (issue #1333)
2650-
if (!(translatedTarget.Type is ByReferenceType brt && NormalizeTypeVisitor.TypeErasure.EquivalentTypes(brt.ElementType, memberDeclaringType)))
2650+
if (!(translatedTarget.Type is ByReferenceType brt && NormalizeTypeVisitor.TypeErasure.EquivalentTypes(brt.ElementType, constrainedTo ?? memberDeclaringType)))
26512651
{
2652-
translatedTarget = translatedTarget.ConvertTo(new ByReferenceType(memberDeclaringType), this);
2652+
translatedTarget = translatedTarget.ConvertTo(new ByReferenceType(constrainedTo ?? memberDeclaringType), this);
26532653
}
26542654
}
26552655
if (translatedTarget.Expression is DirectionExpression)
@@ -2675,9 +2675,9 @@ internal TranslatedExpression TranslateTarget(ILInstruction target, bool nonVirt
26752675
}
26762676
else
26772677
{
2678-
return new TypeReferenceExpression(ConvertType(memberDeclaringType))
2678+
return new TypeReferenceExpression(ConvertType(constrainedTo ?? memberDeclaringType))
26792679
.WithoutILInstruction()
2680-
.WithRR(new TypeResolveResult(memberDeclaringType));
2680+
.WithRR(new TypeResolveResult(constrainedTo ?? memberDeclaringType));
26812681
}
26822682

26832683
bool ShouldUseBaseReference()
@@ -2686,7 +2686,7 @@ bool ShouldUseBaseReference()
26862686
return false;
26872687
if (!MatchLdThis(target))
26882688
return false;
2689-
if (memberDeclaringType.GetDefinition() == resolver.CurrentTypeDefinition)
2689+
if ((constrainedTo ?? memberDeclaringType).GetDefinition() == resolver.CurrentTypeDefinition)
26902690
return false;
26912691
return true;
26922692
}

0 commit comments

Comments
 (0)