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

Add AOT Friendly Attributes for BindingList<T> Compatibility #987

Merged

Conversation

dwcullop
Copy link
Member

@dwcullop dwcullop commented Feb 28, 2025

Description

  1. Created a Polyfill for the DynamicallyAccessedMembers attribute / enum which isn't available until Net5.

  2. Updated any class using BindingList to include the DynamicallyAccessedMembers attribute so that the type have the same attributes as BindingList.

  3. Recursively walked up the code hierarchy to apply the attribute all the way up.

All of the warnings mentioned in #983 were related to BindingList<T>. Any generic code that uses that class directly (or indirectly) needs to have the same AOT attributes so that the trimmer will be sure to not trim needed type information, so this should resolve all of those warnings.

Bonus Fixes

Converted two files to use file-scoped namespaces.

Created a Polyfill for the `DynamicallyAccessedMembers` attribute / enum which isn't available until Net5.

Updated any class using `BindingList` to include the `DynamicallyAccessedMembers` attribute so that the type have the same attributes as `BindingList`.

Recursively walked up the code hierarchy to apply the attribute all the way up.
@dwcullop dwcullop requested a review from Copilot February 28, 2025 17:30
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR introduces AOT friendliness enhancements by adding a polyfill for the DynamicallyAccessedMembers attribute and applying it to various generic types that work with BindingList, while also converting several files to use file-scoped namespaces.

  • Introduced a polyfill for DynamicallyAccessedMembersAttribute to support pre‑Net5 targets.
  • Updated BindingList-related classes and extension methods to annotate generic parameters for AOT preservation.
  • Converted multiple files to file‑scoped namespaces for consistency.

Reviewed Changes

File Description
src/DynamicData/Polyfills/DynamicallyAccessedMembersAttribute.cs New polyfill implementation for the DynamicallyAccessedMembers attribute and associated enum.
src/DynamicData/Binding/BindingListAdaptor.cs Updated to use file‑scoped namespace and applied the DynamicallyAccessedMembers attribute on the generic parameter.
src/DynamicData/Binding/SortedBindingListAdaptor.cs Applied the DynamicallyAccessedMembers attribute on the generic parameter and updated namespace declaration.
src/DynamicData/Cache/ObservableCacheEx.SortAndBind.cs Annotated generic methods with the DynamicallyAccessedMembers attribute and updated using directives.
src/DynamicData/Cache/ObservableCacheEx.cs Updated extension methods to include attribute annotation on generic parameters.
src/DynamicData/List/ObservableListEx.cs Modified to use file‑scoped namespace and applied the attribute on generic method parameters.
src/DynamicData/Binding/BindingListEventsSuspender.cs Added the attribute on the generic parameter and updated using directives.
src/DynamicData/Binding/BindPaged.cs Applied the DynamicallyAccessedMembers attribute on the generic parameter and updated namespace usage.
src/DynamicData/Binding/BindVirtualized.cs Updated class declaration to include the attribute on the generic parameter and switched to file‑scoped namespace.
src/DynamicData/Binding/SortAndBind.cs Annotated the generic parameter with DynamicallyAccessedMembers and updated using directives.

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

@dwcullop dwcullop changed the title AOT Friendliness Enhancements Add AOT Friendly Attributes for BindingList<T> Compatibility Feb 28, 2025
@dwcullop dwcullop enabled auto-merge (squash) February 28, 2025 18:01
Corrected the spelling of "similiar" to "similar" in the XML documentation comments throughout the `ObservableCacheEx` class. This change improves the clarity and professionalism of the documentation without affecting the code's functionality.
Copy link
Collaborator

@JakenVeina JakenVeina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like I mentioned, I don't really have the context for whether this is functionally correct or appropriatte, but I don't see anything in the code that concerns me.

I will say that diffs on these files in GitHub are abysmal, I believe as a result of all the whitespace changes. Did something change recently with GitHub? Do we have any amount of configurability for how diffs are presented? If not, we maybe need to make a point to keep whitespace changes separate, for reviewability.

@dwcullop dwcullop merged commit 2dafcf1 into reactivemarbles:main Mar 2, 2025
1 check passed
@dwcullop
Copy link
Member Author

dwcullop commented Mar 5, 2025

Like I mentioned, I don't really have the context for whether this is functionally correct or appropriatte, but I don't see anything in the code that concerns me.

Those are the attributes that have gotten rid of AOT warnings for me in other projects. After the next release, I can confirm that they are indeed resolved.

@RolandPheasant
Copy link
Collaborator

Those are the attributes that have gotten rid of AOT warnings for me in other projects. After the next release, I can confirm that they are indeed resolved.

I'll release tomorrow

@xackus
Copy link

xackus commented Mar 12, 2025

This also annotates ReadOnlyObservableCollection overloads of Bind and SortAndBind. Is this correct?

@JakenVeina
Copy link
Collaborator

That seems to be correct, yes.

Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants