Skip to content

Commit

Permalink
improve unmanaged struct validation
Browse files Browse the repository at this point in the history
  • Loading branch information
neuecc committed Jan 2, 2023
1 parent 0604777 commit 760773b
Show file tree
Hide file tree
Showing 8 changed files with 317 additions and 15 deletions.
67 changes: 67 additions & 0 deletions sandbox/SandboxConsoleApp/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -631,3 +631,70 @@ public partial class Sample
[MemoryPackInclude]
int privateProperty2 { get; set; }
}


public struct DateTimeParamDefault
{
public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16
public long Timestamp; // 8
public bool IsItInSeconds; // 1(+padding7) = 8
}

[StructLayout(LayoutKind.Sequential)]
[MemoryPackable]
public partial struct TesTest
{
public ValueTuple<int, int> VTI;
public MyMessageHeader MyMsgHead;
public bool IsItInSeconds; // 1(+padding7) = 8
}

[StructLayout(LayoutKind.Sequential)]
[MemoryPackable]
public partial struct DateTimeParamSequential
{
public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16
public long Timestamp; // 8
public bool IsItInSeconds; // 1(+padding7) = 8
}

[StructLayout(LayoutKind.Auto)]
[MemoryPackable]
public partial struct DateTimeParamAuto
{
public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16
public long Timestamp; // 8
public bool IsItInSeconds; // 1(+padding7) = 8
}

[StructLayout(LayoutKind.Explicit, Size = 25)]
[MemoryPackable]
public partial struct DateTimeParamExplicit
{
[FieldOffset(9)]
public DateTimeOffset DateTime;
[FieldOffset(1)]
public long Timestamp; // 8
[FieldOffset(0)]
public bool IsItInSeconds; // 1
}

[StructLayout(LayoutKind.Auto)]
public struct MyMessageHeader
{
}

[MemoryPackable]
public partial struct HogeEEE
{
public int X;
public int Y;

// [MemoryPackConstructor]
public HogeEEE(int x, int y)
{
this.X = x;
this.Y = y;
}
}

74 changes: 74 additions & 0 deletions sandbox/SandboxNet6/Program.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
// See https://aka.ms/new-console-template for more information
using MemoryPack;
using System.Buffers;
using System.Drawing;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;


// System.Buffers.IBufferWriter<byte>
Expand Down Expand Up @@ -103,3 +105,75 @@ public partial class GenricUnionB<T> : IGenericUnion<T>
}


[MemoryPackable]
public partial struct PartialStructOne
{
public int X;
public int Y;

//[MemoryPackConstructor]
public PartialStructOne(int x)
{
this.X = x;
this.Y = 0;
}

//[MemoryPackConstructor]
public PartialStructOne(int x, int y)
{
this.X = x;
this.Y = y;
}
}

//[MemoryPackable]
//public partial struct DateTimeParamDefault
//{
// public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16
// public long Timestamp; // 8
// public bool IsItInSeconds; // 1(+padding7) = 8
//}

//[StructLayout(LayoutKind.Sequential)]
//[MemoryPackable]
//public partial struct TesTest
//{
// public ValueTuple<int, int> VTI;
// public MyMessageHeader MyMsgHead;
// public bool IsItInSeconds; // 1(+padding7) = 8
//}

//[StructLayout(LayoutKind.Sequential)]
//[MemoryPackable]
//public partial struct DateTimeParamSequential
//{
// public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16
// public long Timestamp; // 8
// public bool IsItInSeconds; // 1(+padding7) = 8
//}

//[StructLayout(LayoutKind.Auto)]
//[MemoryPackable]
//public partial struct DateTimeParamAuto
//{
// public DateTimeOffset DateTime; // short offset(2+padding) + dateTime/ulong(8) = 16
// public long Timestamp; // 8
// public bool IsItInSeconds; // 1(+padding7) = 8
//}

//[StructLayout(LayoutKind.Explicit, Size = 25)]
//[MemoryPackable]
//public partial struct DateTimeParamExplicit
//{
// [FieldOffset(9)]
// public DateTimeOffset DateTime;
// [FieldOffset(1)]
// public long Timestamp; // 8
// [FieldOffset(0)]
// public bool IsItInSeconds; // 1
//}

//[StructLayout(LayoutKind.Auto)]
//public struct MyMessageHeader
//{
//}
16 changes: 16 additions & 0 deletions src/MemoryPack.Generator/DiagnosticDescriptors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -276,4 +276,20 @@ internal static class DiagnosticDescriptors
category: Category,
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);

public static readonly DiagnosticDescriptor UnamangedStructWithLayoutAutoField = new(
id: "MEMPACK034",
title: "Before .NET 7 unmanaged struct must annotate LayoutKind.Auto or Explicit",
messageFormat: "The unmanaged struct '{0}' has LayoutKind.Auto field('{1}'). Before .NET 7, if field contains Auto then automatically promote to LayoutKind.Auto but .NET 7 is Sequential so breaking binary compatibility when runtime upgraded. To safety, you have to annotate [StructLayout(LayoutKind.Auto)] or LayoutKind.Explicit to type.",
category: Category,
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);

public static readonly DiagnosticDescriptor UnamangedStructMemoryPackCtor = new(
id: "MEMPACK035",
title: "Unamanged strcut does not allow [MemoryPackConstructor]",
messageFormat: "The unamanged struct '{0}' can not annotate with [MemoryPackConstructor] because don't call any constructors",
category: Category,
defaultSeverity: DiagnosticSeverity.Error,
isEnabledByDefault: true);
}
99 changes: 89 additions & 10 deletions src/MemoryPack.Generator/MemoryPackGenerator.Parser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using System.ComponentModel;
using System.Reflection;
using System.Runtime.InteropServices;
using System.Runtime.Serialization;
using System.Text;

Expand Down Expand Up @@ -40,7 +42,7 @@ public enum MemberKind
CustomFormatter, // used [MemoryPackCustomFormatterAttribtue]
}

partial class TypeMeta
public partial class TypeMeta
{
DiagnosticDescriptor? ctorInvalid = null;

Expand Down Expand Up @@ -145,13 +147,22 @@ public TypeMeta(INamedTypeSymbol symbol, ReferenceSymbols reference)
return null; // allows null as ok(not exists explicitly declared constructor == has implictly empty ctor)
}

if (ctors.Length == 1)
if (!Symbol.IsUnmanagedType && ctors.Length == 1)
{
return ctors[0];
}

var ctorWithAttrs = ctors.Where(x => x.ContainsAttribute(reference.MemoryPackConstructorAttribute)).ToArray();

if (Symbol.IsUnmanagedType)
{
if (ctorWithAttrs.Length != 0)
{
ctorInvalid = DiagnosticDescriptors.UnamangedStructMemoryPackCtor;
}
return null;
}

if (ctorWithAttrs.Length == 0)
{
ctorInvalid = DiagnosticDescriptors.MultipleCtorWithoutAttribute;
Expand Down Expand Up @@ -274,23 +285,86 @@ public bool Validate(TypeDeclarationSyntax syntax, IGeneratorContext context, bo
noError = false;
}

// ctor
if (ctorInvalid != null)
{
context.ReportDiagnostic(Diagnostic.Create(ctorInvalid, syntax.Identifier.GetLocation(), Symbol.Name));
noError = false;
}

// check ctor members
if (this.Constructor != null)
if (this.IsUnmanagedType)
{
var structLayoutFields = this.Symbol.GetAllMembers()
.OfType<IFieldSymbol>()
.Select(x =>
{
if (x.IsStatic) return null;

// ValueTuple, DateTime, DateTimeOffset is auto but can not get from Roslyn GetAttributes.

if (SymbolEqualityComparer.Default.Equals(x.Type, reference.KnownTypes.System_DateTime) || SymbolEqualityComparer.Default.Equals(x.Type, reference.KnownTypes.System_DateTimeOffset))
{
return Tuple.Create(x.Type, LayoutKind.Auto);
}

if (x.Type is INamedTypeSymbol nts && nts.IsGenericType)
{
var fullyQualifiedString = nts.ConstructUnboundGenericType().FullyQualifiedToString();
if (fullyQualifiedString.StartsWith("global::System.ValueTuple<"))
{
return Tuple.Create(x.Type, LayoutKind.Auto);
}
}

var structLayout = x.Type.GetAttribute(reference.KnownTypes.System_Runtime_InteropServices_StructLayout);
var layoutKind = (structLayout != null && structLayout.ConstructorArguments.Length != 0)
? structLayout.ConstructorArguments[0].Value
: null;

if (layoutKind != null)
{
return Tuple.Create(x.Type, (LayoutKind)layoutKind);
}

return null;
})
.Where(x => x != null && x.Item2 == LayoutKind.Auto)
.ToArray();

// has auto field, should mark Auto in lower Net6
if (structLayoutFields.Length != 0)
{
var structLayout = Symbol.GetAttribute(reference.KnownTypes.System_Runtime_InteropServices_StructLayout);
var layoutKind = (structLayout != null && structLayout.ConstructorArguments.Length != 0)
? structLayout.ConstructorArguments[0].Value
: null;

if (layoutKind == null || (LayoutKind)layoutKind == LayoutKind.Sequential)
{
var autoTypes = string.Join(", ", structLayoutFields.Select(x => x!.Item1.Name));

if (!context.IsNet7OrGreater)
{
context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.UnamangedStructWithLayoutAutoField, syntax.Identifier.GetLocation(), Symbol.Name, autoTypes));
noError = false;
}
}
}
}
else
{
var nameDict = new HashSet<string>(Members.Where(x => x.IsConstructorParameter).Select(x => x.Name), StringComparer.OrdinalIgnoreCase);
var allParameterExists = this.Constructor.Parameters.All(x => nameDict.Contains(x.Name));
if (!allParameterExists)
// check ctor members
if (this.Constructor != null)
{
var location = Constructor.Locations.FirstOrDefault() ?? syntax.Identifier.GetLocation();
var nameDict = new HashSet<string>(Members.Where(x => x.IsConstructorParameter).Select(x => x.Name), StringComparer.OrdinalIgnoreCase);
var allParameterExists = this.Constructor.Parameters.All(x => nameDict.Contains(x.Name));
if (!allParameterExists)
{
var location = Constructor.Locations.FirstOrDefault() ?? syntax.Identifier.GetLocation();

context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.ConstructorHasNoMatchedParameter, location, Symbol.Name));
noError = false;
context.ReportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.ConstructorHasNoMatchedParameter, location, Symbol.Name));
noError = false;
}
}
}

Expand Down Expand Up @@ -474,6 +548,11 @@ public bool Validate(TypeDeclarationSyntax syntax, IGeneratorContext context, bo

return noError;
}

public override string ToString()
{
return this.TypeName;
}
}

partial class MemberMeta
Expand Down
9 changes: 9 additions & 0 deletions src/MemoryPack.Generator/ReferenceSymbols.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Microsoft.CodeAnalysis;
using System.Reflection;
using System.Runtime.InteropServices;

namespace MemoryPack.Generator;

Expand Down Expand Up @@ -84,6 +85,10 @@ public class WellKnownTypes
public INamedTypeSymbol System_Collections_Generic_KeyValuePair_T { get; }
public INamedTypeSymbol System_Nullable_T { get; }

public INamedTypeSymbol System_DateTime { get; }
public INamedTypeSymbol System_DateTimeOffset { get; }
public INamedTypeSymbol System_Runtime_InteropServices_StructLayout { get; }

// netstandard2.0 source generator has there reference so use string instead...
//public INamedTypeSymbol System_Memory_T { get; }
//public INamedTypeSymbol System_ReadOnlyMemory_T { get; }
Expand Down Expand Up @@ -199,6 +204,10 @@ public WellKnownTypes(ReferenceSymbols parent)
//System_Buffers_ReadOnlySequence_T = GetTypeByMetadataName("System.Buffers.ReadOnlySequence").ConstructUnboundGenericType();
//System_Collections_Generic_PriorityQueue_T = GetTypeByMetadataName("System.Collections.Generic.PriorityQueue").ConstructUnboundGenericType();

System_DateTime = GetTypeByMetadataName("System.DateTime");
System_DateTimeOffset = GetTypeByMetadataName("System.DateTimeOffset");
System_Runtime_InteropServices_StructLayout = GetTypeByMetadataName("System.Runtime.InteropServices.StructLayoutAttribute");

knownTypes = new HashSet<ITypeSymbol>(new[]
{
System_Collections_Generic_IEnumerable_T,
Expand Down
4 changes: 2 additions & 2 deletions tests/MemoryPack.Tests/GeneratorDiagnosticsTest.TypeScript.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public partial class GeneratorDiagnosticsTest
{
void Compile2(int id, string code, bool allowMultipleError = false)
{
var diagnostics = CSharpGeneratorRunner.RunGenerator(code, new TypeScriptOptionProvider());
var diagnostics = CSharpGeneratorRunner.RunGenerator(code, options: new TypeScriptOptionProvider());
if (!allowMultipleError)
{
diagnostics.Length.Should().Be(1);
Expand All @@ -41,7 +41,7 @@ string CompileAndRead(string code, string fileName, bool enableNullableTypes = t
optionProvider["build_property.MemoryPackGenerator_TypeScriptOutputDirectory"] = outputDir;
optionProvider["build_property.MemoryPackGenerator_TypeScriptEnableNullableTypes"] = enableNullableTypes ? "true" : "false";

CSharpGeneratorRunner.RunGenerator(code, optionProvider);
CSharpGeneratorRunner.RunGenerator(code,options: optionProvider);

var outputFilePath = Path.Combine(outputDir, fileName);

Expand Down
Loading

0 comments on commit 760773b

Please sign in to comment.