Skip to content

Remove some unnecessary EventSource suppressions#125258

Merged
agocke merged 1 commit intodotnet:mainfrom
agocke:fix-eventsource-annotations
Mar 6, 2026
Merged

Remove some unnecessary EventSource suppressions#125258
agocke merged 1 commit intodotnet:mainfrom
agocke:fix-eventsource-annotations

Conversation

@agocke
Copy link
Member

@agocke agocke commented Mar 6, 2026

Due to the DynamicallyAccessedMembersAttribute on the EventSource type, all members that have DAM/RUC on them produce warnings that need to be suppressed. In NAOT it also causes extra metadata to be preserved.

By moving these members into a nested type, it excludes them from reflection visibility and means we don't have to have suppressions.

Due to the DynamicallyAccessedMembersAttribute on the EventSource type,
all members that have DAM/RUC on them produce warnings that need to
be suppressed. In NAOT it also causes extra metadata to be preserved.

By moving these members into a nested type, it excludes them from reflection
visibility and means we don't have to have suppressions.
Copilot AI review requested due to automatic review settings March 6, 2026 07:29
@agocke agocke requested review from hoyosjs and noahfalk March 6, 2026 07:30
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @steveisok, @dotnet/area-system-diagnostics-tracing
See info in area-owners.md if you want to be subscribed.

@agocke
Copy link
Member Author

agocke commented Mar 6, 2026

This diff looks messier than it is -- I just moved the two functions inside a nested class and removed the UnconditionalSuppressMessage attributes.

Copy link
Contributor

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.

Pull request overview

Refactors EventSource manifest/attribute reflection helpers into a private nested type to avoid trim-analysis suppressions triggered by EventSource’s [DynamicallyAccessedMembers] annotation (and reduce preserved metadata for NativeAOT).

Changes:

  • Route GetGuid, GetName, manifest generation, and descriptor initialization through a new EventSourceHelpers nested type.
  • Move CreateManifestAndDescriptors and GetCustomAttributeHelper into the nested helper to remove prior suppressions on those members.

Copy link
Member

@noahfalk noahfalk left a comment

Choose a reason for hiding this comment

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

Looks good to me. The big diff appears to be caused by the method order in the new nested type not matching exactly the original method order.

@agocke
Copy link
Member Author

agocke commented Mar 6, 2026

/ba-g issue is #125245

@agocke agocke merged commit 54bc18d into dotnet:main Mar 6, 2026
149 of 154 checks passed
@agocke agocke deleted the fix-eventsource-annotations branch March 6, 2026 18:48
@github-project-automation github-project-automation bot moved this to Done in AppModel Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants