Skip to content

Commit aa909d2

Browse files
authored
Fix nested generic constructor (#3501)
***NO_CI***
1 parent 78d91c9 commit aa909d2

2 files changed

Lines changed: 173 additions & 36 deletions

File tree

src/CLR/Core/CLAUDE.md

Lines changed: 25 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -349,40 +349,31 @@ slots:
349349
Passing no instance here would leave every VAR field wrongly typed as
350350
the OBJECT block that `ExtractHeapBlocksForObjects` produced.
351351

352-
### KNOWN LIMITATION — nested generic construction (deferred)
353-
354-
A generic type whose `.cctor` constructs a **different** generic type
355-
parameterized by the holder's own type parameter is not yet supported.
356-
Canonical shape:
357-
358-
```csharp
359-
static class Holder<T> {
360-
static readonly Box<T> Field = new Box<T>(); // Box<!0> — open target
361-
}
362-
```
363-
364-
`Holder<int>`'s `.cctor` emits `newobj Box<!0>::.ctor()` where `!0` is
365-
`Holder`'s `T`. `CEE_NEWOBJ` (`Interpreter.cpp`, §7) only knows how to
366-
prefer the caller's closed TypeSpec when caller and callee share a
367-
**typedef** (the `List<int>`-builds-`List<int>` case). Here the typedefs
368-
differ (`Holder` vs `Box`), so the pushed `Box::.ctor` frame inherits the
369-
**open** `Box<T>` as its `genericType`. Any `new T[]` / VAR use inside that
370-
ctor then fails to resolve `T`, the `.cctor` aborts before its `stsfld`,
371-
and the per-instantiation field stays null. Symptom: `NullReferenceException`
372-
on first use of the static field, even though the field slot was allocated.
373-
374-
The demand gate (§10) and the bare-VAR storage fix above are **not** the
375-
problem — the `.cctor` is scheduled correctly; it dies inside its own body.
376-
377-
A proper fix must resolve the callee's open type argument (`Box<!0>`) to the
378-
concrete closed form (`Box<int>`) via the caller's closed `genericType`, and
379-
flow that closed instantiation into **both** the `NewObject` object tag and
380-
the pushed ctor frame's `genericType` (so single-VAR cases can also lean on
381-
the `arrayElementType` chain in §5/§8). You cannot rely on a matching closed
382-
TypeSpec row already existing in metadata — it only exists when some other
383-
non-generic site happens to use the same closed instantiation. Repro:
384-
`Generic_StaticField_GenericHolder` in `NFUnitTestClasses/UnitTestGenericStaticTests.cs`
385-
(currently commented out with a pointer here).
352+
### Nested generic construction
353+
354+
When a generic `.cctor` constructs a **different** generic type parameterized
355+
by the holder's own type parameter (`Holder<T>` building `Box<T>`), the
356+
`newobj Box<!0>::.ctor()` leaves the callee's owner TypeSpec **open** because
357+
caller and callee have different typedefs — the §7 caller-preference does not
358+
fire. The pushed ctor frame inherits the open `Box<T>`, so `new T[]` inside
359+
the ctor can't resolve `T`; the `.cctor` aborts before `stsfld` and the
360+
static field stays null.
361+
362+
Resolution is in `CEE_NEWOBJ` (after `GetDeclaringType`): parse the open
363+
owner TypeSpec, resolve each argument against the caller's closed TypeSpec
364+
via `GetGenericParam`, then either find a matching closed TypeSpec row in
365+
metadata or fall back to `arrayElementType` for single-VAR cases.
366+
367+
**Invariant: never set both `genericType` and `arrayElementType`** on the
368+
same ctor frame — doing so corrupts `newarr`'s element-type resolution
369+
(native `AccessViolation`). The `arrayElementType` fallback is used **only**
370+
when no closed TypeSpec row exists.
371+
372+
**Why not widen the `ResolveToken` gate:** `ResolveToken` has a closed-row
373+
search for MVAR / `arrayElementType` cases, gated so it skips pure-VAR
374+
callers. Widening that gate made the search fire for every generic
375+
`newobj`/`call`, crashing during startup `.cctor` crawling. The fix is
376+
scoped inside `CEE_NEWOBJ` behind a strict nested-case guard.
386377

387378
---
388379

src/CLR/Core/Interpreter.cpp

Lines changed: 148 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3067,6 +3067,147 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg)
30673067
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
30683068
}
30693069

3070+
// Nested generic construction (Holder<T> builds Box<!0>). See CLAUDE.md §9.
3071+
const CLR_RT_TypeSpec_Index *nestedGenericTag = nullptr;
3072+
3073+
if (cls.target->genericParamCount > 0 && stack->m_call.genericType != nullptr &&
3074+
NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType) && calleeInst.genericType != nullptr &&
3075+
NANOCLR_INDEX_IS_VALID(*calleeInst.genericType))
3076+
{
3077+
CLR_RT_TypeSpec_Instance callerTs{};
3078+
CLR_RT_TypeSpec_Instance calleeOwnerTs{};
3079+
3080+
if (callerTs.InitializeFromIndex(*stack->m_call.genericType) &&
3081+
calleeOwnerTs.InitializeFromIndex(*calleeInst.genericType) &&
3082+
NANOCLR_INDEX_IS_VALID(calleeOwnerTs.genericTypeDef) &&
3083+
calleeOwnerTs.genericTypeDef.data == cls.data && callerTs.genericTypeDef.data != cls.data &&
3084+
callerTs.IsClosedGenericType() && !calleeOwnerTs.IsClosedGenericType())
3085+
{
3086+
CLR_RT_SignatureParser ownerParser{};
3087+
ownerParser.Initialize_TypeSpec(calleeOwnerTs);
3088+
3089+
CLR_RT_SignatureParser::Element ownerElem{};
3090+
if (SUCCEEDED(ownerParser.Advance(ownerElem)) &&
3091+
ownerElem.DataType == DATATYPE_GENERICINST && SUCCEEDED(ownerParser.Advance(ownerElem)))
3092+
{
3093+
int argCount = ownerElem.GenParamCount;
3094+
CLR_RT_TypeDef_Index resolvedArgs[8];
3095+
bool allResolved = (argCount > 0 && argCount <= 8);
3096+
3097+
for (int a = 0; a < argCount && allResolved; a++)
3098+
{
3099+
int targetAvail = ownerParser.Available() - 1;
3100+
3101+
CLR_RT_SignatureParser::Element argElem{};
3102+
if (FAILED(ownerParser.Advance(argElem)))
3103+
{
3104+
allResolved = false;
3105+
break;
3106+
}
3107+
3108+
if (argElem.Levels > 0 || argElem.DataType == DATATYPE_GENERICINST)
3109+
{
3110+
allResolved = false;
3111+
}
3112+
else if (argElem.DataType == DATATYPE_VAR)
3113+
{
3114+
CLR_RT_SignatureParser::Element paramElem{};
3115+
if (callerTs.GetGenericParam(argElem.GenericParamPosition, paramElem) &&
3116+
NANOCLR_INDEX_IS_VALID(paramElem.Class))
3117+
{
3118+
resolvedArgs[a] = paramElem.Class;
3119+
}
3120+
else
3121+
{
3122+
allResolved = false;
3123+
}
3124+
}
3125+
else if (NANOCLR_INDEX_IS_VALID(argElem.Class))
3126+
{
3127+
resolvedArgs[a] = argElem.Class;
3128+
}
3129+
else
3130+
{
3131+
allResolved = false;
3132+
}
3133+
3134+
while (ownerParser.Available() > targetAvail)
3135+
{
3136+
CLR_RT_SignatureParser::Element drained{};
3137+
if (FAILED(ownerParser.Advance(drained)))
3138+
{
3139+
allResolved = false;
3140+
break;
3141+
}
3142+
}
3143+
}
3144+
3145+
if (allResolved)
3146+
{
3147+
const CLR_RT_TypeSpec_Index *closedMatch = nullptr;
3148+
3149+
for (size_t ai = 0;
3150+
ai < g_CLR_RT_TypeSystem.m_assembliesMax && closedMatch == nullptr;
3151+
ai++)
3152+
{
3153+
CLR_RT_Assembly *pASSM = g_CLR_RT_TypeSystem.m_assemblies[ai];
3154+
if (pASSM == nullptr)
3155+
{
3156+
continue;
3157+
}
3158+
3159+
for (int tsIdx = 0; tsIdx < pASSM->tablesSize[TBL_TypeSpec]; tsIdx++)
3160+
{
3161+
const CLR_RT_TypeSpec_Index *candidateIdx =
3162+
&pASSM->crossReferenceTypeSpec[tsIdx].genericType;
3163+
3164+
if (!NANOCLR_INDEX_IS_VALID(*candidateIdx))
3165+
{
3166+
continue;
3167+
}
3168+
3169+
CLR_RT_TypeSpec_Instance candidateInst{};
3170+
if (!candidateInst.InitializeFromIndex(*candidateIdx) ||
3171+
candidateInst.genericTypeDef.data != cls.data ||
3172+
!candidateInst.IsClosedGenericType())
3173+
{
3174+
continue;
3175+
}
3176+
3177+
bool argsMatch = true;
3178+
for (int a = 0; a < argCount && argsMatch; a++)
3179+
{
3180+
CLR_RT_SignatureParser::Element candidateArg{};
3181+
if (!candidateInst.GetGenericParam(a, candidateArg) ||
3182+
candidateArg.Class.data != resolvedArgs[a].data)
3183+
{
3184+
argsMatch = false;
3185+
}
3186+
}
3187+
3188+
if (argsMatch)
3189+
{
3190+
closedMatch = candidateIdx;
3191+
break;
3192+
}
3193+
}
3194+
}
3195+
3196+
if (closedMatch != nullptr)
3197+
{
3198+
calleeInst.genericType = closedMatch;
3199+
nestedGenericTag = closedMatch;
3200+
}
3201+
else if (argCount == 1)
3202+
{
3203+
// Never set alongside genericType above — see CLAUDE.md §9.
3204+
calleeInst.arrayElementType = resolvedArgs[0];
3205+
}
3206+
}
3207+
}
3208+
}
3209+
}
3210+
30703211
{
30713212
const CLR_RT_TypeSpec_Index *tsForCctor = nullptr;
30723213

@@ -3172,11 +3313,16 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg)
31723313
top->SetObjectReference(nullptr);
31733314

31743315
// NEWOBJ: prefer the caller's closed TypeSpec over the callee's (open)
3175-
// MethodRef TypeSpec. See CLAUDE.md "NEWOBJ on generic types".
3316+
// MethodRef TypeSpec. See CLAUDE.md §7, §9.
31763317
const CLR_RT_TypeSpec_Index *genericTypeForContext = nullptr;
31773318

3319+
if (nestedGenericTag != nullptr)
3320+
{
3321+
genericTypeForContext = nestedGenericTag;
3322+
}
31783323
// Prefer the caller's generic type if available and valid
3179-
if (stack->m_call.genericType != nullptr && NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType))
3324+
else if (
3325+
stack->m_call.genericType != nullptr && NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType))
31803326
{
31813327
genericTypeForContext = stack->m_call.genericType;
31823328
}

0 commit comments

Comments
 (0)