Fix #2392: consistent package order for <Program> TypeDef and MethodDef row planning

[DavidObando]

Summary

Fixes #2392: multi-file top-level-program members (<Main>$, hoisted top-level local functions, and non-capturing top-level lambdas) emitted into the wrong package's <Program> holder whenever the package-less ("Default") top-level-statement (TLS) file was not the first syntax tree the binder saw.

Root cause

ReflectionMetadataEmitter's "Phase A" TypeDef loop always emits the entry-point/globals-host package's <Program> TypeDef first in the TypeDef table — issue #191 requires this so the entry package's FieldDef range (top-level var/let/const globals) stays monotone relative to every other package's (empty) field range.

MethodDef row planning, however, walked packages in raw first-seen-syntax-tree order (packageByTree's first-occurrence order, itself a function of build-tool/file enumeration order, not package identity):

// Plan method rows for packages (per-package ctor + functions + entry).
var packageCtorRows = new Dictionary<PackageSymbol, int>();
var nextRow = firstPackageCtorRow;
foreach (var pkg in packages)          // <-- raw first-seen order
{
    packageCtorRows[pkg] = nextRow++;
    ...
}

ECMA-335 §II.22.37 requires TypeDef.MethodList to be non-decreasing down the TypeDef table — the CLR (and ilverify/decompilers) derive each TypeDef's owned method range from consecutive MethodList values, not an explicit foreign key. Whenever the entry-point package was not already packages[0], forcing its <Program> TypeDef to the front of the table while its method rows were planned last (or in the middle) produced a non-monotone MethodList sequence. The rows that are really the entry-point package's <Main>$ / hoisted func / lambda then silently attribute — via the same range-lookup logic every metadata reader uses — to a sibling package's <Program>, which has no accessibility into the entry-point package's private/internal members. That surfaces as ilverify FieldAccess/MethodAccess failures.

This was discovered migrating the real Oahu.Diagnostics app after #2382 added native top-level-statement translation: Program.cs has no C# namespace (→ package-less "Default"), while the sibling Checks/*.cs files (→ package Oahu.Diagnostics.Checks) and DiagnosticResult.cs/DiagnosticRunner.cs (→ package Oahu.Diagnostics) are bound first, so Default was never packages[0].

Fix

Reorder packages once, right before row planning, so every package-ordered loop below it (row planning, method-body emission, and the TypeDef-emission loop itself) shares one entry-point-first order:

var programEntryPackage = entryPointPackage ?? (packages.IsDefaultOrEmpty ? null : packages[0]);
if (programEntryPackage != null && packages.Length > 0 && packages[0] != programEntryPackage)
{
    var reorderedPackages = ImmutableArray.CreateBuilder<PackageSymbol>(packages.Length);
    reorderedPackages.Add(programEntryPackage);
    foreach (var pkg in packages)
    {
        if (pkg != programEntryPackage)
        {
            reorderedPackages.Add(pkg);
        }
    }

    packages = reorderedPackages.MoveToImmutable();
}

This is a no-op (identical output) whenever the entry-point package already happens to be packages[0] (the common case, and every existing passing scenario), and is also a no-op for library compilations / programs with no top-level statements (entryPointPackage falls back to packages[0] itself).

Audit of other synthesized entry-point-adjacent members

Per the issue's request, I audited every other synthesized type/member that could plausibly share this bug class:

• Closure classes (capturing lambdas) and async/iterator state-machine classes are nested inside their host <Program> via explicit NestedClass metadata rows (Metadata.AddNestedType(nestedHandle, enclosingHandle)), looked up from programTypeDefHandles by dictionary key (PackageSymbol reference), not by row-range. These are immune to the ordering bug — confirmed by reading ReflectionMetadataEmitter.cs lines ~3383-3415.
• Class/struct instance and static methods are always emitted in their own type's contiguous row range (planned before package functions), never sharing a <Program> row range — unaffected.
• Only package-level functions (the functionsByPackage bucket: the entry point, ordinary top-level funcs, extension functions, and non-capturing top-level lambda literals promoted to static <Program> methods) rely on the implicit range-based ownership mechanism this bug breaks. All of them are covered by the fix and the new regression tests.

Tests

Added test/Compiler.Tests/Emit/Issue2392TopLevelProgramPackageHolderEmitTests.cs:

• TwoPackages_EntryPointPackageSecond_HostsEntryPointHoistedFuncAndLambdaOnItsOwnProgram — 2 packages, package-less TLS file (with a global let, a top-level func, and a non-capturing top-level lambda calling it) bound second. Asserts via TypeDefinition.GetMethods() (the same range-based ownership resolution a real CLR consumer uses) that <Main>$/the top-level func land on Default's <Program> and not PackageA's, that PackageA's own func still lands correctly on its own <Program>, plus ilverify and runtime-output checks.
• ThreePackages_EntryPointPackageMiddle_HostsEntryPointOnItsOwnProgram — 3 packages with the package-less TLS file bound in the middle, proving the fix reorders by package identity rather than special-casing "first" or "last".

Both tests fail before the fix (verified by temporarily reverting the emitter change and rebuilding — the 2-package repro produced ilverify CallCtor/StackUnexpected/ThisUninitReturn errors) and pass after it.

Full suite results (rebased on origin/main @ 358e3e20, incorporating #2382/#2386 native top-level-statement translation, #2383, #2384, #2385, #2386, #2387)

Suite	Result
Core.Tests	6298/6298 (1 skip) (pre-rebase baseline; unaffected by this branch's changes)
Compiler.Tests	3110/3110 (3108 baseline + 2 new) (pre-rebase baseline; full re-run in progress post-rebase, see below)
InternalAnalyzers.Tests	7/7 (pre-rebase baseline; unaffected by this branch's changes)
Issue2392 focused tests (post-rebase, --filter FullyQualifiedName~Issue2392)	2/2 pass
Cs2Gs.Tests (post-rebase, serialized via RunConfiguration.DisableParallelization=true)	1423/1423 pass (up from the pre-rebase 1410 baseline; +13 from #2382/#2385's new translator/test additions now merged in)
Compiler.Tests (post-rebase full re-run)	3115/3115 pass (27m14s; = 3108 pre-#2392 baseline + 2 new tests from this PR + 5 new tests from #2385's Issue2385NullableSameCompilationStructGenericArgEmitTests.cs, now merged in — no regressions)

Real Oahu.Diagnostics end-to-end pipeline verification (now unblocked by #2386)

With #2382/#2386 (native top-level-statement translation) merged to main, the full cs2gs migrate pipeline can now translate real Oahu C# source directly, superseding the standalone-gsc-only verification from the original PR description below. Ran directly against the real Oahu repository (read-only; no Oahu files modified):

cs2gs migrate --corpus /Users/davidobando/GitHub/DavidObando/Oahu --app corpus/Oahu.Diagnostics \
  --out <session-output-dir> --config Release

Result:

app                      translate     compile       ilverify      test-parity 
-------------------------------------------------------------------------------
corpus/Oahu.Diagnostics  PASS          PASS          PASS          PASS        

1/1 apps green; run PASSED.

This is the definitive real-world confirmation: translate → compile → ilverify → test-parity all pass end-to-end for Oahu.Diagnostics with this fix + #2386 both applied — the exact scenario #2392 was filed against.

git status --porcelain in the Oahu working tree remained empty (only the two pre-existing untracked ilverify.allow-unsafe waiver files present) throughout this verification.

Real Oahu.Diagnostics corpus verification (original, pre-#2386, kept for history)

cs2gs's native top-level-statement translation (#2382) was not yet on main at the time this PR was opened, so cs2gs migrate could not re-translate Program.cs from this branch. I verified directly against the already-translated .gs corpus captured during #2382's investigation (13 files: Program.gs, DiagnosticResult.gs, DiagnosticRunner.gs, 10 files under Checks/), compiling it standalone with this branch's gsc.dll against the same reference set used by the real dotnet build (Oahu.Data.dll, Oahu.Decrypt.dll, Oahu.Foundation.dll, System.CommandLine.dll, EF Core, etc.):

• Before the fix (reverted locally): ilverify reported 13 errors (2× FieldAccess ×5 offsets + 1× MethodAccess on <lambda10>, and the same on <Main>$) — reproducing the exact 4 fingerprints from gsc: multi-file top-level program members emit into the wrong package holder #2392, landing on whichever sibling package (Oahu.Diagnostics or Oahu.Diagnostics.Checks, depending on file-argument order) happened to be packages[0].
• After the fix: ilverify reports "All Classes and Methods ... Verified." in both the original argument order and a reordered (Checks/* first, Program.gs last) order.

No changes were made to the Oahu repository or any of its tracked/untracked files.

Deferred / follow-up

None identified beyond this fix — the audit above found no other synthesized member class sharing this bug. The full end-to-end cs2gs migrate --app Oahu.Diagnostics pipeline confirmation (previously deferred pending #2382/#2386) is now complete and passing, as shown above.