Skip to content

Conversation

@MichalStrehovsky
Copy link
Member

This functionality of DefaultValueAttribute constructor is disabled by default and warns if it gets enabled. We shouldn't need to keep things on the type passed to the constructor.

Cc @eerhardt - as you suggested on Teams

This functionality of `DefaultValueAttribute` constructor is disabled by default and warns if it gets enabled. We shouldn't need to keep things on the type passed to the constructor.
Copilot AI review requested due to automatic review settings July 16, 2025 10:22
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

This PR removes the DynamicallyAccessedMemberTypes.All annotation from the DefaultValueAttribute constructor that takes a Type and string parameter. The change is motivated by the fact that this functionality is disabled by default and generates warnings when enabled, making the broad type access requirement unnecessary.

Key changes:

  • Removed DynamicallyAccessedMembersAttribute annotation from the constructor parameter
  • Added API compatibility suppressions for the breaking change
  • Added IL linker warning suppressions to maintain existing developer experience

Reviewed Changes

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

File Description
src/libraries/System.Private.CoreLib/src/System/ComponentModel/DefaultValueAttribute.cs Removed DynamicallyAccessedMembersAttribute from Type parameter
src/libraries/System.Runtime/ref/System.Runtime.cs Updated reference assembly to match implementation
src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.xml Added suppressions for API compatibility breaking changes
src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Suppressions.LibraryBuild.xml Added IL2067 suppression for the constructor

@MichalStrehovsky
Copy link
Member Author

/ba-g helix deadletter

@MichalStrehovsky MichalStrehovsky merged commit d4f8b88 into dotnet:main Jul 16, 2025
138 of 147 checks passed
@MichalStrehovsky MichalStrehovsky deleted the defaultvalattr branch July 16, 2025 21:47
@MichalStrehovsky
Copy link
Member Author

@eerhardt do you think we'd need to mark this one as a breaking change too? it's not clear to me how much this actually worked with the feature switch set to the untrimmable value. presumably one got a hard-to-suppress warning in corelib to begin with.

@eerhardt
Copy link
Member

This one doesn't feel critical. But if we are marking a breaking change for others, it feels like we should be consistent.

@MichalStrehovsky MichalStrehovsky added the breaking-change Issue or PR that represents a breaking API or functional change over a previous release. label Jul 17, 2025
@dotnet-policy-service

This comment was marked as resolved.

@dotnet-policy-service dotnet-policy-service bot added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Jul 17, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2025
@jeffhandley jeffhandley removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.ComponentModel breaking-change Issue or PR that represents a breaking API or functional change over a previous release. linkable-framework Issues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants