Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Commit 83fd293

Browse files
nattressnattress
committed
Code Review Feedback
1 parent 7ca90e2 commit 83fd293

20 files changed

Lines changed: 106 additions & 168 deletions

File tree

‎src/Common/src/TypeSystem/Common/TypeSystemContext.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -492,6 +492,9 @@ protected override MethodForInstantiatedType CreateValueFromKey(MethodForInstant
492492

493493
public MethodDesc GetMethodForInstantiatedType(MethodDesc typicalMethodDef, InstantiatedType instantiatedType)
494494
{
495+
Debug.Assert(!(typicalMethodDef is MethodForInstantiatedType));
496+
Debug.Assert(!(typicalMethodDef is InstantiatedMethod));
497+
495498
return _methodForInstantiatedTypes.GetOrCreateValue(new MethodForInstantiatedTypeKey(typicalMethodDef, instantiatedType));
496499
}
497500

‎src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/EETypeNode.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ public override ObjectData GetData(NodeFactory factory, bool relocsOnly)
142142
objData.EmitZeroPointer();
143143
return objData.ToObjectData();
144144
}
145-
145+
146146
ComputeOptionalEETypeFields(factory);
147147

148148
if (null == _optionalFieldsNode)

‎src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/InterfaceDispatchCellNode.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
using System;
2-
using System.Diagnostics;
3-
using System.Collections.Generic;
4-
using System.Text;
1+
// Copyright (c) Microsoft. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
54
using Internal.TypeSystem;
5+
using System;
6+
using System.Diagnostics;
67

78
namespace ILCompiler.DependencyAnalysis
89
{

‎src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/InterfaceDispatchMapNode.cs‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
11
// Copyright (c) Microsoft. All rights reserved.
22
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
33

4-
using System;
4+
using Internal.TypeSystem;
55
using System.Collections.Generic;
66
using System.Diagnostics;
7-
using System.Linq;
8-
using System.Text;
9-
using System.Threading.Tasks;
10-
using Internal.TypeSystem;
11-
using ILCompiler.DependencyAnalysisFramework;
127

138
namespace ILCompiler.DependencyAnalysis
149
{
@@ -72,6 +67,7 @@ public override string Section
7267

7368
internal void SetDispatchMapTableIndex(uint index)
7469
{
70+
Debug.Assert(_dispatchMapTableIndex == uint.MaxValue);
7571
_dispatchMapTableIndex = index;
7672
}
7773

@@ -83,8 +79,8 @@ protected override void OnMarked(NodeFactory context)
8379

8480
DispatchMapEntry[] BuildDispatchMap(NodeFactory factory)
8581
{
86-
List<DispatchMapEntry> dispatchMapEntries = new List<DispatchMapEntry>();
87-
82+
ArrayBuilder<DispatchMapEntry> dispatchMapEntries = new ArrayBuilder<DispatchMapEntry>();
83+
8884
for (int i = 0; i < _type.RuntimeInterfaces.Length; i++)
8985
{
9086
var interfaceType = _type.RuntimeInterfaces[i];

‎src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/InterfaceDispatchMapTableNode.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
using System;
2-
using System.Diagnostics;
1+
// Copyright (c) Microsoft. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
34
using System.Collections.Generic;
4-
using System.Text;
5-
using Internal.TypeSystem;
65

76
namespace ILCompiler.DependencyAnalysis
87
{

‎src/ILCompiler.Compiler/src/Compiler/DependencyAnalysis/ReadyToRunHelperNode.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public override string MangledName
8383
case ReadyToRunHelperId.DelegateCtor:
8484
return "__DelegateCtor_" + NodeFactory.NameMangler.GetMangledMethodName(((DelegateInfo)_target).Target);
8585
case ReadyToRunHelperId.InterfaceDispatch:
86-
return "__InterfaceDispatchHelper_" + NodeFactory.NameMangler.GetMangledMethodName((MethodDesc)_target);
86+
return "__InterfaceDispatch_" + NodeFactory.NameMangler.GetMangledMethodName((MethodDesc)_target);
8787
default:
8888
throw new NotImplementedException();
8989
}

‎src/ILCompiler.TypeSystem/src/ILCompiler.TypeSystem.csproj‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@
6464
<Compile Include="..\..\Common\src\TypeSystem\Common\MethodDesc.cs" />
6565
<Compile Include="..\..\Common\src\TypeSystem\Common\VirtualFunctionResolution.cs" />
6666
<Compile Include="..\..\Common\src\TypeSystem\Common\TypeDesc.cs" />
67-
<Compile Include="..\..\Common\src\TypeSystem\Common\TypeDesc.MethodImpls.cs" />
6867
<Compile Include="..\..\Common\src\TypeSystem\Common\TypeDesc.Interfaces.cs" />
6968
<Compile Include="..\..\Common\src\TypeSystem\Common\DefType.cs" />
7069
<Compile Include="..\..\Common\src\TypeSystem\Common\DefType.FieldLayout.cs" />
@@ -114,6 +113,8 @@
114113
<Compile Include="..\..\Common\src\TypeSystem\Common\LocalVariableDefinition.cs">
115114
<Link>LocalVariableDefinition.cs</Link>
116115
</Compile>
116+
<Compile Include="InstantiatedType.MethodImpls.cs" />
117+
<Compile Include="MetadataType.MethodImpls.cs" />
117118
</ItemGroup>
118119
<ItemGroup>
119120
<None Include="project.json" />

src/Common/src/TypeSystem/Common/TypeDesc.MethodImpls.cs renamed to src/ILCompiler.TypeSystem/src/InstantiatedType.MethodImpls.cs

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,47 +5,6 @@
55

66
namespace Internal.TypeSystem
77
{
8-
public struct MethodImplRecord
9-
{
10-
public MethodDesc Decl;
11-
public MethodDesc Body;
12-
}
13-
14-
// MethodImpl api surface for types.
15-
public partial class MetadataType
16-
{
17-
/// <summary>
18-
/// Compute an array of all MethodImpls that pertain to overriding virtual (non-interface methods) on this type.
19-
/// May be expensive.
20-
/// </summary>
21-
protected abstract MethodImplRecord[] ComputeVirtualMethodImplsForType();
22-
23-
private MethodImplRecord[] _allVirtualMethodImplsForType;
24-
/// <summary>
25-
/// Get an array of all MethodImpls that pertain to overriding virtual (non-interface methods) on this type.
26-
/// Expected to cache results so this api can be used repeatedly.
27-
/// </summary>
28-
public MethodImplRecord[] VirtualMethodImplsForType
29-
{
30-
get
31-
{
32-
if (_allVirtualMethodImplsForType == null)
33-
{
34-
_allVirtualMethodImplsForType = ComputeVirtualMethodImplsForType();
35-
}
36-
37-
return _allVirtualMethodImplsForType;
38-
}
39-
}
40-
41-
/// <summary>
42-
/// Get an array of MethodImpls where the Decl method matches by name with the specified name.
43-
/// </summary>
44-
/// <param name="name"></param>
45-
/// <returns></returns>
46-
public abstract MethodImplRecord[] FindMethodsImplWithMatchingDeclName(string name);
47-
}
48-
498
// Implementation of MethodImpl api surface implemented without metadata access.
509
public partial class InstantiatedType
5110
{
@@ -63,7 +22,16 @@ private MethodImplRecord[] InstantiateMethodImpls(MethodImplRecord[] uninstMetho
6322

6423
for (int i = 0; i < uninstMethodImpls.Length; i++)
6524
{
66-
instMethodImpls[i].Decl = _typeDef.Context.GetMethodForInstantiatedType(uninstMethodImpls[i].Decl, this);
25+
var implTypeInstantiated = uninstMethodImpls[i].Decl.OwningType.InstantiateSignature(this.Instantiation, new Instantiation());
26+
if (implTypeInstantiated is InstantiatedType)
27+
{
28+
instMethodImpls[i].Decl = _typeDef.Context.GetMethodForInstantiatedType(uninstMethodImpls[i].Decl.GetTypicalMethodDefinition(), (InstantiatedType)implTypeInstantiated);
29+
}
30+
else
31+
{
32+
instMethodImpls[i].Decl = uninstMethodImpls[i].Decl;
33+
}
34+
6735
instMethodImpls[i].Body = _typeDef.Context.GetMethodForInstantiatedType(uninstMethodImpls[i].Body, this);
6836
}
6937

‎src/ILCompiler.TypeSystem/src/MetadataType.MethodImpls.cs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright (c) Microsoft. All rights reserved.
2+
// Licensed under the MIT license. See LICENSE file in the project root for full license information.
3+
4+
using System;
5+
6+
namespace Internal.TypeSystem
7+
{
8+
public struct MethodImplRecord
9+
{
10+
public MethodDesc Decl;
11+
public MethodDesc Body;
12+
}
13+
14+
// MethodImpl api surface for types.
15+
public partial class MetadataType
16+
{
17+
/// <summary>
18+
/// Compute an array of all MethodImpls that pertain to overriding virtual (non-interface methods) on this type.
19+
/// May be expensive.
20+
/// </summary>
21+
protected abstract MethodImplRecord[] ComputeVirtualMethodImplsForType();
22+
23+
private MethodImplRecord[] _allVirtualMethodImplsForType;
24+
/// <summary>
25+
/// Get an array of all MethodImpls that pertain to overriding virtual (non-interface methods) on this type.
26+
/// Expected to cache results so this api can be used repeatedly.
27+
/// </summary>
28+
public MethodImplRecord[] VirtualMethodImplsForType
29+
{
30+
get
31+
{
32+
if (_allVirtualMethodImplsForType == null)
33+
{
34+
_allVirtualMethodImplsForType = ComputeVirtualMethodImplsForType();
35+
}
36+
37+
return _allVirtualMethodImplsForType;
38+
}
39+
}
40+
41+
/// <summary>
42+
/// Get an array of MethodImpls where the Decl method matches by name with the specified name.
43+
/// </summary>
44+
/// <param name="name"></param>
45+
/// <returns></returns>
46+
public abstract MethodImplRecord[] FindMethodsImplWithMatchingDeclName(string name);
47+
}
48+
}

‎src/ILCompiler/repro/Program.cs‎

Lines changed: 1 addition & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -7,76 +7,6 @@ internal class Program
77
{
88
private static void Main(string[] args)
99
{
10-
MyInterface i1 = new Foo(false, 1, "Foo implementation") as MyInterface;
11-
MyInterface i2 = new Foo(true, 2, "Foo2 implementation") as MyInterface;
12-
13-
Console.WriteLine(i1.GetAString());
14-
Console.WriteLine(i2.GetAString());
15-
Console.WriteLine(4);
10+
Console.WriteLine("Hello world");
1611
}
1712
}
18-
19-
interface MyInterface
20-
{
21-
int GetAnInt();
22-
bool GetABool();
23-
string GetAString();
24-
25-
}
26-
27-
class Foo : MyInterface
28-
{
29-
string s;
30-
protected int i;
31-
bool b;
32-
33-
public Foo(bool b, int i, string s)
34-
{
35-
this.s = s;
36-
this.b = b;
37-
this.i = i;
38-
}
39-
public bool GetABool()
40-
{
41-
return b;
42-
}
43-
44-
public virtual int GetAnInt()
45-
{
46-
return i;
47-
}
48-
49-
public string GetAString()
50-
{
51-
return s;
52-
}
53-
}
54-
55-
class Foo2 : MyInterface
56-
{
57-
string s;
58-
int i;
59-
bool b;
60-
61-
public Foo2(bool b, int i, string s)
62-
{
63-
this.s = s;
64-
this.b = b;
65-
this.i = i;
66-
}
67-
68-
public int GetAnInt()
69-
{
70-
return i;
71-
}
72-
73-
public bool GetABool()
74-
{
75-
return b;
76-
}
77-
78-
public string GetAString()
79-
{
80-
return s;
81-
}
82-
}

0 commit comments

Comments
 (0)