Project Windows Runtime struct fields as C# fields - #2505
Project Windows Runtime struct fields as C# fields#2505Sergio Pedri (Sergio0694) wants to merge 5 commits into
Conversation
Projected structs were emitting each Windows Runtime struct field as a
'{ readonly get; set; }' auto-property. That turned out to be problematic,
most notably for authoring: the WinMD generator maps public instance fields
back to Windows Runtime struct fields, so an authored struct written against
the projected shape produced an empty struct in the generated metadata. It
also prevents callers from taking a reference to a member, and is simply
confusing, given Windows Runtime structs are plain data.
Emit plain public fields instead, declared before the constructor so that
declaration order (which drives the sequential layout of the struct) mirrors
the metadata order.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d136d263-1a79-45e8-8a92-fb6dd28b2d79
'Point', 'Rect', 'Size' and 'EventRegistrationToken' are manually projected in
'WinRT.Runtime', and were mirroring the Windows Runtime struct fields as
'{ readonly get; set; }' properties. Expose them as plain public fields, to
match how projected structs are now generated.
The negative-value validation that lived in the 'Rect.Width'/'Rect.Height' and
'Size.Width'/'Size.Height' setters moves into the constructors that document it
(this is also what CsWinRT 2.x did). The remaining members ('Left', 'Top',
'Right', 'Bottom', 'IsEmpty', 'Empty', ...) are managed-only conveniences and
stay as properties.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d136d263-1a79-45e8-8a92-fb6dd28b2d79
The custom-mapped XAML struct types are defined in full by their addition
files, and mirrored the Windows Runtime struct fields as properties over
private backing fields (a shape inherited from the UWP/WPF managed
projections). Now that generated structs project their fields as C# fields,
these would otherwise be inconsistent with their siblings: 'Thickness',
'Matrix', 'GeneratorPosition' and 'Color' are generated types (so their
members are fields), while 'CornerRadius' and friends are hand-written.
Expose the actual metadata fields as public fields for:
- 'CornerRadius': TopLeft, TopRight, BottomRight, BottomLeft
- 'GridLength': Value, GridUnitType
- 'Duration': TimeSpan, Type
- 'KeyTime': TimeSpan
- 'RepeatBehavior': Count, Duration, Type
- 'Matrix3D': M11-M34, OffsetX, OffsetY, OffsetZ, M44
Field declaration order (and therefore the sequential layout each of these
blittable structs marshals with) is unchanged. Managed-only members are left
untouched: 'GridLength.IsAbsolute'/'IsAuto'/'IsStar'/'Auto',
'Duration.HasTimeSpan'/'Automatic'/'Forever', 'RepeatBehavior.HasCount'/
'HasDuration'/'Forever', 'Matrix3D.Identity'/'IsIdentity'/'HasInverse'/... and
all the operators, conversions and formatting helpers.
Validation that lived in the removed property setters is preserved where the
constructors already performed it ('CornerRadius.Validate', 'GridLength',
'KeyTime.FromTimeSpan'). 'GridLength' and 'KeyTime' are no longer 'readonly
struct'-s, since Windows Runtime structs are mutable data.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: d136d263-1a79-45e8-8a92-fb6dd28b2d79
Refreshes the 'EventRegistrationToken' snippet in the event infrastructure doc, and documents the projection rule in the CsWinRT 3.0 spec (renaming the neighbouring 'Point'/'Rect'/'Size' section, which was already about their fields rather than properties). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d136d263-1a79-45e8-8a92-fb6dd28b2d79
Both comments justified skipping ABI struct emission / per-field marshalling for mapped structs by claiming their projected public fields don't match the WinMD field layout. That is no longer accurate now that the addition files expose the metadata fields directly. The actual reason is that these types are defined in full by an addition file rather than generated from metadata, so they're passed through by value and only get BoxToUnmanaged/UnboxToManaged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d136d263-1a79-45e8-8a92-fb6dd28b2d79
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
| #endif | ||
| public readonly struct Duration : IEquatable<Duration> | ||
| public struct Duration : IEquatable<Duration> | ||
| { |
There was a problem hiding this comment.
These always had special handling as properties before to enable throwing the exception and so on, do we want to change that?
There was a problem hiding this comment.
Same as below, basically I'm not convinced we should be arbitrarily changing the WinRT API surface here? Like, we can add new helpers, constructors, etc. and stuff that adds to WinRT types, but we have no precedent in 3.0 for just modifying existing APIs. It would also make these types be inconsistent with the other ones: no projected struct is readonly, and all other projected structs always keep the projected fields public. I feel like if people set wrong values we should just let WinUI itself throw an exception appropriately (I also don't remember ever hitting this exception myself, so I'm not even convinced it's that common/useful). And we can still keep the validation in the constructors, since those augment the WinRT types.
What do you think?
Edit: also to note, changing the public fields makes the docs for them incorrect too (since the fields are not there).
There was a problem hiding this comment.
The docs do have this section today. Similar section exists for GridLength too and I believe the others that were customized. It looks like they were customized for providing additional safety, not sure if they gained additional optimizations by being readonly for these particular types. I can see both sides to this.
There was a problem hiding this comment.
That page would still be valid. We do maintain all of those members, which are just additions. And we'd actually still be matching those fields mentioned in the docs as well, exactly like they're defined in the .winmd too. Basically I'm saying, having helpful additions (including constructors with validation) is completely fine. But we should not be opinionated in removing projected surface areas from arbitrary WinRT structs.
| { | ||
| private readonly double _unitValue; | ||
| private readonly GridUnitType _unitType; | ||
| public double Value; |
There was a problem hiding this comment.
This was also kept private before to allow to go through the constructor with validation.
There was a problem hiding this comment.
Note that e.g. for this, the docs do mention them publicly (here) too.
2 participants
Summary
Projects the fields of Windows Runtime struct types as C# fields instead of
{ readonly get; set; }properties, across the projection writer, the manually projected types inWinRT.Runtime, and the custom-mapped XAML types defined by addition files.Motivation
Projecting struct fields as properties turned out to be problematic, and confusing.
The biggest issue is authoring.
cswinrtwinmdgen.exemaps the public instance fields of an authoredstructto Windows Runtime struct fields, which is the only shape that can round-trip: an author writing a struct against the projected shape (properties) would silently produce an empty struct in the generated.winmd. Projecting fields as fields makes consumption and authoring symmetrical: the shape you consume is exactly the shape you author.Beyond authoring, Windows Runtime structs are plain data — there is no accessor to run any logic behind, so a property only obscures that, and it prevents callers from taking a reference to a member (e.g. to pass it as a
refargument, or to mutate an element of an array of structs in place). This also matches what C++/WinRT does, and what CsWinRT 2.x did for generated struct projections.Changes
Projection writer
src/WinRT.Projection.Writer/Builders/ProjectionFileBuilder.cs:WriteStructnow emits a public field per Windows Runtime struct field, declared before the constructor so declaration order (which drives the sequential layout of the struct) mirrors the metadata order.src/WinRT.Projection.Writer/Factories/AbiStructFactory.cs,src/WinRT.Projection.Writer/Factories/StructEnumMarshallerFactory.cs: refresh comments that justified the mapped-struct special cases by claiming their projected public fields don't match the WinMD field layout, which is no longer accurate.Manually projected types
src/WinRT.Runtime2/Windows.Foundation/Point.cs,Rect.cs,Size.cs,src/WinRT.Runtime2/InteropServices/Events/EventRegistrationToken.cs:X,Y,Width,HeightandValuebecome public fields. The negative-value validation that lived in theRect/Sizesetters moves into the constructors that document it (which is also what CsWinRT 2.x did). Managed-only members (Left,Top,Right,Bottom,IsEmpty,Empty, ...) are unchanged.Custom-mapped XAML types (addition files, both the
Microsoft.UI.XamlandWindows.UI.Xamlcopies)…/Microsoft.UI.Xaml.CornerRadius.cs:TopLeft,TopRight,BottomRight,BottomLeft…/Microsoft.UI.Xaml.GridLength.cs:Value,GridUnitType…/Microsoft.UI.Xaml.Duration.cs:TimeSpan,Type…/Microsoft.UI.Xaml.Media.Animation.KeyTime.cs:TimeSpan…/Microsoft.UI.Xaml.Media.Animation.RepeatBehavior.cs:Count,Duration,Type…/Microsoft.UI.Xaml.Media.Media3D.Matrix3D.cs:M11-M34,OffsetX,OffsetY,OffsetZ,M44These types are defined in full by their addition files, so they kept the property-over-private-backing-field shape inherited from the UWP/WPF managed projections. Leaving them alone would have made them inconsistent with their own siblings, since
Thickness,Matrix,GeneratorPositionandColorare generated types in CsWinRT 3.0 and therefore now expose fields. Field declaration order (and therefore the sequential layout each of these blittable structs marshals with) is unchanged, and validation that lived in the removed setters is preserved where the constructors already performed it.GridLengthandKeyTimeare no longerreadonly struct-s, since Windows Runtime structs are mutable data. Members that are not Windows Runtime struct fields are left untouched (GridLength.IsAbsolute/IsAuto/IsStar/Auto,Duration.HasTimeSpan/Automatic/Forever,RepeatBehavior.HasCount/HasDuration/Forever,Matrix3D.Identity/IsIdentity/HasInverse, and all operators, conversions and formatting helpers).Tests and docs
src/Tests/ProjectionWriterTest/Test_ProjectedStructs.cs: new tests asserting that projected struct fields are emitted as C# fields, and that no projected struct member is emitted as an auto-property, in both projection modes.docs/cswinrt3.0-spec.md: documents the projection rule, and renames the neighbouringPoint/Rect/Sizesection (which was already about their fields rather than properties).docs/event-infrastructure.md: refreshes theEventRegistrationTokensnippet.Validation
ProjectionWriterTest(16),WinMDGeneratorTest(17) andSourceGenerator2Test(129) all pass.Windows,Windows.UI.XamlandWinAppSDKprojections were regenerated end-to-end and compile cleanly, which covers the emitted structs, their marshallers, and all of the addition files.