Skip to content

Commit 4ce0c0e

Browse files
T-GroCopilot
andcommitted
Round 3 floor: trim chatty docstrings in CGS + collapse mkFileNamingScope helper
Last round-3 micro-compactions (GPT-5.5 + Opus-xhigh consensus): - CompilerGlobalState.fs: trim multi-paragraph rationale on FreshCompilerGeneratedNameInScope, StableNiceNameGenerator's Lazy<_> wrapper, PerFileNamingScope, PerFileClosureNameScope. Public .fsi keeps full docs; .fs gets concise summaries + issue link. Also slightly tighter EmitClosureName bucket-update (single dictionary write). - OptimizeInputs.fs: collapse 5-line rationale + mkFileNamingScope helper into a one-line scopeOf and inline calls. SEQ=PAR FCS.dll byte-identical preserved. EmittedIL 488/488, RIS 789/789, RecordTypes 19/19, sin/abs quotation regression all pass. Three-round compaction now at floor. Per consensus of 5 agents x 3 rounds, remaining structural changes (CodegenFileScope.OrderKey int64 widening, fieldSpecByRange.GetOrAdd Lazy wrap, 4-encodings-of-current-file unification) are Tier-4 follow-ups intentionally deferred. (#19928) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 406de25 commit 4ce0c0e

2 files changed

Lines changed: 30 additions & 59 deletions

File tree

src/Compiler/Driver/OptimizeInputs.fs

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -437,12 +437,10 @@ let ApplyAllOptimizations
437437
if tcConfig.extraOptimizationIterations > 0 then
438438
addPhase "ExtraLoop" extraLoop
439439

440-
// A per-file naming scope is created at this per-file optimization boundary so that
441-
// compiler-generated names from the Detuple and TLR passes are bucketed by the consumer
442-
// file currently being optimized, rather than by the (possibly inlined) source range of
443-
// each value. This keeps those names deterministic under parallel optimization.
444-
// See https://github.com/dotnet/fsharp/issues/19732.
445-
let mkFileNamingScope (file: CheckedImplFile) =
440+
// Each optimizer pass owns a per-file naming scope so its compiler-generated names
441+
// are bucketed by the consumer file (not by the inlined source range), making them
442+
// deterministic under parallel optimization. See #19732.
443+
let scopeOf (file: CheckedImplFile) =
446444
tcGlobals.CompilerGlobalState.Value.NewFileScope(file.QualifiedNameOfFile.Range)
447445

448446
let detuple
@@ -452,8 +450,7 @@ let ApplyAllOptimizations
452450
PrevFile = _prevFile
453451
}: PhaseInputs)
454452
: PhaseRes =
455-
let scope = mkFileNamingScope file
456-
let file = file |> Detuple.DetupleImplFile scope ccu tcGlobals
453+
let file = file |> Detuple.DetupleImplFile (scopeOf file) ccu tcGlobals
457454
file, prevPhase
458455

459456
if tcConfig.doDetuple then
@@ -466,11 +463,9 @@ let ApplyAllOptimizations
466463
PrevFile = _prevFile
467464
}: PhaseInputs)
468465
: PhaseRes =
469-
let scope = mkFileNamingScope file
470-
471466
let file =
472467
file
473-
|> InnerLambdasToTopLevelFuncs.MakeTopLevelRepresentationDecisions scope ccu tcGlobals
468+
|> InnerLambdasToTopLevelFuncs.MakeTopLevelRepresentationDecisions (scopeOf file) ccu tcGlobals
474469

475470
file, prevPhase
476471

src/Compiler/TypedTree/CompilerGlobalState.fs

Lines changed: 24 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,8 @@ type NiceNameGenerator() =
4141

4242
member _.IncrementOnly(name: string, m: range) = increment name m
4343

44-
/// Allocate a fresh compiler-generated name whose uniqueness counter is bucketed by an
45-
/// explicit per-file scope (see PerFileNamingScope) rather than by the file index of 'm'.
46-
/// 'm' is used only for the human-readable start-line marker baked into the generated name,
47-
/// so passing a range that points at inlined source code can no longer make compiler-generated
48-
/// names non-deterministic under parallel optimization. See
49-
/// https://github.com/dotnet/fsharp/issues/19732.
44+
/// Allocate a fresh name bucketed by `scopeFileIndex` (not by `m.FileIndex`),
45+
/// so inlined ranges can't make names non-deterministic. See #19732.
5046
member _.FreshCompilerGeneratedNameInScope (scopeFileIndex: int, name: string, m: range) =
5147
let basicName = GetBasicNameOfPossibleCompilerGeneratedName name
5248
let count = incrementBucket basicName scopeFileIndex
@@ -58,13 +54,10 @@ type NiceNameGenerator() =
5854
///
5955
/// This type may be accessed concurrently, though in practice it is only used from the compilation thread.
6056
/// It is made concurrency-safe since a global instance of the type is allocated in tast.fs.
61-
type StableNiceNameGenerator() =
57+
type StableNiceNameGenerator() =
6258

63-
// The value is wrapped in Lazy<_> so the inner counter-incrementing factory runs exactly once
64-
// per cache key, even when ConcurrentDictionary.GetOrAdd's value-factory is invoked on multiple
65-
// threads under contention. Without the Lazy wrapper, spurious factory invocations would
66-
// increment the counter and produce non-deterministic suffixes. See
67-
// https://github.com/dotnet/fsharp/issues/19732.
59+
// Lazy<_> ensures the inner counter-incrementing factory runs exactly once per key
60+
// even when GetOrAdd's value-factory is invoked concurrently. See #19732.
6861
let niceNames = ConcurrentDictionary<string * int64, Lazy<string>>(max Environment.ProcessorCount 1, 127)
6962
let innerGenerator = NiceNameGenerator()
7063

@@ -76,27 +69,22 @@ type StableNiceNameGenerator() =
7669
lazy innerGenerator.FreshCompilerGeneratedNameOfBasicName(basicName, m))
7770
lazyName.Value
7871

79-
/// A compiler-generated-name allocation scope bound to a single ImplFile being optimized. The
80-
/// constructor is not part of the public signature: a scope can only be obtained from
81-
/// CompilerGlobalState.NewFileScope so a call site can't accidentally bucket names by the wrong
82-
/// (e.g. inlined-source) file and reintroduce the non-determinism fixed by
83-
/// https://github.com/dotnet/fsharp/issues/19732.
72+
/// Per-file scope for optimizer name allocation. The internal constructor forces
73+
/// callers to obtain a scope via `CompilerGlobalState.NewFileScope`, so they can't
74+
/// accidentally bucket names by an inlined-source file. See #19732.
8475
[<Sealed>]
8576
type PerFileNamingScope internal (nng: NiceNameGenerator, fileIndex: int) =
8677

87-
/// Allocate a fresh compiler-generated name within this file's scope. 'm' contributes only the
88-
/// source-location marker in the generated name; the determinism-critical uniqueness bucket is
89-
/// fixed by this scope's file and never by 'm'.
78+
/// Allocate a fresh name within this file's bucket. `m` contributes only the
79+
/// human-readable source-location marker.
9080
member _.Fresh (name: string, m: range) =
9181
nng.FreshCompilerGeneratedNameInScope(fileIndex, name, m)
9282

93-
/// Per-consumer-file closure type-name allocation scope used by IlxGen for cross-file
94-
/// inlined closures only. In-file closures are handled directly through
95-
/// StableNiceNameGenerator (with priming) to keep their names identical to the legacy
96-
/// format. See https://github.com/dotnet/fsharp/issues/19928.
97-
///
98-
/// Cross-file inlined closures get an `F<consumerFileIndex>` marker to disambiguate
99-
/// parallel consumer files inlining the same source range.
83+
/// Per-consumer-file scope for IlxGen closure type names, used only for cross-file
84+
/// inlined closures. In-file closures stay on StableNiceNameGenerator to keep the
85+
/// legacy `basicName@<line>[-N]` format; cross-file ones get an `F<consumerFileIndex>`
86+
/// marker so parallel consumer files inlining the same source range don't collide.
87+
/// See #19928.
10088
[<Sealed>]
10189
type PerFileClosureNameScope(consumerFileIndex: int) =
10290

@@ -108,33 +96,21 @@ type PerFileClosureNameScope(consumerFileIndex: int) =
10896

10997
member _.EmitClosureName(basicName: string, m: range, uniq: int64) =
11098
match byUniq.TryGetValue uniq with
111-
| true, cached ->
112-
cached
99+
| true, cached -> cached
113100
| false, _ ->
114-
// Bucket key omits StartColumn so two Lambdas sharing a line (different columns)
115-
// share one bucket and produce different `-N` suffixes.
116-
let bucketKey =
117-
struct (basicName, m.FileIndex, m.StartLine)
101+
// Bucket key omits StartColumn so two Lambdas sharing a line share one bucket.
102+
let bucketKey = struct (basicName, m.FileIndex, m.StartLine)
118103

119104
let occ =
120105
match buckets.TryGetValue bucketKey with
121-
| true, n ->
122-
buckets[bucketKey] <- n + 1
123-
n
124-
| false, _ ->
125-
buckets[bucketKey] <- 1
126-
0
127-
128-
let lineMarker =
129-
string m.StartLine + "F" + string consumerFileIndex
130-
131-
let suffix =
132-
if occ = 0 then ""
133-
else "-" + string occ
106+
| true, n -> n
107+
| false, _ -> 0
134108

135-
let name =
136-
CompilerGeneratedNameSuffix basicName (lineMarker + suffix)
109+
buckets[bucketKey] <- occ + 1
137110

111+
let lineMarker = string m.StartLine + "F" + string consumerFileIndex
112+
let suffix = if occ = 0 then "" else "-" + string occ
113+
let name = CompilerGeneratedNameSuffix basicName (lineMarker + suffix)
138114
byUniq[uniq] <- name
139115
name
140116

0 commit comments

Comments
 (0)