Skip to content

Fix nested generic constructor #3501

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
josesimoes merged 4 commits into from
Jul 24, 2026
Merged

Fix nested generic constructor #3501

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8680d6f
Fix nested generic constructor
josesimoes Jul 24, 2026
ca318d7
Code style fixes
nfbot Jul 24, 2026
8c6a72e
Merge pull request #138 from nanoframework/nfbot/clang-format-fix/e51…
josesimoes Jul 24, 2026
7bec340
Fix from code review
josesimoes Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 25 additions & 34 deletions src/CLR/Core/CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,40 +349,31 @@ slots:
Passing no instance here would leave every VAR field wrongly typed as
the OBJECT block that `ExtractHeapBlocksForObjects` produced.

### KNOWN LIMITATION — nested generic construction (deferred)

A generic type whose `.cctor` constructs a **different** generic type
parameterized by the holder's own type parameter is not yet supported.
Canonical shape:

```csharp
static class Holder<T> {
static readonly Box<T> Field = new Box<T>(); // Box<!0> — open target
}
```

`Holder<int>`'s `.cctor` emits `newobj Box<!0>::.ctor()` where `!0` is
`Holder`'s `T`. `CEE_NEWOBJ` (`Interpreter.cpp`, §7) only knows how to
prefer the caller's closed TypeSpec when caller and callee share a
**typedef** (the `List<int>`-builds-`List<int>` case). Here the typedefs
differ (`Holder` vs `Box`), so the pushed `Box::.ctor` frame inherits the
**open** `Box<T>` as its `genericType`. Any `new T[]` / VAR use inside that
ctor then fails to resolve `T`, the `.cctor` aborts before its `stsfld`,
and the per-instantiation field stays null. Symptom: `NullReferenceException`
on first use of the static field, even though the field slot was allocated.

The demand gate (§10) and the bare-VAR storage fix above are **not** the
problem — the `.cctor` is scheduled correctly; it dies inside its own body.

A proper fix must resolve the callee's open type argument (`Box<!0>`) to the
concrete closed form (`Box<int>`) via the caller's closed `genericType`, and
flow that closed instantiation into **both** the `NewObject` object tag and
the pushed ctor frame's `genericType` (so single-VAR cases can also lean on
the `arrayElementType` chain in §5/§8). You cannot rely on a matching closed
TypeSpec row already existing in metadata — it only exists when some other
non-generic site happens to use the same closed instantiation. Repro:
`Generic_StaticField_GenericHolder` in `NFUnitTestClasses/UnitTestGenericStaticTests.cs`
(currently commented out with a pointer here).
### Nested generic construction

When a generic `.cctor` constructs a **different** generic type parameterized
by the holder's own type parameter (`Holder<T>` building `Box<T>`), the
`newobj Box<!0>::.ctor()` leaves the callee's owner TypeSpec **open** because
caller and callee have different typedefs — the §7 caller-preference does not
fire. The pushed ctor frame inherits the open `Box<T>`, so `new T[]` inside
the ctor can't resolve `T`; the `.cctor` aborts before `stsfld` and the
static field stays null.

Resolution is in `CEE_NEWOBJ` (after `GetDeclaringType`): parse the open
owner TypeSpec, resolve each argument against the caller's closed TypeSpec
via `GetGenericParam`, then either find a matching closed TypeSpec row in
metadata or fall back to `arrayElementType` for single-VAR cases.

**Invariant: never set both `genericType` and `arrayElementType`** on the
same ctor frame — doing so corrupts `newarr`'s element-type resolution
(native `AccessViolation`). The `arrayElementType` fallback is used **only**
when no closed TypeSpec row exists.

**Why not widen the `ResolveToken` gate:** `ResolveToken` has a closed-row
search for MVAR / `arrayElementType` cases, gated so it skips pure-VAR
callers. Widening that gate made the search fire for every generic
`newobj`/`call`, crashing during startup `.cctor` crawling. The fix is
scoped inside `CEE_NEWOBJ` behind a strict nested-case guard.

---

Expand Down
150 changes: 148 additions & 2 deletions src/CLR/Core/Interpreter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3067,6 +3067,147 @@ HRESULT CLR_RT_Thread::Execute_IL(CLR_RT_StackFrame &stackArg)
NANOCLR_SET_AND_LEAVE(CLR_E_WRONG_TYPE);
}

// Nested generic construction (Holder<T> builds Box<!0>). See CLAUDE.md §9.
const CLR_RT_TypeSpec_Index *nestedGenericTag = nullptr;

if (cls.target->genericParamCount > 0 && stack->m_call.genericType != nullptr &&
NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType) && calleeInst.genericType != nullptr &&
NANOCLR_INDEX_IS_VALID(*calleeInst.genericType))
{
CLR_RT_TypeSpec_Instance callerTs{};
CLR_RT_TypeSpec_Instance calleeOwnerTs{};

if (callerTs.InitializeFromIndex(*stack->m_call.genericType) &&
calleeOwnerTs.InitializeFromIndex(*calleeInst.genericType) &&
NANOCLR_INDEX_IS_VALID(calleeOwnerTs.genericTypeDef) &&
calleeOwnerTs.genericTypeDef.data == cls.data && callerTs.genericTypeDef.data != cls.data &&
callerTs.IsClosedGenericType() && !calleeOwnerTs.IsClosedGenericType())
{
CLR_RT_SignatureParser ownerParser{};
ownerParser.Initialize_TypeSpec(calleeOwnerTs);

CLR_RT_SignatureParser::Element ownerElem{};
if (SUCCEEDED(ownerParser.Advance(ownerElem)) &&
ownerElem.DataType == DATATYPE_GENERICINST && SUCCEEDED(ownerParser.Advance(ownerElem)))
{
int argCount = ownerElem.GenParamCount;
CLR_RT_TypeDef_Index resolvedArgs[8];
bool allResolved = (argCount > 0 && argCount <= 8);

for (int a = 0; a < argCount && allResolved; a++)
{
int targetAvail = ownerParser.Available() - 1;

CLR_RT_SignatureParser::Element argElem{};
if (FAILED(ownerParser.Advance(argElem)))
{
allResolved = false;
break;
}

if (argElem.Levels > 0 || argElem.DataType == DATATYPE_GENERICINST)
{
allResolved = false;
}
else if (argElem.DataType == DATATYPE_VAR)
{
CLR_RT_SignatureParser::Element paramElem{};
if (callerTs.GetGenericParam(argElem.GenericParamPosition, paramElem) &&
NANOCLR_INDEX_IS_VALID(paramElem.Class))
{
resolvedArgs[a] = paramElem.Class;
}
else
{
allResolved = false;
}
}
else if (NANOCLR_INDEX_IS_VALID(argElem.Class))
{
resolvedArgs[a] = argElem.Class;
}
else
{
allResolved = false;
}

while (ownerParser.Available() > targetAvail)
{
CLR_RT_SignatureParser::Element drained{};
if (FAILED(ownerParser.Advance(drained)))
{
allResolved = false;
break;
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (allResolved)
{
const CLR_RT_TypeSpec_Index *closedMatch = nullptr;

for (size_t ai = 0;
ai < g_CLR_RT_TypeSystem.m_assembliesMax && closedMatch == nullptr;
ai++)
{
CLR_RT_Assembly *pASSM = g_CLR_RT_TypeSystem.m_assemblies[ai];
if (pASSM == nullptr)
{
continue;
}

for (int tsIdx = 0; tsIdx < pASSM->tablesSize[TBL_TypeSpec]; tsIdx++)
{
const CLR_RT_TypeSpec_Index *candidateIdx =
&pASSM->crossReferenceTypeSpec[tsIdx].genericType;

if (!NANOCLR_INDEX_IS_VALID(*candidateIdx))
{
continue;
}

CLR_RT_TypeSpec_Instance candidateInst{};
if (!candidateInst.InitializeFromIndex(*candidateIdx) ||
candidateInst.genericTypeDef.data != cls.data ||
!candidateInst.IsClosedGenericType())
{
continue;
}

bool argsMatch = true;
for (int a = 0; a < argCount && argsMatch; a++)
{
CLR_RT_SignatureParser::Element candidateArg{};
if (!candidateInst.GetGenericParam(a, candidateArg) ||
candidateArg.Class.data != resolvedArgs[a].data)
{
argsMatch = false;
}
}

if (argsMatch)
{
closedMatch = candidateIdx;
break;
}
}
}

if (closedMatch != nullptr)
{
calleeInst.genericType = closedMatch;
nestedGenericTag = closedMatch;
}
else if (argCount == 1)
{
// Never set alongside genericType above — see CLAUDE.md §9.
calleeInst.arrayElementType = resolvedArgs[0];
}
}
}
}
}

{
const CLR_RT_TypeSpec_Index *tsForCctor = nullptr;

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

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

if (nestedGenericTag != nullptr)
{
genericTypeForContext = nestedGenericTag;
}
// Prefer the caller's generic type if available and valid
if (stack->m_call.genericType != nullptr && NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType))
else if (
stack->m_call.genericType != nullptr && NANOCLR_INDEX_IS_VALID(*stack->m_call.genericType))
{
genericTypeForContext = stack->m_call.genericType;
}
Expand Down
Loading