Skip to content

Conversation

davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Aug 5, 2025

  • Instead of loading the static field type, use metadata directly to pull the needed info if we can't load the target type.
  • Add tests covering various scenarios found by customers in the last year or so
  • Remove existing scheme which could handle directly self referential statics
  • Defer checking RVA static valuetypes for their field type until a later type loader stage
    • Add VMFLAG for HasRVAStaticFields
  • Remove VMFLAG for HasPublicFields, this was found to be unused (its a legacy of an old CAS type loader check which no longer exists)

Fixes #104511

- Instead of loading the static field type, use metadata directly to pull the needed info
- Add tests covering various scenarios found by customers in the last year or so
- Remove existing scheme which could handle directly self referential statics
- Defer checking RVA static valuetypes for their field type until a later type loader stage
  - Add VMFLAG for HasRVAStaticFields
- Remove VMFLAG for HasPublicFields, this was found to be unused (its a legacy of an old CAS type loader check which no longer exists)
…ential type triggers a type load failure

- This should mitigate any performance concerns since we are able to use the existing caching schemes in most cases
@davidwrighton davidwrighton marked this pull request as ready for review August 13, 2025 19:04
@Copilot Copilot AI review requested due to automatic review settings August 13, 2025 19:04
Copy link
Contributor

@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.

Pull Request Overview

This PR adds support for arbitrary self-referential static fields on types by improving type loading and validation mechanisms. Instead of relying on loading the static field type, it uses metadata directly to extract needed information when the target type cannot be loaded, preventing circular dependency issues.

Key changes:

  • Replaces the existing self-referential static field handling scheme with a more robust metadata-based approach
  • Defers RVA static valuetype field validation to a later type loader stage to avoid circular dependencies
  • Removes the unused HasPublicFields VM flag and replaces it with HasRVAStaticFields

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
SelfReferentialStatics.csproj New test project configuration with unsafe blocks enabled
SelfReferentialStatics.cs Comprehensive test cases covering various self-referential static field scenarios
methodtablebuilder.h Removes HasNonPublicFields methods and adds GetCorElementTypeOfTypeDefOrRefForStaticField declaration
methodtablebuilder.cpp Core implementation replacing old self-ref handling with metadata-based approach
class.h Renames VMFLAG_HASNONPUBLICFIELDS to VMFLAG_HASRVASTATICFIELDS and related methods
class.cpp Adds deferred validation for RVA static fields during LoadExactParents

Copy link
Member

@kg kg left a comment

Choose a reason for hiding this comment

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

So much deleted code 🥳
LGTM aside from comments

@davidwrighton davidwrighton merged commit e914183 into dotnet:main Aug 15, 2025
96 of 99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System.TypeLoadException When Interface Type Contains Static Field/Property for Derived Struct Type
4 participants