Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete StructLayoutAttribute.Pack adjustment #64965

Merged
merged 1 commit into from
Feb 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1873,12 +1873,6 @@ internal static bool IsDefined(RuntimeFieldInfo field, RuntimeType? caType)
}
type.GetRuntimeModule().MetadataImport.GetClassLayout(type.MetadataToken, out int pack, out int size);

// Metadata parameter checking should not have allowed 0 for packing size.
// The runtime later converts a packing size of 0 to 8 so do the same here
// because it's more useful from a user perspective.
if (pack == 0)
pack = 8; // DEFAULT_PACKING_SIZE

StructLayoutAttribute attribute = new StructLayoutAttribute(layoutKind);

attribute.Pack = pack;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,6 @@ public sealed override StructLayoutAttribute StructLayoutAttribute
{
get
{
const int DefaultPackingSize = 8;

// Note: CoreClr checks HasElementType and IsGenericParameter in addition to IsInterface but those properties cannot be true here as this
// RuntimeTypeInfo subclass is solely for TypeDef types.)
if (IsInterface)
Expand All @@ -156,15 +154,7 @@ public sealed override StructLayoutAttribute StructLayoutAttribute
default: charSet = CharSet.None; break;
}

int pack;
int size;
GetPackSizeAndSize(out pack, out size);

// Metadata parameter checking should not have allowed 0 for packing size.
// The runtime later converts a packing size of 0 to 8 so do the same here
// because it's more useful from a user perspective.
if (pack == 0)
pack = DefaultPackingSize;
GetPackSizeAndSize(out int pack, out int size);

return new StructLayoutAttribute(layoutKind)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ public sealed override StructLayoutAttribute? StructLayoutAttribute
{
get
{
const int DefaultPackingSize = 8;

// Note: CoreClr checks HasElementType and IsGenericParameter in addition to IsInterface but those properties cannot be true here as this
// RoType subclass is solely for TypeDef types.)
if (IsInterface)
Expand All @@ -170,12 +168,6 @@ public sealed override StructLayoutAttribute? StructLayoutAttribute
};
GetPackSizeAndSize(out int pack, out int size);

// Metadata parameter checking should not have allowed 0 for packing size.
// The runtime later converts a packing size of 0 to 8 so do the same here
// because it's more useful from a user perspective.
if (pack == 0)
pack = DefaultPackingSize;

return new StructLayoutAttribute(layoutKind)
{
CharSet = charSet,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public static void Test_UndecoratedClass()
StructLayoutAttribute s = t.StructLayoutAttribute;
Assert.Equal(LayoutKind.Auto, s.Value);
Assert.Equal(CharSet.Ansi, s.CharSet);
Assert.Equal(8, s.Pack);
Assert.Equal(0, s.Pack);
Assert.Equal(0, s.Size);
}

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

Expand Down Expand Up @@ -149,7 +149,7 @@ public static void Test_Derived()
StructLayoutAttribute s = t.StructLayoutAttribute;
Assert.Equal(LayoutKind.Auto, s.Value);
Assert.Equal(CharSet.Ansi, s.CharSet);
Assert.Equal(8, s.Pack);
Assert.Equal(0, s.Pack);
Assert.Equal(0, s.Size);
}

Expand Down
4 changes: 2 additions & 2 deletions src/libraries/System.Reflection/tests/TypeInfoTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1490,9 +1490,9 @@ public void IsNestedFamORAssem(Type type, bool expected)
}

[Theory]
[InlineData(typeof(StructWithoutExplicitStructLayout), LayoutKind.Sequential, CharSet.Ansi, 8)]
[InlineData(typeof(StructWithoutExplicitStructLayout), LayoutKind.Sequential, CharSet.Ansi, 0)]
[InlineData(typeof(StructWithExplicitStructLayout), LayoutKind.Explicit, CharSet.Ansi, 1)]
[InlineData(typeof(ClassWithoutExplicitStructLayout), LayoutKind.Auto, CharSet.Ansi, 8)]
[InlineData(typeof(ClassWithoutExplicitStructLayout), LayoutKind.Auto, CharSet.Ansi, 0)]
[InlineData(typeof(ClassWithExplicitStructLayout), LayoutKind.Explicit, CharSet.Unicode, 2)]
public void StructLayoutAttribute(Type type, LayoutKind kind, CharSet charset, int pack)
{
Expand Down
11 changes: 1 addition & 10 deletions src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2317,14 +2317,11 @@ public override bool IsSubclassOf(Type type)
return RuntimeTypeHandle.IsSubclassOf(this, rtType);
}

private const int DEFAULT_PACKING_SIZE = 8;

internal StructLayoutAttribute? GetStructLayoutAttribute()
{
if (IsInterface || HasElementType || IsGenericParameter)
return null;

int pack, size;
LayoutKind layoutKind = LayoutKind.Auto;
switch (Attributes & TypeAttributes.LayoutMask)
{
Expand All @@ -2343,13 +2340,7 @@ public override bool IsSubclassOf(Type type)
default: break;
}

GetPacking(out pack, out size);

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

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