Fix support generic struct arrays with generic TypeSpecs#3340
Conversation
- ResolveToken for a TypeDef instance now resolves generic TypeSpec tokens to their open generic TypeDef, enabling newarr, ldelem.any and stelem.any to succeed on generic struct element types. - InitializeReference now properly handles a VAR field without generic instance context, falling back to Object instead of faiing, mirroring the existing MVAR behaviour.
📝 WalkthroughWalkthroughThe changes update generic type instantiation handling in the CLR. They modify reflection element type selection for array creation, add lenient handling for unresolved generic parameters during field initialization, and simplify type token resolution logic for generic instantiations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ 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. Comment |
Automated fixes for code style.
…687af-89e1-4a08-b12c-d539e7b510c9 Code style fixes for nanoframework/nf-interpreter PR#3340
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/Execution.cpp`:
- Around line 2118-2131: The current fallback that sets VAR to DATATYPE_OBJECT
when genericInstance is null is too broad and incorrectly affects callers like
InitializeReference used during static field initialization; narrow the fallback
so only the open-generic pre-allocation path uses DATATYPE_OBJECT. Specifically,
in the block that checks genericInstance and NANOCLR_INDEX_IS_VALID, detect the
pre-allocation scenario (the same condition used when preparing array element
structs for open generics) and only in that case set dt = DATATYPE_OBJECT;
otherwise return/propagate the unresolved/generic error (i.e., preserve failure)
so callers such as InitializeReference (static field init) continue to handle
value-type generic parameters correctly; keep the call to
ResolveGenericTypeParameter and the goto process_datatype for the
normal/closed-generic path.
🪄 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: 3619426b-7257-4cdd-9003-be399d7a40b1
📒 Files selected for processing (3)
src/CLR/Core/CLR_RT_HeapBlock_Array.cppsrc/CLR/Core/Execution.cppsrc/CLR/Core/TypeSystem.cpp
- Add allowUnresolvedVarFallback parameter to InitializeReference. - Now using this when resolving VAR. - Update callers, noticably NewObject as this is the only path reached from ClearElements → NewObjectFromIndex during open-generic array pre-allocation.
Description
Motivation and Context
Pair<TKey,TValue>inside a generic method emits a generic TypeSpec token. The runtime had no handling for this token class in ResolveToken, causing CLR_E_WRONG_TYPE for newarr, ldelem.any and stelem.any on generic struct arrays.Even after resolving the element type, pre-allocating struct slots in the new array calls InitializeReference on each field. For open generic structs the field types are unresolved VAR parameters; without a closed generic context at array-creation time the call unconditionally returned CLR_E_FAIL. Fields are written correctly by subsequent stfld instructions, so initialising them as objects (which are null) is safe.
How Has This Been Tested?
Screenshots
Types of changes
Checklist