Skip to content

Commit

Permalink
fix: generic arguments resolution (#2300)
Browse files Browse the repository at this point in the history
* fix: generic arguments lookup

The weaver was not able to figure out the synclist type in code like this:

```cs
    public class SomeList<G, T> : SyncList<T> { }
    public class SomeListInt : SomeList<string, int> { }
```

This code fixes that problem, and greatly reduces the complexity
of argument lookup.

* linting
  • Loading branch information
paulpach committed Sep 29, 2020
1 parent 720a06e commit 8dbf467
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 117 deletions.
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/Weaver/Extensions.cs
Expand Up @@ -249,12 +249,12 @@ public static MethodDefinition GetMethodInBaseType(this TypeDefinition td, strin
/// <param name="methodName"></param>
/// <param name="stopAt"></param>
/// <returns></returns>
public static bool HasMethodInBaseType(this TypeDefinition td, string methodName, TypeReference stopAt)
public static bool HasMethodInBaseType(this TypeDefinition td, string methodName, Type stopAt)
{
TypeDefinition typedef = td;
while (typedef != null)
{
if (typedef.FullName == stopAt.FullName)
if (typedef.Is(stopAt))
break;

foreach (MethodDefinition md in typedef.Methods)
Expand Down
117 changes: 33 additions & 84 deletions Assets/Mirror/Editor/Weaver/Processors/GenericArgumentResolver.cs
@@ -1,107 +1,56 @@
using System.Collections.Generic;
using System;
using Mono.CecilX;

namespace Mirror.Weaver
{
public class GenericArgumentResolver
public static class GenericArgumentResolver
{
readonly Stack<TypeReference> stack = new Stack<TypeReference>();
readonly int maxGenericArgument;

public GenericArgumentResolver(int maxGenericArgument)
static TypeReference[] GetGenericArguments(TypeReference tr, Type baseType, TypeReference[] genericArguments)
{
this.maxGenericArgument = maxGenericArgument;
}
if (tr == null)
return null;

public bool GetGenericFromBaseClass(TypeDefinition td, int genericArgument, TypeReference baseType, out TypeReference itemType)
{
itemType = null;
if (GetGenericBaseType(td, baseType, out GenericInstanceType parent))
{
TypeReference arg = parent.GenericArguments[genericArgument];
if (arg.IsGenericParameter)
{
itemType = FindParameterInStack(td, genericArgument);
}
else
{
itemType = Weaver.CurrentAssembly.MainModule.ImportReference(arg);
}
}
TypeReference[] resolvedArguments = new TypeReference[0];

return itemType != null;
}

TypeReference FindParameterInStack(TypeDefinition td, int genericArgument)
{
while (stack.Count > 0)
if (tr is GenericInstanceType genericInstance)
{
TypeReference next = stack.Pop();
// this type is a generic instance, for example List<int>
// however, the parameter may be generic itself, for example:
// List<T>. If T is a generic parameter, then look it up in
// from the arguments we got.
resolvedArguments = genericInstance.GenericArguments.ToArray();

if (!(next is GenericInstanceType genericType))
for (int i = 0; i< resolvedArguments.Length; i++)
{
// if type is not GenericInstanceType something has gone wrong
return null;
TypeReference argument = resolvedArguments[i];
if (argument is GenericParameter genericArgument)
{
resolvedArguments[i] = genericArguments[genericArgument.Position];
}
}
}

if (genericType.GenericArguments.Count < genericArgument)
{
// if less than `genericArgument` then we didnt find generic argument
return null;
}
if (tr.Is(baseType))
return resolvedArguments;

if (genericType.GenericArguments.Count > maxGenericArgument)
{
// if greater than `genericArgument` it is hard to know which generic arg we want
// See SyncListGenericInheritanceWithMultipleGeneric test
Weaver.Error($"Type {td.Name} has too many generic arguments in base class {next}", td);
return null;
}
if (tr.CanBeResolved())
return GetGenericArguments(tr.Resolve().BaseType, baseType, resolvedArguments);

TypeReference genericArg = genericType.GenericArguments[genericArgument];
if (!genericArg.IsGenericParameter)
{
// if not generic, successfully found type
return Weaver.CurrentAssembly.MainModule.ImportReference(genericArg);
}
}

// nothing left in stack, something went wrong
return null;
}

bool GetGenericBaseType(TypeDefinition td, TypeReference baseType, out GenericInstanceType found)
/// <summary>
/// Find out what the arguments are to a generic base type
/// </summary>
/// <param name="td"></param>
/// <param name="baseType"></param>
/// <returns></returns>
public static TypeReference[] GetGenericArguments(TypeDefinition td, Type baseType)
{
stack.Clear();
TypeReference parent = td.BaseType;
found = null;

while (parent != null)
{
string parentName = parent.FullName;

// strip generic <T> parameters from class name (if any)
parentName = Extensions.StripGenericParametersFromClassName(parentName);

if (parentName == baseType.FullName)
{
found = parent as GenericInstanceType;
break;
}

try
{
stack.Push(parent);
parent = parent.Resolve().BaseType;
}
catch (AssemblyResolutionException)
{
// this can happen for plugins.
break;
}
}
if (td == null)
return null;

return found != null;
return GetGenericArguments(td.BaseType, baseType, new TypeReference[] { });
}
}
}
19 changes: 4 additions & 15 deletions Assets/Mirror/Editor/Weaver/Processors/SyncDictionaryProcessor.cs
Expand Up @@ -13,22 +13,11 @@ static class SyncDictionaryProcessor
/// <param name="td">The synclist class</param>
public static void Process(TypeDefinition td)
{
GenericArgumentResolver resolver = new GenericArgumentResolver(2);
TypeReference syncDictionaryType = WeaverTypes.Import(typeof(SyncDictionary<,>));

if (resolver.GetGenericFromBaseClass(td, 0, syncDictionaryType, out TypeReference keyType))
{
SyncObjectProcessor.GenerateSerialization(td, keyType, syncDictionaryType, "SerializeKey", "DeserializeKey");
}
else
{
Weaver.Error($"Could not find generic arguments for SyncDictionary in {td.Name}", td);
return;
}

if (resolver.GetGenericFromBaseClass(td, 1, syncDictionaryType, out TypeReference itemType))
TypeReference []arguments = GenericArgumentResolver.GetGenericArguments(td, typeof(SyncDictionary<,>));
if (arguments != null)
{
SyncObjectProcessor.GenerateSerialization(td, itemType, syncDictionaryType, "SerializeItem", "DeserializeItem");
SyncObjectProcessor.GenerateSerialization(td, arguments[0], typeof(SyncDictionary<,>), "SerializeKey", "DeserializeKey");
SyncObjectProcessor.GenerateSerialization(td, arguments[1], typeof(SyncDictionary<,>), "SerializeItem", "DeserializeItem");
}
else
{
Expand Down
10 changes: 5 additions & 5 deletions Assets/Mirror/Editor/Weaver/Processors/SyncListProcessor.cs
@@ -1,3 +1,4 @@
using System;
using Mono.CecilX;

namespace Mirror.Weaver
Expand All @@ -12,13 +13,12 @@ static class SyncListProcessor
/// </summary>
/// <param name="td">The synclist class</param>
/// <param name="mirrorBaseType">the base SyncObject td inherits from</param>
public static void Process(TypeDefinition td, TypeReference mirrorBaseType)
public static void Process(TypeDefinition td, Type mirrorBaseType)
{
GenericArgumentResolver resolver = new GenericArgumentResolver(1);

if (resolver.GetGenericFromBaseClass(td, 0, mirrorBaseType, out TypeReference itemType))
TypeReference [] arguments = GenericArgumentResolver.GetGenericArguments(td, mirrorBaseType);
if (arguments != null)
{
SyncObjectProcessor.GenerateSerialization(td, itemType, mirrorBaseType, "SerializeItem", "DeserializeItem");
SyncObjectProcessor.GenerateSerialization(td, arguments[0], mirrorBaseType, "SerializeItem", "DeserializeItem");
}
else
{
Expand Down
7 changes: 4 additions & 3 deletions Assets/Mirror/Editor/Weaver/Processors/SyncObjectProcessor.cs
@@ -1,3 +1,4 @@
using System;
using System.Collections.Generic;
using Mono.CecilX;
using Mono.CecilX.Cil;
Expand Down Expand Up @@ -48,7 +49,7 @@ public static List<FieldDefinition> FindSyncObjectsFields(TypeDefinition td)
/// <param name="mirrorBaseType">the base SyncObject td inherits from</param>
/// <param name="serializeMethod">The name of the serialize method</param>
/// <param name="deserializeMethod">The name of the deserialize method</param>
public static void GenerateSerialization(TypeDefinition td, TypeReference itemType, TypeReference mirrorBaseType, string serializeMethod, string deserializeMethod)
public static void GenerateSerialization(TypeDefinition td, TypeReference itemType, Type mirrorBaseType, string serializeMethod, string deserializeMethod)
{
Weaver.DLog(td, "SyncObjectProcessor Start item:" + itemType.FullName);

Expand All @@ -65,7 +66,7 @@ public static void GenerateSerialization(TypeDefinition td, TypeReference itemTy
}

// serialization of individual element
static bool GenerateSerialization(string methodName, TypeDefinition td, TypeReference itemType, TypeReference mirrorBaseType)
static bool GenerateSerialization(string methodName, TypeDefinition td, TypeReference itemType, Type mirrorBaseType)
{
Weaver.DLog(td, " GenerateSerialization");
bool existing = td.HasMethodInBaseType(methodName, mirrorBaseType);
Expand Down Expand Up @@ -109,7 +110,7 @@ static bool GenerateSerialization(string methodName, TypeDefinition td, TypeRefe
return true;
}

static bool GenerateDeserialization(string methodName, TypeDefinition td, TypeReference itemType, TypeReference mirrorBaseType)
static bool GenerateDeserialization(string methodName, TypeDefinition td, TypeReference itemType, Type mirrorBaseType)
{
Weaver.DLog(td, " GenerateDeserialization");
bool existing = td.HasMethodInBaseType(methodName, mirrorBaseType);
Expand Down
4 changes: 2 additions & 2 deletions Assets/Mirror/Editor/Weaver/Weaver.cs
Expand Up @@ -210,12 +210,12 @@ static bool WeaveSyncObject(TypeDefinition td)
{
if (td.IsDerivedFrom(typeof(SyncList<>)))
{
SyncListProcessor.Process(td, WeaverTypes.Import(typeof(SyncList<>)));
SyncListProcessor.Process(td, typeof(SyncList<>));
modified = true;
}
else if (td.IsDerivedFrom(typeof(SyncSet<>)))
{
SyncListProcessor.Process(td, WeaverTypes.Import(typeof(SyncSet<>)));
SyncListProcessor.Process(td, typeof(SyncSet<>));
modified = true;
}
else if (td.IsDerivedFrom(typeof(SyncDictionary<,>)))
Expand Down
3 changes: 1 addition & 2 deletions Assets/Mirror/Tests/Editor/Weaver/WeaverSyncListTests.cs
Expand Up @@ -31,8 +31,7 @@ public void SyncListGenericInheritance()
[Test]
public void SyncListGenericInheritanceWithMultipleGeneric()
{
HasError("Could not find generic arguments for SyncList`1 in WeaverSyncListTests.SyncListGenericInheritanceWithMultipleGeneric.SyncListGenericInheritanceWithMultipleGeneric/SomeListInt",
"WeaverSyncListTests.SyncListGenericInheritanceWithMultipleGeneric.SyncListGenericInheritanceWithMultipleGeneric/SomeListInt");
IsSuccess();
}

[Test]
Expand Down
Expand Up @@ -3,11 +3,8 @@
namespace WeaverSyncListTests.SyncListGenericInheritanceWithMultipleGeneric
{
/*
This test will fail
It is hard to know which generic argument we want from `SomeList<string, int>`
So instead give a useful error for this edge case
This test should pass
*/

class SyncListGenericInheritanceWithMultipleGeneric : NetworkBehaviour
{
readonly SomeListInt someList = new SomeListInt();
Expand Down

0 comments on commit 8dbf467

Please sign in to comment.