Add MethodDef instance parameter to MethodDef_Instance::InitializeFromIndex()

[josesimoes]

Description

• Now it has enough context to be able to resolve MVARs.
• Adjust callers accordingly.

Motivation and Context

• Reported when working in implementation of Linq library.
• There was no unit test or code exercising the execution path the required resolution of a MVAR inside a MethodSpec.
• Related with Support for generics <T> Home#782.

How Has This Been Tested?

• Running Add more unit tests for generics collections System.Collections#165.
• [tested against Merge generics support in mscorlib CoreLibrary#245]

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

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes enhance the generic method initialization path in the CLR runtime by adding caller context support. The InitializeFromIndex method signature is updated to accept an optional caller parameter, enabling improved resolution of method-generic parameters through the caller's method specification and generic type context.

Changes

Cohort / File(s)	Summary
Type System Method Signature src/CLR/Include/nanoCLR_Runtime.h, src/CLR/Core/TypeSystem.cpp	Updated CLR_RT_MethodDef_Instance::InitializeFromIndex to accept optional caller parameter for method-generic parameter resolution via caller's methodSpec and closed generic type context; extended resolution logic handles DATATYPE_MVAR and DATATYPE_VAR parameter resolution.
Interpreter Integration src/CLR/Core/Interpreter.cpp	Updated virtual dispatch generic method initialization to pass &stack->m_call as caller context to InitializeFromIndex; minor formatting consolidations of multiline NANOCLR_CHECK_HRESULT calls.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: adding a MethodDef instance parameter to InitializeFromIndex(), which aligns with the core modification across all three files.
Description check	✅ Passed	The pull request description clearly relates to the changeset, explaining the motivation (MVAR resolution for Linq library implementation) and testing approach (System.Collections#165).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

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: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/CLR/Core/Interpreter.cpp`:
- Around line 2420-2423: The call to calleeInst.InitializeFromIndex(calleeReal,
*calleeInst.genericType, &stack->m_call) is fallible but its HRESULT is ignored;
update the dispatch code to capture and check the return value using
NANOCLR_CHECK_HRESULT (or NANOCLR_SET_AND_LEAVE on failure) so the function
bails out on error instead of continuing with an invalid calleeInst state;
locate the call site referencing InitializeFromIndex, calleeInst, calleeReal and
stack->m_call and wrap the call with the appropriate NANOCLR error-check macro
to propagate/handle failures.

In `@src/CLR/Core/TypeSystem.cpp`:
- Around line 1832-1870: The resolved generic argument handling in the MVAR
branch must detect and normalize constructed generic instances: after calls to
msInst.GetGenericArgument (and callerTs.GetGenericParam) check if the returned
CLR_RT_SignatureParser::Element has DataType == DATATYPE_GENERICINST and, if so,
replace it with its underlying generic definition token (or otherwise fail-fast)
before assigning ownerTypeIdx; ensure this logic is applied both where
argElement.Class and paramElement.Class are used (including the other block
around lines 1885–1889) so ownerTypeIdx is never set from an invalid Class field
of a GENERICINST element.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: ea4b26a0-2eb0-4f51-8ad6-6ea11b1227a7

📥 Commits

Reviewing files that changed from the base of the PR and between f977c0f and fec7f96.

📒 Files selected for processing (3)

• src/CLR/Core/Interpreter.cpp
• src/CLR/Core/TypeSystem.cpp
• src/CLR/Include/nanoCLR_Runtime.h