Skip to content

Commit

Permalink
feat: Support recursive data types (#2288)
Browse files Browse the repository at this point in the history 
Remove all the recursionCount nonsense.
This was added to prevent infinite recursion with types that reference themselves.

No need to check anymore, the weaver can generate readers and writers for types that reference themselves such as:

```cs
class Tree {
    Tree child1;
    Tree child2;
}
```

This works by the weaver doing it the way the compiler does: Create a function first, memoize it, then write the body. If the body needs the function, it will get itself and issue a call to itself.
  • Loading branch information
@paulpach
paulpach committed Sep 29, 2020
1 parent 513a0f9 commit 3ccb7d9
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 70 deletions.
46 changes: 20 additions & 26 deletions Assets/Mirror/Editor/Weaver/Readers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ namespace Mirror.Weaver
{
public static class Readers
{
const int MaxRecursionCount = 128;
static Dictionary<string, MethodReference> readFuncs;

public static void Init()
Expand All @@ -21,7 +20,7 @@ internal static void Register(TypeReference dataType, MethodReference methodRefe
readFuncs[dataType.FullName] = methodReference;
}

public static MethodReference GetReadFunc(TypeReference variableReference, int recursionCount = 0)
public static MethodReference GetReadFunc(TypeReference variableReference)
{
if (readFuncs.TryGetValue(variableReference.FullName, out MethodReference foundFunc))
{
Expand All @@ -34,7 +33,7 @@ public static MethodReference GetReadFunc(TypeReference variableReference, int r
// thus check if it is an array and skip all the checks.
if (variableReference.IsArray)
{
return GenerateArrayReadFunc(variableReference, recursionCount);
return GenerateArrayReadFunc(variableReference);
}

TypeDefinition variableDefinition = variableReference.Resolve();
Expand Down Expand Up @@ -86,14 +85,14 @@ public static MethodReference GetReadFunc(TypeReference variableReference, int r
}
else if (variableDefinition.Is(typeof(ArraySegment<>)))
{
return GenerateArraySegmentReadFunc(variableReference, recursionCount);
return GenerateArraySegmentReadFunc(variableReference);
}
else if (variableDefinition.Is(typeof(List<>)))
{
return GenerateListReadFunc(variableReference, recursionCount);
return GenerateListReadFunc(variableReference);
}

return GenerateClassOrStructReadFunction(variableReference, recursionCount);
return GenerateClassOrStructReadFunction(variableReference);
}

static void RegisterReadFunc(TypeReference typeReference, MethodDefinition newReaderFunc)
Expand All @@ -105,23 +104,23 @@ static void RegisterReadFunc(TypeReference typeReference, MethodDefinition newRe
Weaver.WeaveLists.generateContainerClass.Methods.Add(newReaderFunc);
}

static MethodDefinition GenerateArrayReadFunc(TypeReference variable, int recursionCount)
static MethodDefinition GenerateArrayReadFunc(TypeReference variable)
{
if (!variable.IsArrayType())
{
Weaver.Error($"{variable.Name} is an unsupported type. Jagged and multidimensional arrays are not supported", variable);
return null;
}

MethodDefinition readerFunc = GenerateReaderFunction(variable);
TypeReference elementType = variable.GetElementType();
MethodReference elementReadFunc = GetReadFunc(elementType, recursionCount + 1);
MethodReference elementReadFunc = GetReadFunc(elementType);
if (elementReadFunc == null)
{
Weaver.Error($"Cannot generate reader for Array because element {elementType.Name} does not have a reader. Use a supported type or provide a custom reader", variable);
return null;
return readerFunc;
}

MethodDefinition readerFunc = GenerateReaderFunction(variable);

readerFunc.Body.Variables.Add(new VariableDefinition(WeaverTypes.Import<int>()));
readerFunc.Body.Variables.Add(new VariableDefinition(variable));
Expand Down Expand Up @@ -201,12 +200,12 @@ static MethodDefinition GenerateEnumReadFunc(TypeReference variable)
return readerFunc;
}

static MethodDefinition GenerateArraySegmentReadFunc(TypeReference variable, int recursionCount)
static MethodDefinition GenerateArraySegmentReadFunc(TypeReference variable)
{
GenericInstanceType genericInstance = (GenericInstanceType)variable;
TypeReference elementType = genericInstance.GenericArguments[0];

MethodReference elementReadFunc = GetReadFunc(elementType, recursionCount + 1);
MethodReference elementReadFunc = GetReadFunc(elementType);
if (elementReadFunc == null)
{
Weaver.Error($"Cannot generate reader for ArraySegment because element {elementType.Name} does not have a reader. Use a supported type or provide a custom reader", variable);
Expand Down Expand Up @@ -293,19 +292,20 @@ private static MethodDefinition GenerateReaderFunction(TypeReference variable)
return readerFunc;
}

static MethodDefinition GenerateListReadFunc(TypeReference variable, int recursionCount)
static MethodDefinition GenerateListReadFunc(TypeReference variable)
{
MethodDefinition readerFunc = GenerateReaderFunction(variable);

GenericInstanceType genericInstance = (GenericInstanceType)variable;
TypeReference elementType = genericInstance.GenericArguments[0];

MethodReference elementReadFunc = GetReadFunc(elementType, recursionCount + 1);
MethodReference elementReadFunc = GetReadFunc(elementType);
if (elementReadFunc == null)
{
Weaver.Error($"Cannot generate reader for List because element {elementType.Name} does not have a reader. Use a supported type or provide a custom reader", variable);
return null;
return readerFunc;
}

MethodDefinition readerFunc = GenerateReaderFunction(variable);

readerFunc.Body.Variables.Add(new VariableDefinition(WeaverTypes.Import<int>()));
readerFunc.Body.Variables.Add(new VariableDefinition(variable));
Expand Down Expand Up @@ -380,14 +380,8 @@ static MethodDefinition GenerateListReadFunc(TypeReference variable, int recursi
}


static MethodDefinition GenerateClassOrStructReadFunction(TypeReference variable, int recursionCount)
static MethodDefinition GenerateClassOrStructReadFunction(TypeReference variable)
{
if (recursionCount > MaxRecursionCount)
{
Weaver.Error($"{variable.Name} can't be deserialized because it references itself", variable);
return null;
}

MethodDefinition readerFunc = GenerateReaderFunction(variable);

// create local for return value
Expand All @@ -401,7 +395,7 @@ static MethodDefinition GenerateClassOrStructReadFunction(TypeReference variable
GenerateNullCheck(worker);

CreateNew(variable, worker, td);
ReadAllFields(variable, recursionCount, worker);
ReadAllFields(variable, worker);

worker.Append(worker.Create(OpCodes.Ldloc_0));
worker.Append(worker.Create(OpCodes.Ret));
Expand Down Expand Up @@ -459,7 +453,7 @@ static void CreateNew(TypeReference variable, ILProcessor worker, TypeDefinition
}
}

static void ReadAllFields(TypeReference variable, int recursionCount, ILProcessor worker)
static void ReadAllFields(TypeReference variable, ILProcessor worker)
{
uint fields = 0;
foreach (FieldDefinition field in variable.FindAllPublicFields())
Expand All @@ -468,7 +462,7 @@ static void ReadAllFields(TypeReference variable, int recursionCount, ILProcesso
OpCode opcode = variable.IsValueType ? OpCodes.Ldloca : OpCodes.Ldloc;
worker.Append(worker.Create(opcode, 0));

MethodReference readFunc = GetReadFunc(field.FieldType, recursionCount + 1);
MethodReference readFunc = GetReadFunc(field.FieldType);
if (readFunc != null)
{
worker.Append(worker.Create(OpCodes.Ldarg_0));
Expand Down
72 changes: 30 additions & 42 deletions Assets/Mirror/Editor/Weaver/Writers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ namespace Mirror.Weaver
{
public static class Writers
{
const int MaxRecursionCount = 128;

static Dictionary<string, MethodReference> writeFuncs;

public static void Init()
Expand All @@ -21,9 +19,9 @@ public static void Register(TypeReference dataType, MethodReference methodRefere
writeFuncs[dataType.FullName] = methodReference;
}

static void RegisterWriteFunc(string name, MethodDefinition newWriterFunc)
static void RegisterWriteFunc(TypeReference typeReference, MethodDefinition newWriterFunc)
{
writeFuncs[name] = newWriterFunc;
writeFuncs[typeReference.FullName] = newWriterFunc;
Weaver.WeaveLists.generatedWriteFunctions.Add(newWriterFunc);

Weaver.WeaveLists.ConfirmGeneratedCodeClass();
Expand All @@ -37,36 +35,29 @@ static void RegisterWriteFunc(string name, MethodDefinition newWriterFunc)
/// <param name="variable"></param>
/// <param name="recursionCount"></param>
/// <returns>Returns <see cref="MethodReference"/> or null</returns>
public static MethodReference GetWriteFunc(TypeReference variable, int recursionCount = 0)
public static MethodReference GetWriteFunc(TypeReference variable)
{
if (writeFuncs.TryGetValue(variable.FullName, out MethodReference foundFunc))
{
return foundFunc;
}
else
{
MethodDefinition newWriterFunc = null;

// this try/catch will be removed in future PR and make `GetWriteFunc` throw instead
try
{
newWriterFunc = GenerateWriter(variable, recursionCount);
return GenerateWriter(variable);
}
catch (GenerateWriterException e)
{
Weaver.Error(e.Message, e.MemberReference);
return null;
}

if (newWriterFunc != null)
{
RegisterWriteFunc(variable.FullName, newWriterFunc);
}
return newWriterFunc;
}
}

/// <exception cref="GenerateWriterException">Throws when writer could not be generated for type</exception>
static MethodDefinition GenerateWriter(TypeReference variableReference, int recursionCount = 0)
static MethodDefinition GenerateWriter(TypeReference variableReference)
{
if (variableReference.IsByReference)
{
Expand All @@ -78,7 +69,7 @@ static MethodDefinition GenerateWriter(TypeReference variableReference, int recu
// therefore process this before checks below
if (variableReference.IsArray)
{
return GenerateArrayWriteFunc(variableReference, recursionCount);
return GenerateArrayWriteFunc(variableReference);
}

if (variableReference.Resolve()?.IsEnum ?? false)
Expand All @@ -91,11 +82,11 @@ static MethodDefinition GenerateWriter(TypeReference variableReference, int recu

if (variableReference.Is(typeof(ArraySegment<>)))
{
return GenerateArraySegmentWriteFunc(variableReference, recursionCount);
return GenerateArraySegmentWriteFunc(variableReference);
}
if (variableReference.Is(typeof(List<>)))
{
return GenerateListWriteFunc(variableReference, recursionCount);
return GenerateListWriteFunc(variableReference);
}

// check for invalid types
Expand Down Expand Up @@ -132,7 +123,7 @@ static MethodDefinition GenerateWriter(TypeReference variableReference, int recu

// generate writer for class/struct

return GenerateClassOrStructWriterFunction(variableReference, recursionCount);
return GenerateClassOrStructWriterFunction(variableReference);
}

private static MethodDefinition GenerateEnumWriteFunc(TypeReference variable)
Expand Down Expand Up @@ -164,24 +155,21 @@ private static MethodDefinition GenerateWriterFunc(TypeReference variable)
writerFunc.Parameters.Add(new ParameterDefinition("writer", ParameterAttributes.None, WeaverTypes.Import<Mirror.NetworkWriter>()));
writerFunc.Parameters.Add(new ParameterDefinition("value", ParameterAttributes.None, Weaver.CurrentAssembly.MainModule.ImportReference(variable)));
writerFunc.Body.InitLocals = true;

RegisterWriteFunc(variable, writerFunc);
return writerFunc;
}

static MethodDefinition GenerateClassOrStructWriterFunction(TypeReference variable, int recursionCount)
static MethodDefinition GenerateClassOrStructWriterFunction(TypeReference variable)
{
if (recursionCount > MaxRecursionCount)
{
throw new GenerateWriterException($"{variable.Name} can't be serialized because it references itself", variable);
}

MethodDefinition writerFunc = GenerateWriterFunc(variable);

ILProcessor worker = writerFunc.Body.GetILProcessor();

if (!variable.Resolve().IsValueType)
WriteNullCheck(worker);

if (!WriteAllFields(variable, recursionCount, worker))
if (!WriteAllFields(variable, worker))
return null;

worker.Append(worker.Create(OpCodes.Ret));
Expand Down Expand Up @@ -216,15 +204,14 @@ private static void WriteNullCheck(ILProcessor worker)
/// Find all fields in type and write them
/// </summary>
/// <param name="variable"></param>
/// <param name="recursionCount"></param>
/// <param name="worker"></param>
/// <returns>false if fail</returns>
static bool WriteAllFields(TypeReference variable, int recursionCount, ILProcessor worker)
static bool WriteAllFields(TypeReference variable, ILProcessor worker)
{
uint fields = 0;
foreach (FieldDefinition field in variable.FindAllPublicFields())
{
MethodReference writeFunc = GetWriteFunc(field.FieldType, recursionCount + 1);
MethodReference writeFunc = GetWriteFunc(field.FieldType);
// need this null check till later PR when GetWriteFunc throws exception instead
if (writeFunc == null) { return false; }

Expand All @@ -245,25 +232,25 @@ static bool WriteAllFields(TypeReference variable, int recursionCount, ILProcess
return true;
}

static MethodDefinition GenerateArrayWriteFunc(TypeReference variable, int recursionCount)
static MethodDefinition GenerateArrayWriteFunc(TypeReference variable)
{
if (!variable.IsArrayType())
{
throw new GenerateWriterException($"{variable.Name} is an unsupported type. Jagged and multidimensional arrays are not supported", variable);
}
MethodDefinition writerFunc = GenerateWriterFunc(variable);

TypeReference elementType = variable.GetElementType();
MethodReference elementWriteFunc = GetWriteFunc(elementType, recursionCount + 1);
MethodReference elementWriteFunc = GetWriteFunc(elementType);
MethodReference intWriterFunc = GetWriteFunc(WeaverTypes.Import<int>());

// need this null check till later PR when GetWriteFunc throws exception instead
if (elementWriteFunc == null)
{
Weaver.Error($"Cannot generate writer for Array because element {elementType.Name} does not have a writer. Use a supported type or provide a custom writer", variable);
return null;
return writerFunc;
}

MethodDefinition writerFunc = GenerateWriterFunc(variable);
writerFunc.Body.Variables.Add(new VariableDefinition(WeaverTypes.Import<int>()));
writerFunc.Body.Variables.Add(new VariableDefinition(WeaverTypes.Import<int>()));

Expand Down Expand Up @@ -332,22 +319,22 @@ static MethodDefinition GenerateArrayWriteFunc(TypeReference variable, int recur
return writerFunc;
}

static MethodDefinition GenerateArraySegmentWriteFunc(TypeReference variable, int recursionCount)
static MethodDefinition GenerateArraySegmentWriteFunc(TypeReference variable)
{
MethodDefinition writerFunc = GenerateWriterFunc(variable);

GenericInstanceType genericInstance = (GenericInstanceType)variable;
TypeReference elementType = genericInstance.GenericArguments[0];
MethodReference elementWriteFunc = GetWriteFunc(elementType, recursionCount + 1);
MethodReference elementWriteFunc = GetWriteFunc(elementType);
MethodReference intWriterFunc = GetWriteFunc(WeaverTypes.Import<int>());

// need this null check till later PR when GetWriteFunc throws exception instead
if (elementWriteFunc == null)
{
Weaver.Error($"Cannot generate writer for ArraySegment because element {elementType.Name} does not have a writer. Use a supported type or provide a custom writer", variable);
return null;
return writerFunc;
}

MethodDefinition writerFunc = GenerateWriterFunc(variable);

writerFunc.Body.Variables.Add(new VariableDefinition(WeaverTypes.Import<int>()));
writerFunc.Body.Variables.Add(new VariableDefinition(WeaverTypes.Import<int>()));

Expand Down Expand Up @@ -410,21 +397,22 @@ static MethodDefinition GenerateArraySegmentWriteFunc(TypeReference variable, in
return writerFunc;
}

static MethodDefinition GenerateListWriteFunc(TypeReference variable, int recursionCount)
static MethodDefinition GenerateListWriteFunc(TypeReference variable)
{
MethodDefinition writerFunc = GenerateWriterFunc(variable);

GenericInstanceType genericInstance = (GenericInstanceType)variable;
TypeReference elementType = genericInstance.GenericArguments[0];
MethodReference elementWriteFunc = GetWriteFunc(elementType, recursionCount + 1);
MethodReference elementWriteFunc = GetWriteFunc(elementType);
MethodReference intWriterFunc = GetWriteFunc(WeaverTypes.Import<int>());

// need this null check till later PR when GetWriteFunc throws exception instead
if (elementWriteFunc == null)
{
Weaver.Error($"Cannot generate writer for List because element {elementType.Name} does not have a writer. Use a supported type or provide a custom writer", variable);
return null;
return writerFunc;
}

MethodDefinition writerFunc = GenerateWriterFunc(variable);

writerFunc.Body.Variables.Add(new VariableDefinition(WeaverTypes.Import<int>()));
writerFunc.Body.Variables.Add(new VariableDefinition(WeaverTypes.Import<int>()));
Expand Down
3 changes: 1 addition & 2 deletions Assets/Mirror/Tests/Editor/Weaver/WeaverGeneralTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@ public class WeaverGeneralTests : WeaverTestsBuildFromTestName
[Test]
public void RecursionCount()
{
HasError("Potato1 can't be serialized because it references itself",
"WeaverGeneralTests.RecursionCount.RecursionCount/Potato1");
IsSuccess();
}

[Test]
Expand Down

0 comments on commit 3ccb7d9

Please sign in to comment.