Fixes StructLayout being mishandled (#205)

adrianoc · Aug 26, 2023 · 384faf5 · 384faf5
1 parent ad90bbe
commit 384faf5
Show file tree

Hide file tree

Showing 4 changed files with 90 additions and 9 deletions.
diff --git a/Cecilifier.Core.Tests/Tests/Unit/AttributesTest.cs b/Cecilifier.Core.Tests/Tests/Unit/AttributesTest.cs
@@ -1,3 +1,4 @@
+using System.Text.RegularExpressions;
 using NUnit.Framework;
 
 namespace Cecilifier.Core.Tests.Tests.Unit;
@@ -69,6 +70,39 @@ class Foo<TFoo> {{ }}");
                 @"cls_foo_\d+\.CustomAttributes\.Add\(\1\);"));        
     }
 
+    [TestCase("LayoutKind.Auto, Pack=1, Size=12", 1, 12)]
+    [TestCase("LayoutKind.Auto, Pack=1", 1, 0)]
+    [TestCase("LayoutKind.Auto, Size=42", 0, 42)]
+    [TestCase("LayoutKind.Sequential")]
+    public void StructLayout_ItNotEmitted(string initializationData, int expectedPack = -1, int expectedSize = -1)
+    {
+        // StructLayout attribute should not be emitted to metadata as an attribute;
+        // instead, the respective properties in the type definition should be set. 
+
+        var result = RunCecilifier($$"""
+                                   using System.Runtime.InteropServices;
+                                   [StructLayout({{initializationData}})]
+                                   struct Foo { }
+                                   """);
+
+        var cecilifiedCode = result.GeneratedCode.ReadToEnd();
+
+        Assert.That(cecilifiedCode, Does.Not.Match(@"st_foo_\d+.CustomAttributes.Add\(attr_structLayout_\d+\);"));
+
+        if (expectedSize == -1 && expectedPack == -1)
+        {
+            Assert.That(cecilifiedCode, Does.Not.Match(@"st_foo_\d+.ClassSize = \d+;"));
+            Assert.That(cecilifiedCode, Does.Not.Match(@"st_foo_\d+.PackingSize = \d+;"));
+        }
+        else
+        {
+            Assert.That(cecilifiedCode, Does.Match(@$"st_foo_\d+.ClassSize = {expectedSize};"));
+            Assert.That(cecilifiedCode, Does.Match(@$"st_foo_\d+.PackingSize = {expectedPack};"));
+        }
+
+        Assert.That(cecilifiedCode, Does.Match($@"\|\s+TypeAttributes.{Regex.Match(initializationData, @"LayoutKind\.([^,$]+)").Groups[1].Value}Layout"));
+    }
+
     private const string AttributeDefinition = "class MyAttribute : System.Attribute { public MyAttribute(string message) {} } ";
     private const string GenericAttributeDefinition = @"
                                 [System.AttributeUsage(System.AttributeTargets.All, AllowMultiple =true)]

diff --git a/Cecilifier.Core/AST/SyntaxWalkerBase.Private.cs b/Cecilifier.Core/AST/SyntaxWalkerBase.Private.cs
@@ -0,0 +1,26 @@
+using Microsoft.CodeAnalysis;
+
+namespace Cecilifier.Core.AST;
+
+// Helper types / members useful only to SyntaxWalkerBase.
+// Some of these may be refactored and made public/internal if needed in the future.
+internal partial class SyntaxWalkerBase
+{
+    internal enum AttributeKind
+    {
+        DllImport,
+        StructLayout,
+        Ordinary
+    }
+}
+
+public static class PrivateExtensions
+{
+    internal static SyntaxWalkerBase.AttributeKind AttributeKind(this ITypeSymbol self) => (self.ContainingNamespace.ToString(), self.Name) switch
+    {
+        ("System.Runtime.InteropServices", "DllImportAttribute") => SyntaxWalkerBase.AttributeKind.DllImport,
+        ("System.Runtime.InteropServices", "StructLayoutAttribute") => SyntaxWalkerBase.AttributeKind.StructLayout,
+        _ => SyntaxWalkerBase.AttributeKind.Ordinary,
+    };
+}
+
diff --git a/Cecilifier.Core/AST/SyntaxWalkerBase.cs b/Cecilifier.Core/AST/SyntaxWalkerBase.cs
@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Diagnostics.CodeAnalysis;
 using System.Linq;
 using System.Runtime.InteropServices;
 using System.Text;
@@ -17,7 +18,7 @@
 
 namespace Cecilifier.Core.AST
 {
-    internal class SyntaxWalkerBase : CSharpSyntaxWalker
+    internal partial class SyntaxWalkerBase : CSharpSyntaxWalker
     {
         internal SyntaxWalkerBase(IVisitorContext ctx)
         {
@@ -365,7 +366,7 @@ private static void AppendStructLayoutTo(ITypeSymbol typeSymbol, StringBuilder t
                     _ => throw new ArgumentException($"Invalid StructLayout value for {typeSymbol.Name}")
                 };
 
-                typeAttributes.AppendModifier($"TypeAttributes.{specifiedLayout}");
+                typeAttributes.AppendModifier(specifiedLayout);
             }
         }
 
@@ -798,10 +799,13 @@ private static void HandleAttributesInMemberDeclaration(IVisitorContext context,
                 //TODO: Pass the correct list of type parameters when C# supports generic attributes.
                 TypeDeclarationVisitor.EnsureForwardedTypeDefinition(context, type, Array.Empty<TypeParameterSyntax>());
 
-                var attrsExp = context.SemanticModel.GetSymbolInfo(attribute.Name).Symbol.IsDllImportCtor()
-                    ? ProcessDllImportAttribute(context, attribute, targetDeclarationVar)
-                    : ProcessNormalMemberAttribute(context, attribute, targetDeclarationVar);
-
+                var attrsExp = type.AttributeKind() switch
+                    {
+                        AttributeKind.DllImport => ProcessDllImportAttribute(context, attribute, targetDeclarationVar),
+                        AttributeKind.StructLayout => ProcessStructLayoutAttribute(attribute, targetDeclarationVar),
+                        _ => ProcessNormalMemberAttribute(context, attribute, targetDeclarationVar)
+                    };
+
                 AddCecilExpressions(context, attrsExp);
             }
         }
@@ -912,6 +916,26 @@ string AttributePropertyOrDefaultValue(AttributeSyntax attr, string propertyName
             }
         }
 
+        private static IEnumerable<string> ProcessStructLayoutAttribute(AttributeSyntax attribute, string typeVar)
+        {
+            Debug.Assert(attribute.ArgumentList != null);
+            if (attribute.ArgumentList.Arguments.Count == 0 || attribute.ArgumentList.Arguments.All(a => a.NameEquals == null))
+                return Array.Empty<string>();
+
+            return new[]
+            {
+                $"{typeVar}.ClassSize = { AssignedValue(attribute, "Size") };",
+                $"{typeVar}.PackingSize = { AssignedValue(attribute, "Pack") };",
+            };
+
+            static int AssignedValue(AttributeSyntax attribute, string parameterName)
+            {
+                // whenever Size/Pack are omitted the corresponding property should be set to 0. See Ecma-335 II 22.8.
+                var parameterAssignmentExpression = attribute.ArgumentList?.Arguments.FirstOrDefault(a => a.NameEquals?.Name.Identifier.Text == parameterName)?.Expression;
+                return parameterAssignmentExpression != null ? Int32.Parse(((LiteralExpressionSyntax) parameterAssignmentExpression).Token.Text) : 0;
+            }
+        }
+
         private static string CallingConventionToCecil(CallingConvention callingConvention)
         {
             var pinvokeAttribute = callingConvention switch

diff --git a/Cecilifier.Core/Extensions/ISymbolExtensions.cs b/Cecilifier.Core/Extensions/ISymbolExtensions.cs
@@ -1,7 +1,6 @@
 using System;
 using System.Diagnostics.CodeAnalysis;
 using System.Linq;
-using System.Runtime.CompilerServices;
 using Cecilifier.Core.AST;
 using Cecilifier.Core.Misc;
 using Cecilifier.Core.Naming;
@@ -89,8 +88,6 @@ public static T EnsureNotNull<T>([NotNullIfNotNull("symbol")] this T symbol) whe
             return symbol;
         }
 
-        public static bool IsDllImportCtor(this ISymbol self) => self != null && self.ContainingType.Name == "DllImportAttribute";
-
         public static string AsParameterAttribute(this IParameterSymbol symbol)
         {
             var refRelatedAttr = symbol.RefKind.AsParameterAttribute();