Determinism: closure type names differ between --parallelcompilation- and --parallelcompilation+

[T-Gro]

Spun off from #19732 / #19810.

Problem

After PR #19810 lands, same-flags determinism (race detector) on Determinism_Release passes — two builds of the same code with identical flags produce byte-identical FSharp.Compiler.Service.dll. That closes the original #19732 ask (non-deterministic #Strings heap).

However, byte equality between --parallelcompilation- and --parallelcompilation+ does not hold for FSharp.Compiler.Service.dll. A seq-vs-par 1-shot diff produces 100–191 differences in compiler-generated method names, all of the shape:

seq: catchHandler@1     par: catchHandler@1-2
seq: contains@1         par: contains@1-16
seq: contains@1-29      par: contains@1-25
seq: func2@1-23         par: func2@1-17
…

Type list, field list, method count are byte-identical between modes. Only the -N suffix counter on closure type names differs.

Root cause

Expr.Lambda uniq values come from newUnique() = Interlocked.Increment(&uniqueCount). Under parallel typecheck (TypeCheckingMode.Graph) and parallel optimizer (OptimizationProcessingMode.Parallel), inlined Lambda copies (TypedTreeOps.Remapping.fs:1829) allocate uniqs in scheduler order — the same source position gets different uniq values across runs of the parallel pipeline.

StableNiceNameGenerator (CompilerGlobalState.fs:61) caches by (basicName, uniq). Different uniqs ⇒ different cache entries ⇒ different -N suffix allocations on the inner NiceNameGenerator bucket counter.

The race surfaces:

• For Lambdas only — top-level Val names already use stable Val.Stamp and are mitigated by the source-position sort in 6f59f9f278.
• For closures of cross-file-inlined helpers (FSharp.Core's catchHandler, contains, func2, …) — most user code is unaffected because (basicName, fileIndex) buckets only contend when multiple consumer files inline the same source.

Why same-flags passes but seq-vs-par doesn't

Same-flags runs the same parallel pipeline twice. On identical hardware with deterministic enough timing (CI build agents), the Interlocked.Increment calls race-resolve identically across both runs, so the suffix assignments line up.

seq-vs-par contrasts a fully serialised pipeline (--parallelcompilation- --test:ParallelOff) against the parallel pipeline (--parallelcompilation+). The two regimes assign uniqs in genuinely different orders, so suffix allocations diverge.

Attempted fixes (did not land)

1. PrimeStableNamesForCodegen Lambda walk — pre-walks every Expr.Lambda / Expr.TyLambda / Expr.Obj in source-deterministic order before parallel emit. Doesn't see Lambdas synthesized at codegen time (state-machine lowering creates new Lambdas inside the parallel emit phase). Removed in 955fe85.

2. Per-consumer-file PerFileClosureNameScope — replaces the global (basicName, uniq) cache with a per-consumer-file scope. Reviewed by three models (Opus 4.8, GPT-5.5, Opus 4.7) who approved the shape and caught 3 BLOCKERS (uniq dedup, no fallback, extra-bindings scope) which were addressed. Implementation built clean but produced duplicate IL type defs during bootstrap (AddTypeDef called twice with same cloName). The root cause of the dup wasn't fully diagnosed — likely a NamedLocalIlxClosureInfoGenerator visit-twice path or similar. Reverted; tracking here for a clean retry.

Possible directions

• (A) Source-position-only cache key. Replace (basicName, uniq) with (basicName, m.FileIndex, m.StartLine, m.StartColumn, occurrenceInConsumerFile). Removes the uniq race entirely. Requires solving the per-consumer-file scoping correctly (which the failed attempt didn't) AND preserving repeat-visit dedup (NamedLocalIlxClosureInfoGenerator at IlxGen.fs:~10495, let-rec fixup at IlxGen.fs:~8745).

• (B) Deterministic newUnique under parallel optimizer. Lock the optimizer's uniq allocation to a deterministic order (e.g., per-file lifecycle with per-file-Lambda-counter). Big optimizer-side change; high blast radius.

• (C) Include uniq-independent identifier in the name. E.g., basicName@line_file<sha256(consumer-file-path):8>. Eliminates collision potential at the cost of longer / less readable closure names and significant baseline churn.

• (D) Pre-emit-phase synchronisation. Lock-protect StableNameGenerator during parallel emit so allocations occur in a deterministic queue order. Adds contention but minimal code change.

How to reproduce locally

./build.sh -c Release --bootstrap
dotnet msbuild src/Compiler/FSharp.Compiler.Service.fsproj /t:Rebuild \
  /p:Configuration=Release /p:TargetFramework=netstandard2.0 \
  /p:AdditionalFscCmdFlags="--parallelcompilation- --test:ParallelOff --nowarn:75" \
  /p:Deterministic=true /p:DebugDeterminism=true \
  /p:Features=debug-determinism /p:ContinuousIntegrationBuild=false
cp artifacts/bin/FSharp.Compiler.Service/Release/netstandard2.0/FSharp.Compiler.Service.dll /tmp/seq.dll

dotnet msbuild src/Compiler/FSharp.Compiler.Service.fsproj /t:Rebuild \
  /p:Configuration=Release /p:TargetFramework=netstandard2.0 \
  /p:AdditionalFscCmdFlags="--parallelcompilation+" \
  /p:Deterministic=true /p:DebugDeterminism=true \
  /p:Features=debug-determinism /p:ContinuousIntegrationBuild=false
cp artifacts/bin/FSharp.Compiler.Service/Release/netstandard2.0/FSharp.Compiler.Service.dll /tmp/par.dll

md5 -q /tmp/seq.dll /tmp/par.dll  # expect: differ today, should match after fix

CI re-enabling the strict gate

When this is fixed, add back the seq-vs-par leg to azure-pipelines-PR.yml Determinism_Release job (it was removed in PR #19810 to unblock the same-flags fix from merging):

- script: .\eng\test-determinism.cmd -configuration Release -mode seq-vs-par
  displayName: Determinism tests (1-shot diff — sequential vs parallel)