Skip to content

Commit 75589ae

Browse files
jkotasjkotas
authored
Delete StructLayoutAttribute.Pack adjustment (#64965)
This adjustment was trying to minic adjustment performed by type loader, but the logic has been out-of-sync with the type loader evolution for years. It does not make sense for custom attribute reading via reflection to perform this adjustment. Fixes #12480
1 parent faaa900 commit 75589ae

6 files changed

Lines changed: 7 additions & 40 deletions

File tree

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1873,12 +1873,6 @@ internal static bool IsDefined(RuntimeFieldInfo field, RuntimeType? caType)
18731873
}
18741874
type.GetRuntimeModule().MetadataImport.GetClassLayout(type.MetadataToken, out int pack, out int size);
18751875

1876-
// Metadata parameter checking should not have allowed 0 for packing size.
1877-
// The runtime later converts a packing size of 0 to 8 so do the same here
1878-
// because it's more useful from a user perspective.
1879-
if (pack == 0)
1880-
pack = 8; // DEFAULT_PACKING_SIZE
1881-
18821876
StructLayoutAttribute attribute = new StructLayoutAttribute(layoutKind);
18831877

18841878
attribute.Pack = pack;

‎src/coreclr/nativeaot/System.Private.Reflection.Core/src/System/Reflection/Runtime/TypeInfos/RuntimeNamedTypeInfo.cs‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,6 @@ public sealed override StructLayoutAttribute StructLayoutAttribute
129129
{
130130
get
131131
{
132-
const int DefaultPackingSize = 8;
133-
134132
// Note: CoreClr checks HasElementType and IsGenericParameter in addition to IsInterface but those properties cannot be true here as this
135133
// RuntimeTypeInfo subclass is solely for TypeDef types.)
136134
if (IsInterface)
@@ -156,15 +154,7 @@ public sealed override StructLayoutAttribute StructLayoutAttribute
156154
default: charSet = CharSet.None; break;
157155
}
158156

159-
int pack;
160-
int size;
161-
GetPackSizeAndSize(out pack, out size);
162-
163-
// Metadata parameter checking should not have allowed 0 for packing size.
164-
// The runtime later converts a packing size of 0 to 8 so do the same here
165-
// because it's more useful from a user perspective.
166-
if (pack == 0)
167-
pack = DefaultPackingSize;
157+
GetPackSizeAndSize(out int pack, out int size);
168158

169159
return new StructLayoutAttribute(layoutKind)
170160
{

‎src/libraries/System.Reflection.MetadataLoadContext/src/System/Reflection/TypeLoading/Types/RoDefinitionType.cs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,6 @@ public sealed override StructLayoutAttribute? StructLayoutAttribute
146146
{
147147
get
148148
{
149-
const int DefaultPackingSize = 8;
150-
151149
// Note: CoreClr checks HasElementType and IsGenericParameter in addition to IsInterface but those properties cannot be true here as this
152150
// RoType subclass is solely for TypeDef types.)
153151
if (IsInterface)
@@ -170,12 +168,6 @@ public sealed override StructLayoutAttribute? StructLayoutAttribute
170168
};
171169
GetPackSizeAndSize(out int pack, out int size);
172170

173-
// Metadata parameter checking should not have allowed 0 for packing size.
174-
// The runtime later converts a packing size of 0 to 8 so do the same here
175-
// because it's more useful from a user perspective.
176-
if (pack == 0)
177-
pack = DefaultPackingSize;
178-
179171
return new StructLayoutAttribute(layoutKind)
180172
{
181173
CharSet = charSet,

‎src/libraries/System.Reflection.MetadataLoadContext/tests/src/Tests/Type/TypeTests.StructLayoutAttribute.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public static void Test_UndecoratedClass()
1515
StructLayoutAttribute s = t.StructLayoutAttribute;
1616
Assert.Equal(LayoutKind.Auto, s.Value);
1717
Assert.Equal(CharSet.Ansi, s.CharSet);
18-
Assert.Equal(8, s.Pack);
18+
Assert.Equal(0, s.Pack);
1919
Assert.Equal(0, s.Size);
2020
}
2121

@@ -71,7 +71,7 @@ public static void TestSequentialAutoZeroZero()
7171
StructLayoutAttribute s = t.StructLayoutAttribute;
7272
Assert.Equal(LayoutKind.Sequential, s.Value);
7373
Assert.Equal(CharSet.Auto, s.CharSet);
74-
Assert.Equal(8, s.Pack); // Not an error: Pack=0 is treated as if it were Pack=8.
74+
Assert.Equal(0, s.Pack);
7575
Assert.Equal(0, s.Size);
7676
}
7777

@@ -149,7 +149,7 @@ public static void Test_Derived()
149149
StructLayoutAttribute s = t.StructLayoutAttribute;
150150
Assert.Equal(LayoutKind.Auto, s.Value);
151151
Assert.Equal(CharSet.Ansi, s.CharSet);
152-
Assert.Equal(8, s.Pack);
152+
Assert.Equal(0, s.Pack);
153153
Assert.Equal(0, s.Size);
154154
}
155155

‎src/libraries/System.Reflection/tests/TypeInfoTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1490,9 +1490,9 @@ public void IsNestedFamORAssem(Type type, bool expected)
14901490
}
14911491

14921492
[Theory]
1493-
[InlineData(typeof(StructWithoutExplicitStructLayout), LayoutKind.Sequential, CharSet.Ansi, 8)]
1493+
[InlineData(typeof(StructWithoutExplicitStructLayout), LayoutKind.Sequential, CharSet.Ansi, 0)]
14941494
[InlineData(typeof(StructWithExplicitStructLayout), LayoutKind.Explicit, CharSet.Ansi, 1)]
1495-
[InlineData(typeof(ClassWithoutExplicitStructLayout), LayoutKind.Auto, CharSet.Ansi, 8)]
1495+
[InlineData(typeof(ClassWithoutExplicitStructLayout), LayoutKind.Auto, CharSet.Ansi, 0)]
14961496
[InlineData(typeof(ClassWithExplicitStructLayout), LayoutKind.Explicit, CharSet.Unicode, 2)]
14971497
public void StructLayoutAttribute(Type type, LayoutKind kind, CharSet charset, int pack)
14981498
{

‎src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2317,14 +2317,11 @@ public override bool IsSubclassOf(Type type)
23172317
return RuntimeTypeHandle.IsSubclassOf(this, rtType);
23182318
}
23192319

2320-
private const int DEFAULT_PACKING_SIZE = 8;
2321-
23222320
internal StructLayoutAttribute? GetStructLayoutAttribute()
23232321
{
23242322
if (IsInterface || HasElementType || IsGenericParameter)
23252323
return null;
23262324

2327-
int pack, size;
23282325
LayoutKind layoutKind = LayoutKind.Auto;
23292326
switch (Attributes & TypeAttributes.LayoutMask)
23302327
{
@@ -2343,13 +2340,7 @@ public override bool IsSubclassOf(Type type)
23432340
default: break;
23442341
}
23452342

2346-
GetPacking(out pack, out size);
2347-
2348-
// Metadata parameter checking should not have allowed 0 for packing size.
2349-
// The runtime later converts a packing size of 0 to 8 so do the same here
2350-
// because it's more useful from a user perspective.
2351-
if (pack == 0)
2352-
pack = DEFAULT_PACKING_SIZE;
2343+
GetPacking(out int pack, out int size);
23532344

23542345
return new StructLayoutAttribute(layoutKind) { Pack = pack, Size = size, CharSet = charSet };
23552346
}

0 commit comments

Comments
 (0)