Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 14, 2025

CompilerTypeSystemContext.ValueTypeMethods.cs originally injected __GetFieldHelper only into value types but was later extended to also handle attribute types. The naming remained ValueType-specific, creating confusion.

Renamed classes:

  • ValueTypeMethodHashtableGetFieldMethodHashtable
  • ValueTypeGetFieldHelperMethodOverrideGetFieldHelperMethodOverride

Renamed files:

  • CompilerTypeSystemContext.ValueTypeMethods.csCompilerTypeSystemContext.GetFieldMethodOverrides.cs
  • ValueTypeGetFieldHelperMethodOverride.csGetFieldHelperMethodOverride.cs
  • ValueTypeGetFieldHelperMethodOverride.Sorting.csGetFieldHelperMethodOverride.Sorting.cs

Updated references:

  • Project file compile includes
  • Usage sites in EETypeNode.cs, NoMetadataBlockingPolicy.cs, ObjectDataInterner.cs
Original prompt

CompilerTypeSystemContext.ValueTypeMethods.cs is an old file in the repo that originally only handled injecting a __GetField method override into types that derive from ValueType. It was later repurposed to also inject the __GetField method into types that derive from Attribute. The identifier names such as ValueTypeMethodHashtable or ValueTypeGetFieldHelperMethodOverride are now confusing because they're used for both attributes and value types. Rename these to more appropriate names such as GetFieldMethodHashtable and GetFieldHelperMethodOverride. Make sure to also rename the files. Open a Github pull request with this change.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Renamed ValueTypeMethodHashtable to GetFieldMethodHashtable
- Renamed _valueTypeMethodHashtable to _getFieldMethodHashtable
- Renamed ValueTypeGetFieldHelperMethodOverride to GetFieldHelperMethodOverride
- Renamed files:
  - CompilerTypeSystemContext.ValueTypeMethods.cs → CompilerTypeSystemContext.GetFieldMethodOverrides.cs
  - ValueTypeGetFieldHelperMethodOverride.cs → GetFieldHelperMethodOverride.cs
  - ValueTypeGetFieldHelperMethodOverride.Sorting.cs → GetFieldHelperMethodOverride.Sorting.cs
- Updated project file references
- Updated all usages across the codebase

Co-authored-by: MichalStrehovsky <[email protected]>
Copilot AI changed the title [WIP] Rename field method identifiers for clarity Rename ValueType-specific identifiers to reflect dual usage for ValueType and Attribute types Nov 14, 2025
Copilot finished work on behalf of MichalStrehovsky November 14, 2025 07:48
@MichalStrehovsky MichalStrehovsky marked this pull request as ready for review November 14, 2025 08:32
Copilot AI review requested due to automatic review settings November 14, 2025 08:32
Copilot finished reviewing on behalf of MichalStrehovsky November 14, 2025 08:34
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 performs a comprehensive refactoring to rename ValueType-specific identifiers that are now used for both ValueType and Attribute types. The renaming improves code clarity by reflecting the actual dual usage of these methods and classes.

  • Renamed classes from ValueType-specific names to more generic GetField-based names
  • Renamed source files to match the new class names
  • Updated all usage sites and project file references

Reviewed Changes

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

Show a summary per file
File Description
ILCompiler.Compiler.csproj Updated compile includes for renamed source files
ObjectDataInterner.cs Updated type reference from ValueTypeGetFieldHelperMethodOverride to GetFieldHelperMethodOverride
NoMetadataBlockingPolicy.cs Updated reference to GetFieldHelperMethodOverride.MetadataName
EETypeNode.cs Updated reference to GetFieldHelperMethodOverride.MetadataName in vtable emission logic
CompilerTypeSystemContext.GetFieldMethodOverrides.cs Renamed inner class and field from ValueType-specific names to generic names
GetFieldHelperMethodOverride.cs Renamed class and updated documentation to reflect dual usage
GetFieldHelperMethodOverride.Sorting.cs Updated partial class declaration and type cast

@MichalStrehovsky
Copy link
Member

/ba-g timeout in unrelated leg

@MichalStrehovsky MichalStrehovsky merged commit 2678e21 into main Nov 14, 2025
95 of 102 checks passed
@MichalStrehovsky MichalStrehovsky deleted the copilot/rename-getfield-methods branch November 14, 2025 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants