Skip to content

Conversation

@jkotas
Copy link
Member

@jkotas jkotas commented Sep 19, 2025

Use ReflectedType instead DeclaringType since ReflectedType can be colletible even when DeclaringType is not.

These fixes support typical IsCollectible use case where the property is used to determine whether or how the reflection objects can be cached.

Fix #119697

Use ReflectedType instead DeclaringType since ReflectedType can be colletible even when DeclaringType is not.

These fixes support typical IsCollectible use case where the property is used to determine whether or how the reflection objects can be cached.

Fix dotnet#119697
Copilot AI review requested due to automatic review settings September 19, 2025 01:54
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 fixes the IsCollectible property implementation across various reflection member types to use ReflectedType instead of DeclaringType when determining collectibility. This ensures proper caching behavior in scenarios where the reflected type can be collectible even when the declaring type is not collectible.

Key changes:

  • Updated IsCollectible property implementations in RuntimePropertyInfo, RuntimeFieldInfo, RuntimeEventInfo, and RuntimeConstructorInfo to use ReflectedTypeInternal.IsCollectible
  • Enhanced RuntimeMethodInfo.IsCollectible to check both reflected type and method handle collectibility
  • Refactored test methods from theory-based to fact-based tests that iterate through all members

Reviewed Changes

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

Show a summary per file
File Description
RuntimePropertyInfo.cs Changed IsCollectible to use ReflectedTypeInternal instead of m_declaringType
RuntimeMethodInfo.CoreCLR.cs Added check for ReflectedTypeInternal.IsCollectible before existing method handle check
RuntimeFieldInfo.cs Changed IsCollectible to use ReflectedTypeInternal instead of m_declaringType
RuntimeEventInfo.cs Changed IsCollectible to use ReflectedTypeInternal instead of m_declaringType
RuntimeConstructorInfo.CoreCLR.cs Changed IsCollectible to use ReflectedTypeInternal instead of m_declaringType
IsCollectibleTests.cs Refactored from theory-based tests to comprehensive fact-based tests iterating through all members

@dotnet-policy-service
Copy link
Contributor

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

jkotas added a commit to jkotas/dotnet-api-docs that referenced this pull request Sep 19, 2025
MemberInfo can reference multiple different assemblies (e.g. via generic instantiation or via reflected type). IsCollectible property must take all references into account.

Related to dotnet/runtime#119869
@jkotas
Copy link
Member Author

jkotas commented Sep 19, 2025

Also, I have submitted clarification for the docs repo: dotnet/dotnet-api-docs#11825

@jkotas jkotas merged commit 5aa2a3a into dotnet:main Sep 19, 2025
130 checks passed
@jkotas jkotas deleted the iscollectible branch September 19, 2025 21:24
jkotas added a commit to jkotas/runtime that referenced this pull request Sep 19, 2025
…otnet#119869)

Use ReflectedType instead DeclaringType since ReflectedType can be colletible even when DeclaringType is not.

These fixes support typical IsCollectible use case where the property is used to determine whether or how the reflection objects can be cached.

Fix dotnet#119697
jkotas added a commit that referenced this pull request Sep 20, 2025
…119869) (#119911)

Use ReflectedType instead DeclaringType since ReflectedType can be colletible even when DeclaringType is not.

These fixes support typical IsCollectible use case where the property is used to determine whether or how the reflection objects can be cached.

Fix #119697
xtqqczze pushed a commit to xtqqczze/dotnet-runtime that referenced this pull request Sep 20, 2025
Use ReflectedType instead DeclaringType since ReflectedType can be colletible even when DeclaringType is not.

These fixes support typical IsCollectible use case where the property is used to determine whether or how the reflection objects can be cached.

Fix dotnet#119697
BillWagner pushed a commit to dotnet/dotnet-api-docs that referenced this pull request Sep 22, 2025
MemberInfo can reference multiple different assemblies (e.g. via generic instantiation or via reflected type). IsCollectible property must take all references into account.

Related to dotnet/runtime#119869
@github-actions github-actions bot locked and limited conversation to collaborators Oct 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot unload assembly because a reference is stored in System.Dynamic.Utils.TypeExtensions.s_paramInfoCache._entries

2 participants