Add static fields for generic types

[josesimoes]

Description

• TypeSpec cross reference now stores instances of static fields and field defs.
• Storage of static fields for generic types is global to meet ECMA specs.
• Assembly now has GetGenericStaticField() and GetStaticFieldByFieldDef().
• TypeSpec instance now has a helper API IsClosedGenericType().
• Update handlers for stsfld, ldsfld and ldsflda to deal with static fields of closed generic types.

Motivation and Context

• Adds support for static fields in closed generic types. Up until now the CLR was handling static fields in a simplified manner because static fields are unique to the assembly and type they belong to. Static fields should be allocated and initialized for each closed generic type (e.g., List, List, etc.).

How Has This Been Tested?

• Corelib unit tests and new Sys.Collection unit tests with support for generic types.
• Tested against Merge generics support in mscorlib CoreLibrary#245.
  [build with MDP buildId 57148]

Screenshots

Types of changes

• Improvement (non-breaking change that improves a feature, code or algorithm)
• Bug fix (non-breaking change which fixes an issue with code or algorithm)
• New feature (non-breaking change which adds functionality to code)
• Breaking change (fix or feature that would cause existing functionality to change)
• Config and build (change in the configuration and build system, has no impact on code or features)
• Dev Containers (changes related with Dev Containers, has no impact on code or features)
• Dependencies/declarations (update dependencies or assembly declarations and changes associated, has no impact on code or features)
• Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

• My code follows the code style of this project (only if there are changes in source code).
• My changes require an update to the documentation (there are changes that require the docs website to be updated).
• I have updated the documentation accordingly (the changes require an update on the docs in this repo).
• I have read the CONTRIBUTING document.
• I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

Summary by CodeRabbit

• New Features

  • Adds full support for static fields on closed generic types, ensuring correct per-instantiation storage and access at runtime across all relevant operations.
  • Improves generic-type detection to reliably handle closed generic scenarios across assemblies.

• Bug Fixes

  • Fixes incorrect resolution of static fields on generic types that could lead to wrong values or access failures.
  • Ensures safer initialization and access paths for static fields, reducing null/invalid pointer issues and improving runtime stability.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Implements per-TypeSpec static field storage for closed generic types. Adds allocation, relocation, and access APIs for generic statics, integrates allocation into type-system resolution, and updates interpreter static field instructions to resolve and access per-generic static fields when a generic type context is present, falling back to non-generic statics otherwise.

Changes

Cohort / File(s)	Summary
Interpreter generic static access src/CLR/Core/Interpreter.cpp	Adds conditional resolution for static fields: use per-TypeSpec generic statics via GetGenericStaticField when a valid generic context exists; otherwise use AccessStaticField. Introduces local initialization for field and pointer variables. Applies to multiple IL ops (ldfld/ldsfld/ldsflda/stsfld, etc.).
TypeSystem generic statics support src/CLR/Core/TypeSystem.cpp	Adds per-TypeSpec static field allocation/relocation for closed generics, new IsClosedGenericType on TypeSpec, APIs to resolve/get generic statics, hooks allocation into ResolveAll, and initializes/relocates new cross-reference data. Minor init style tweak in InitializeFromIndex.
Public headers/API additions src/CLR/Include/nanoCLR_Runtime.h	Extends CLR_RT_TypeSpec_CrossReference with genericStaticFields, genericStaticFieldDefs, genericStaticFieldsCount. Adds CLR_RT_Assembly methods: ResolveAllocateGenericTypeStaticFields, GetGenericStaticField, GetStaticFieldByFieldDef. Adds CLR_RT_TypeSpec_Instance::IsClosedGenericType.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor IL as IL Instruction
  participant EE as ExecutionEngine
  participant ASM as CLR_RT_Assembly
  participant TS as TypeSpec (generic ctx)

  IL->>EE: Execute static field op (e.g., ldsfld/stsfld)
  EE->>TS: Check generic type context
  alt Closed generic context
    EE->>ASM: GetGenericStaticField(typeSpecIndex, fieldDefIndex)
    ASM-->>EE: CLR_RT_HeapBlock* (per-TypeSpec static)
  else No/invalid generic context
    EE->>EE: AccessStaticField(field)
    EE-->>EE: CLR_RT_HeapBlock* (non-generic static)
  end
  EE->>IL: Continue with field load/store using returned HeapBlock

Loading

sequenceDiagram
  autonumber
  participant TSYS as CLR_RT_TypeSystem::ResolveAll
  participant ASM as CLR_RT_Assembly
  participant TSX as TypeSpec CrossRefs

  TSYS->>ASM: ResolveAllocateStaticFields()
  TSYS->>ASM: ResolveAllocateGenericTypeStaticFields()
  ASM->>TSX: Enumerate TypeSpecs
  alt TypeSpec is closed generic
    ASM->>TSX: Allocate genericStaticFields[] and genericStaticFieldDefs[]
    ASM-->>TSYS: Map FieldDefs to per-TypeSpec static storage
  else Not closed generic
    ASM-->>TSYS: Skip allocation
  end
  note over ASM: Relocate() updated to relocate generic statics and mapping arrays

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title accurately and concisely describes the main enhancement introduced by this pull request, which is adding support for static fields on generic types.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[josesimoes]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 5

🧹 Nitpick comments (1)

src/CLR/Include/nanoCLR_Runtime.h (1)

2121-2122: Make IsClosedGenericType() const

Method does not mutate state. Mark it const for correctness and clarity.

-    bool IsClosedGenericType();
+    bool IsClosedGenericType() const;

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f187be0 and 26314cb.

📒 Files selected for processing (3)

• src/CLR/Core/Interpreter.cpp (3 hunks)
• src/CLR/Core/TypeSystem.cpp (7 hunks)
• src/CLR/Include/nanoCLR_Runtime.h (3 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)

• GitHub Check: nf-interpreter (Build_Azure_RTOS_targets SL_STK3701A)
• GitHub Check: nf-interpreter (Build_WIN32_nanoCLR)
• GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_P4_UART)
• GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_ETHERNET_KIT_1.2)
• GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_S3_ALL)
• GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_H2_THREAD)
• GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C6_THREAD)
• GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_C3)
• GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_BLE_REV0)
• GitHub Check: nf-interpreter (Build_STM32_targets ST_STM32F769I_DISCOVERY)
• GitHub Check: nf-interpreter (Build_NXP_targets NXP_MIMXRT1060_EVK)
• GitHub Check: nf-interpreter (Build_TI_SimpleLink_targets TI_CC1352R1_LAUNCHXL_915)
• GitHub Check: nf-interpreter (Build_ESP32_targets ESP32_PSRAM_REV0)
• GitHub Check: nf-interpreter (Build_STM32_targets ST_STM32F429I_DISCOVERY)
• GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALX)
• GitHub Check: nf-interpreter (Nightly build) (Build_STM32_targets ORGPAL_PALTHREE)
• GitHub Check: nf-interpreter (Check_Code_Style)
• GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
• GitHub Check: nf-interpreter (Check_Build_Options)

🔇 Additional comments (6)

src/CLR/Include/nanoCLR_Runtime.h (1)

1433-1440: Clarify cctor semantics for generic statics accessors

Do GetGenericStaticField() / GetStaticFieldByFieldDef() ensure the declaring type’s static constructor is executed (per closed TypeSpec) before returning the storage, matching AccessStaticField()? If not, add that or gate here to preserve .NET semantics.

src/CLR/Core/TypeSystem.cpp (5)

707-724: Element init tweak looks fine

Using value-initialized Element and the follow-up logic is consistent with surrounding code.

---

2901-2904: Good: zeroing newly introduced TypeSpec cross-ref fields

Explicitly initializing genericStaticFields/Defs/Count prevents stale pointers after load.

---

5059-5086: Prefer per-TypeSpec storage when available; ensure correct assembly context

The lookup sequence is sound, but it relies on GetGenericStaticField using the TypeSpec-owning assembly (see fix above). After that change, this will correctly prefer per-closed-generic statics and fall back to non-generic static storage.

Also, please verify that InitializeReference with generic context is present and used consistently by all call sites that may initialize generic static storage (ctor paths, relocations). I can help scan/update those if needed.

---

5882-5897: Relocation for generic static storage looks correct

• Relocates heapblocks array with count.
• Relocates indices pointer.
• Guards for nulls.

LGTM.

---

6391-6391: Initialize per-TypeSpec statics in resolve flow (non-AppDomains)

Good integration point. For AppDomains builds, there’s no symmetric allocation. If AppDomains are in scope, confirm whether generic statics should be per-AppDomain and wire allocation/relocation accordingly (e.g., alongside AppDomainAssembly::Resolve_AllocateStaticFields).