dotnet
System.Numerics.Vectors reference assembly has reordered fields compared to runtime implementation
Description
We have a tool that generates some code based on a C# reference file. This file uses System.Numerics.Vectors for its math types. When we parse the Vector4 and Quaternion structs, Roslyn returns the components in WXYZ order. We thought this looked strange and maybe a bug on our side at first, because on C++ WindowsNumerics.h uses XYZW, but indeed it seems it's not us and there's a discrepancy here (I'll take Quaternion as example):
-
The core implementation uses XYZW: https://github.com/dotnet/runtime/blob/e64bc548c609455652fcd4107f1f4a2ac3084ff3/src/libraries/System.Private.CoreLib/src/System/Numerics/Quaternion.cs#L11-L25
-
The reference library uses WXYZ:
I assume that this is why Roslyn returns the fields in this order when calling
ITypeSymbol.GetMembers().OfType<IFieldSymbol>(). -
Incidentally, the official Microsoft docs probably follow the reference library and list WXYZ: https://docs.microsoft.com/en-us/dotnet/api/system.numerics.quaternion?view=net-5.0
The actual layout is important for us to decide if we generate some code for a blittable type or not (we assume Vector4 and Quaternion are, but previously we couldn't consider them blittable because we used WXYZ internally whereas they are projected in C++ in WindowsNumerics.h as XYZW). As @tannergooding pointed on another project comment we assume that the layout of those types is Sequential (should be the default for structs) and will not change since they're intrinsics.
Configuration
.NET 5.0 (but was already in .NET Core 3.x) Windows 10 x64 (though it shouldn't matter)
Regression?
Probably not.
Other information
Copying some internal analysis from @tannergooding for reference (and tagging @ericstj as requested):
These are autogenerated by GenAPI https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.GenAPI/Program.cs which uses CCI. At least browsing through the code that writes the fields (https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs#L182-L187) and which traverses the fields (https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs#L155) it looks like we are reordering the members here: https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs#L188 and that is likely causing the difference.
Tagging subscribers to this area: @tannergooding, @pgovind See info in area-owners.md if you want to be subscribed.
Issue Details
Description
We have a tool that generates some code based on a C# reference file. This file uses System.Numerics.Vectors for its math types. When we parse the Vector4 and Quaternion structs, Roslyn returns the components in WXYZ order. We thought this looked strange and maybe a bug on our side at first, because on C++ WindowsNumerics.h uses XYZW, but indeed it seems it's not us and there's a discrepancy here (I'll take Quaternion as example):
-
The core implementation uses XYZW: https://github.com/dotnet/runtime/blob/e64bc548c609455652fcd4107f1f4a2ac3084ff3/src/libraries/System.Private.CoreLib/src/System/Numerics/Quaternion.cs#L11-L25
-
The reference library uses WXYZ:
I assume that this is why Roslyn returns the fields in this order when calling
ITypeSymbol.GetMembers().OfType<IFieldSymbol>(). -
Incidentally, the official Microsoft docs probably follow the reference library and list WXYZ: https://docs.microsoft.com/en-us/dotnet/api/system.numerics.quaternion?view=net-5.0
The actual layout is important for us to decide if we generate some code for a blittable type or not (we assume Vector4 and Quaternion are, but previously we couldn't consider them blittable because we used WXYZ internally whereas they are projected in C++ in WindowsNumerics.h as XYZW). As @tannergooding pointed on another project comment we assume that the layout of those types is Sequential (should be the default for structs) and will not change since they're intrinsics.
Configuration
.NET 5.0 (but was already in .NET Core 3.x) Windows 10 x64 (though it shouldn't matter)
Regression?
Probably not.
Other information
Copying some internal analysis from @tannergooding for reference (and tagging @ericstj as requested):
These are autogenerated by GenAPI https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.GenAPI/Program.cs which uses CCI. At least browsing through the code that writes the fields (https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs#L182-L187) and which traverses the fields (https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs#L155) it looks like we are reordering the members here: https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs#L188 and that is likely causing the difference.
| Author: | djee-ms |
|---|---|
| Assignees: | - |
| Labels: |
|
| Milestone: | - |
Tagging subscribers to this area: @safern, @viktorhofer See info in area-owners.md if you want to be subscribed.
Issue Details
Description
We have a tool that generates some code based on a C# reference file. This file uses System.Numerics.Vectors for its math types. When we parse the Vector4 and Quaternion structs, Roslyn returns the components in WXYZ order. We thought this looked strange and maybe a bug on our side at first, because on C++ WindowsNumerics.h uses XYZW, but indeed it seems it's not us and there's a discrepancy here (I'll take Quaternion as example):
-
The core implementation uses XYZW: https://github.com/dotnet/runtime/blob/e64bc548c609455652fcd4107f1f4a2ac3084ff3/src/libraries/System.Private.CoreLib/src/System/Numerics/Quaternion.cs#L11-L25
-
The reference library uses WXYZ:
I assume that this is why Roslyn returns the fields in this order when calling
ITypeSymbol.GetMembers().OfType<IFieldSymbol>(). -
Incidentally, the official Microsoft docs probably follow the reference library and list WXYZ: https://docs.microsoft.com/en-us/dotnet/api/system.numerics.quaternion?view=net-5.0
The actual layout is important for us to decide if we generate some code for a blittable type or not (we assume Vector4 and Quaternion are, but previously we couldn't consider them blittable because we used WXYZ internally whereas they are projected in C++ in WindowsNumerics.h as XYZW). As @tannergooding pointed on another project comment we assume that the layout of those types is Sequential (should be the default for structs) and will not change since they're intrinsics.
Configuration
.NET 5.0 (but was already in .NET Core 3.x) Windows 10 x64 (though it shouldn't matter)
Regression?
Probably not.
Other information
Copying some internal analysis from @tannergooding for reference (and tagging @ericstj as requested):
These are autogenerated by GenAPI https://github.com/dotnet/arcade/blob/master/src/Microsoft.DotNet.GenAPI/Program.cs which uses CCI. At least browsing through the code that writes the fields (https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.cs#L182-L187) and which traverses the fields (https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs#L155) it looks like we are reordering the members here: https://github.com/dotnet/arcade/blob/4873d157a8f34f8cc7e28b3f9938b32c642ef542/src/Microsoft.Cci.Extensions/Writers/CSharp/CSharpWriter.cs#L188 and that is likely causing the difference.
| Author: | djee-ms |
|---|---|
| Assignees: | - |
| Labels: |
|
| Milestone: | - |
We need to make GenAPI preserve the field order for structs when LayoutKind = sequential. We should also have APICompat check for this.
@ericstj, it should be for any type where LayoutKind = Sequential and where the fields are publicly visible I think.
C# structs are LayoutKind.Sequential by default, while classes are LayoutKind.Auto, but the latter can also become explicitly marked Sequential and used for interop scenarios as well.
However, users shouldn't be relying on internal implementation details (private fields) so I think we can continue changing that to the single private int _field; for that case.
It might be interesting to have a check that we don't have any types that are Sequential and with both private and public fields.
LayoutKind.Explicit probably also needs to be considered, but I don't think we have any such types today.
We need to make GenAPI preserve the field order for structs when LayoutKind = sequential. We should also have APICompat check for this. @ericstj, it should be for any type where LayoutKind = Sequential and where the fields are publicly visible I think. LayoutKind.Explicit probably also needs to be considered, but I don't think we have any such types today.
I added this to the roslyn based GenAPI notes.
LayoutKind.Explicit probably also needs to be considered,
I suspect this will happen automatically since the attributes will be preserved, but it's good to call this out.
Moving this to 7.0 as we won't use a new GenAPI in 6.0. For 7.0 we might even switch to Roslyn's RefOut which should preserve the layout.
@ericstj, any issues with me manually fixing this for .NET 7.
Vector2/3/4 are "special" here in that they some of the only public types in the BCL with public fields, so this won't be widespread
@tannergooding no issue fixing manually for .NET 7. I suspect the GenAPI change for this isn't too difficult so it won't be much in the way of dead code. @ViktorHofer are you tracking this for a rule in the new API compat as well? cc @smasher164
Presumably this will be fixed with the new Roslyn based GenAPI tooling. @andriipatsula can you please verify that?
@ViktorHofer are you tracking this for a rule in the new API compat as well? cc @smasher164
https://github.com/dotnet/sdk/issues/26460