Skip to content

Add nullness regression test for recursive inline SRTP memoization - #20247

Merged
T-Gro merged 6 commits into
mainfrom
t-gro-fix-memoizen-srtp-regression
Aug 17, 2026
Merged

T-Gro merged 6 commits into
mainfrom
t-gro-fix-memoizen-srtp-regression

Conversation

@T-Gro

@T-Gro T-Gro commented Aug 12, 2026 •

Copy link
Copy Markdown
Member

The recursive inline SRTP resolution fix (FSharpPlus memoizeN) landed in #19785. This adds the one piece that PR didn't carry: a --checknulls+ regression test locking that the domain-order fix stays nullness-neutral — a nullable argument annotation still flows through the recursive specialization and produces the same precise FS3261 dereference warning, with no lost or spurious warning and no internal error.

The function-domain unification order change in PR #15181 (nullness) caused
recursive inline SRTP resolution to be truncated by one currying level: an
inference variable still carrying an unsolved member constraint could be
absorbed by the required domain, merging away the pending recursive trait
resolution. This regressed patterns such as FSharpPlus `memoizeN`.

SolveFunTypeEqn now keeps the SRTP-constrained inference variable as the
unification representative for that specific case, and only when not
MatchingOnly (mirroring SolveTypeEqualsType).

Adds two ComponentTests (the memoizeN repro across currying depths, and a
MatchingOnly overload-resolution guard) and repoints the FSharpPlus
regression matrix at a minimal global.json-bump branch that exercises the
fix end-to-end.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4cfb1c86-0d7e-45e6-9529-3c5f301c0912
@github-actions

github-actions Bot commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

✅ No release notes required

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4cfb1c86-0d7e-45e6-9529-3c5f301c0912
@T-Gro
T-Gro marked this pull request as ready for review August 12, 2026 18:47
@T-Gro
T-Gro requested a review from a team as a code owner August 12, 2026 18:47
@github-actions github-actions Bot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Aug 12, 2026
bartelink added a commit to bartelink/fsharp that referenced this pull request Aug 12, 2026
bartelink added a commit to bartelink/fsharp that referenced this pull request Aug 12, 2026
T-Gro and others added 2 commits August 13, 2026 12:00
Trim the SolveFunTypeEqn comment to the why + bug link + MatchingOnly
invariant, drop the depth-2 memoization control block (depth-3 is the
minimal reproducing case), and count invocations with a mutable int
instead of a ResizeArray of unused labels.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4cfb1c86-0d7e-45e6-9529-3c5f301c0912
Locks that the domain-order fix in SolveFunTypeEqn keeps recursive inline
SRTP memoization nullness-neutral under --checknulls+: a nullable argument
annotation still flows through the recursive specialization and produces the
same precise FS3261 deref warning (no lost/spurious warning, no internal error).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 4cfb1c86-0d7e-45e6-9529-3c5f301c0912
@T-Gro
T-Gro requested a review from abonie August 13, 2026 13:35
T-Gro added 2 commits August 13, 2026 15:39
…tp-regression

# Conflicts:
#	azure-pipelines-PR.yml
…tp-regression

# Conflicts:
#	src/Compiler/Checking/ConstraintSolver.fs
#	tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs
@T-Gro T-Gro changed the title Fix recursive inline SRTP resolution truncated by one currying level Add nullness regression test for recursive inline SRTP memoization Aug 13, 2026
T-Gro added a commit that referenced this pull request Oct 16, 2026
* Avoid leaking a MeterListener per Cache in DEBUG builds (#19995)

* Avoid leaking a MeterListener per Cache in DEBUG builds

In DEBUG builds, every Cache instance created a CacheMetrics.CacheMetricsListener,
which starts a System.Diagnostics.Metrics.MeterListener registered in the
process-global metrics registry. These were never disposed, so they accumulated for
the lifetime of the process. Because every cache hit/miss/add publishes a measurement
to all registered listeners, the per-operation cost grew linearly with the number of
leaked listeners, so workloads that create many caches (for example repeated
ParseAndCheckProject / per-file checks) slowed down steadily.

Track the per-cache totals used by DebugDisplay directly, incrementing a small Stats
object alongside the existing global Meter counters, instead of via a per-cache
MeterListener. No listener is created, so nothing leaks, and DebugDisplay still works.
The now-unused CacheMetrics.Hit/Miss/Add/Update/Eviction/EvictionFail helpers are
replaced by a single recordMetric helper.

* Address review: drop per-cache CacheMetricsListener and cacheId tag

- Remove the CacheMetrics.CacheMetricsListener type. Its only per-cache use
  was the #if DEBUG debugListener each Cache created and never disposed, which
  was the leak this PR set out to fix. (majocha)
- Drop the per-instance cacheId tag (and nextCacheId). Measurements now carry
  only the cache name, shrinking the payload published to any connected
  exporter and removing the per-instance filtering that was cacheId's only
  purpose. (majocha)
- DebugDisplay and the cache tests read the existing name-aggregated stats via
  CacheMetrics.getTotalsByName / getRatioByName, populated by the single
  process-wide ListenToAll listener. No per-cache listener is created and no
  per-operation cost is added in any configuration, so there is no DEBUG-only
  overhead left to gate behind a separate directive. (T-Gro)
- Overload-cache tests enable ListenToAll and snapshot totals before/after to
  stay scoped to their own compilation; FSharpChecker
  .CreateOverloadCacheMetricsListener is removed.

* Update public SurfaceArea baseline after removing CacheMetricsListener

CacheMetricsListener was a public type, so dropping it changes the recorded
public surface. Remove its 10 entries from
FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl; the SurfaceArea test
now passes. Also note the single-listener assumption the cache metric tests
rely on.

* Apply fantomas formatting to Caches.fs

* Document why OverloadCacheTests is not parallelizable (global cache metrics state)

---------

Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com>
(cherry picked from commit 7192705)

* Add ResetCompilerGeneratedNameState to compiler-generated name generators (#20017)

Compiler-generated occurrence names (name@line-N) are allocated from process-wide
counters on CompilerGlobalState that accumulate across compilations. When a warm
checker re-emits the same project in-process, an unchanged closure therefore gets a
different occurrence suffix than the previous emit, so consumers that align generated
names across compilations (Edit-and-Continue delta emission, #19941)
cannot match them.

Add an internal ResetCompilerGeneratedNameState to NiceNameGenerator (clears the
per-(name, file) occurrence counters), StableNiceNameGenerator (clears the cached
stable names and the inner counters), and an aggregate on CompilerGlobalState that
resets all three generators, restoring the fresh-process name layout. Callers must
ensure no compilation is concurrently generating names.

No in-tree caller yet; the consumer is the hot reload emit path in #19941.
Covered by unit tests proving drift without reset, exact replay after reset, and that
the stable-name cache itself is cleared.

(cherry picked from commit 60d315a)

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 3013177 (#20023)

Co-authored-by: Copilot <copilot@github.com>
(cherry picked from commit 9fc230a)

* Tests/source context: support multiple carets (#20077)

(cherry picked from commit 69fca7f)

* Move VS language-service logic tests to FSharp.Compiler.Service.Tests (#20033)

* Move VS language-service logic tests to FSharp.Compiler.Service.Tests

Port completion, quick info, parameter info, go-to-definition, and
diagnostics coverage from the Windows-only VS Salsa suite to the
cross-platform FSharp.Compiler.Service.Tests. The legacy suite keeps only
the tests that genuinely exercise Visual Studio integration.

(cherry picked from commit 2f07589)

* Add Roslyn-format EnC CustomDebugInformation codec and portable PDB method CDI emission (#20018)

* Add Roslyn-format EnC CustomDebugInformation codec and portable PDB method CDI emission

Adds an internal AbstractIL module implementing, byte for byte, the three Portable PDB
CustomDebugInformation blob formats Roslyn persists per method for Edit and Continue
(EnC Local Slot Map, EnC Lambda and Closure Map, EnC State Machine State Map), with
serializers, deserializers, a portable PDB read-back helper, and an occurrence-key
packing helper for deterministic syntax-offset slots.

Plumbs an optional methodCustomDebugInfoRows side channel through the IL binary writer
options into the portable PDB generator so a compilation can attach CDI rows to named
methods. Names that do not identify exactly one method row are dropped. All existing
writer call sites pass an empty map, so emitted PDBs are byte-identical to before.

No in-tree caller populates the map yet; the consumer is the F# hot reload work in
#19941, following the same pattern as #20017 (land isolated, test-covered
infrastructure first, wire the feature later).

Tests: blob round-trips, Roslyn golden-byte encodings, cross-validation against
CDI blobs emitted by a real Roslyn compilation, fail-closed occurrence-key packing
(including an int32-overflow regression where a wrapped negative key previously
escaped the bound check), and end-to-end synthetic PDB emission proving correct
MethodDef parenting, zero rows for an empty map, and no rows for absent or
ambiguous names.

(cherry picked from commit 24d731a)

* Add stable synthesized-name replay infrastructure for hot reload (#20024)

* Extract stable synthesized-name replay layer

Add internal generated-name normalization and synthesized-name map replay support as a standalone slice. The new map state is side-channel based, all new compiler modules remain internal, and CompilerGlobalState preserves the existing no-map counter path while checking an accessor captured once per compiler state.

Route existing IlxGen generated-name allocations through inert helper wrappers, add pure name-map and normalizer tests, add a normal compilation determinism guard over emitted generated names, and document the extracted seams in P5_REPORT.md.

Verification: built FSharp.Compiler.Service, FSharp.Compiler.Service.Tests, FSharp.Compiler.ComponentTests, and FSharpSuite.Tests in Release; ran the migrated service test classes, the component determinism class, FSharpSuite DeterministicTests, and the FCS SurfaceArea class successfully.

* Fix generated-name scope test in stable names slice

* Validate hot reload generated names before classification

* Format hot reload compiler sources

Verified with the repository-wide Fantomas check.

* Retry CI after Linux runner memory exhaustion

* Make synthesized name snapshots deterministic

(cherry picked from commit 2e838bb)

* Fix #19457: lift CE constructs from plain let RHS in computation expressions (#19868)

(cherry picked from commit 4a2749e)

* Fix attribute resolution in recursive module/namespace scopes (#19744)

(cherry picked from commit 1d8dc39)

* Correct StructLayout size emission for data-less struct unions (#19759)

(cherry picked from commit 1dc395a)

* Report FS3888 for generic attribute type abbreviations instead of FS0193 (#19915)

(cherry picked from commit 8c0e444)

* Support NotNullIfNotNullAttribute (#19977)

(cherry picked from commit 0c94e4e)

* Compiled ToStrings under -reflectionfree for DUs and Records (#19976)

* Add a compiler intrinsic for the 'string' operator

Adds string_operator_info / mkCallStringOperator so generated code can call
Operators.string. These lines are duplicated by the interpolated-string PR
(#19971); kept identical there so a future merge resolves cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Generate a match-based ToString for unions under --reflectionfree

Under --reflectionfree the union ToString previously emitted nothing, so
DUs fell back to Object.ToString() (the namespace-qualified type name).
Instead generate a match over the cases that builds "CaseName(f0, f1, ...)"
using the 'string' operator on each field, via a TypedTree expression fed
to CodeGenMethodForExpr. This recurses naturally into nested unions and is
reflection-free. The default (sprintf "%+A") path is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Extract mkStringConcat helper for arity-dispatched String.Concat

The "concatenate a list of string exprs, picking the cheapest String.Concat
overload by arity" pattern was duplicated in CheckExpressions (interpolation
lowering) and the optimizer, and our new union ToString used the array
overload unconditionally. Extract mkStringConcat into TypedTreeOps.ExprOps
and route all three through it. This also lets single-field union cases emit
Concat3 instead of allocating a string[] (IlxGen runs after the optimizer, so
nothing else would collapse that array form).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Fix generated union ToString for generic unions

The match-based ToString body is a TypedTree expression codegen'd via
CodeGenMethodForExpr, but it was built with `eenv`, which lacks the tycon's
type parameters. For generic unions this produced wrong IL: the wrong case
branch (always the null-as-true-value case) or a NullReferenceException for
single-case unions. Use `eenvinner` (the per-tycon environment) so the
generic method body resolves its type parameters. The old sprintf path was
unaffected because it emits raw IL off the pre-built ilThisTy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Render union ToString fields like option (null -> "null")

To make a generated union ToString consistent with how option/list format
their contents (LanguagePrimitives.anyToStringShowingNull), format each field
as: if (box field) is non-null then 'string field' else "null". Previously a
null field rendered as "" (the 'string' operator's null behaviour). Generated
inline rather than calling anyToStringShowingNull, which is internal to
FSharp.Core and so not callable from user-compiled code.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Tidy reflection-free union ToString tests

Normalize union declarations to a leading '|', use System.Console.WriteLine
instead of printfn (the printf machinery is what these changes move away
from), and make the null-field test compare the union's rendering directly
against option's rather than asserting a fixed string.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Add reflection-free ToString to Result and Choice

Result and Choice had no ToString override, so they fell back to the
compiler-generated sprintf "%+A" one, which uses reflection. Give them
hand-written overrides mirroring option/list (String.Concat +
anyToStringShowingNull), e.g. Ok 5 -> "Ok(5)", Choice1Of2 7 -> "Choice1Of2(7)".
This is reflection-free / AOT-friendly and consistent with option's "Some(x)"
rendering. Note: this changes the observable ToString of Result/Choice from
the "%A"-style "Ok 5" to "Ok(5)".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Generate a single-line ToString for records under --reflectionfree

Records previously fell back to Object.ToString() (the namespace-qualified
type name) under --reflectionfree. Generate "{ F1 = v1; F2 = v2 }" on a single
line (no line breaks, unlike sprintf "%+A"), with fields formatted like union
fields (null -> "null", otherwise via 'string'). Factor the shared field
formatter and ToString-method emission out of the union path. The default
(sprintf "%+A") path is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Update FSharp.Core surface-area baselines for Result/Choice ToString

Result and Choice`2..7 now declare an explicit ToString() override, so they
appear in the public surface area.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Add release notes

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Generate a single-line ToString for anonymous records under --reflectionfree

Drive anonymous-record ToString through the synthetic record tycon (already
built for equality/comparison) rather than sprintf "%A", so under
--reflectionfree it renders "{| Name = value; ... |}" on a single line.
GenRecordToStringMethod now takes open/close brace strings ("{ "/" }" for
records, "{| "/" |}" for anonymous records). The default (non-reflection-free)
codegen path is unchanged and still falls back to sprintf "%+A".

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Test that a hand-written ToString override is kept under --reflectionfree

Addresses review feedback: generation is gated on `not (HasMember "ToString")`,
so a user-defined ToString on a union or record wins over the generated one.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Rename ToString generators for clarity

Addresses review feedback: distinguish the reflective sprintf path from the
structural one. GenPrintingMethod -> GenSprintfPrintingMethod (the sprintf "%+A"
ToString/get_Message), GenToStringMethodFromExpr -> EmitToStringMethodDef.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Restore tabular layout for string_operator_info in TcGlobals

Addresses review feedback: keep the column-aligned layout of the surrounding
intrinsic table. Also makes these two lines byte-identical to the same intrinsic
added by #19971, so a future merge resolves cleanly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Add reflection-free ToString tests for field shapes, structs, anon records and recursion

Covers DU field shapes (multiple fields vs a single tuple field), explicit
vs unnamed field names rendering identically, struct unions/records,
anonymous and struct anonymous records, and finite recursive/nesting types.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Add EmittedIL tests for reflection-free record and union ToString

Locks in the IL emitted under --reflectionfree: each field is boxed and
rendered through Operators.ToString with a null guard, and the parts are
joined with String.Concat (array form for the record, 3-arg form for the
single-field union case). Nullary union cases return the bare case name.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Generate reflection-free ToString in the augmentation phase

The structural ToString for --reflectionfree records and unions was built in
IlxGen, after the optimizer, so its per-field 'string' operator calls were
never inlined: each value-type field was boxed and rendered through the
generic Operators.ToString, behind a null guard that is dead for a value type.

Move the generation into the type-augmentation phase (alongside
Equals/GetHashCode/CompareTo) so the body flows through the optimizer. The
'string' operator is now specialised - a value-type field renders via a direct,
allocation-free invariant-culture ToString with no boxing and no null guard
(reference fields keep the guard so null still renders as "null"). The shared
body builders live in AugmentTypeDefinitions; anonymous record types are
synthesized too late for augmentation, so they keep generating in IlxGen but
reuse the same builder.

Output is unchanged; the EmittedIL baselines are updated to the leaner IL.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Guard generated reflection-free ToString against deep-recursion overflow

The augmentation-generated structural ToString recurses into fields, so a deeply
nested value can exhaust the stack with an uncatchable StackOverflowException.
Emit RuntimeHelpers.EnsureSufficientExecutionStack() at method entry (as C# records
do in PrintMembers) so it throws a catchable InsufficientExecutionStackException
instead, when the runtime provides the method. The guard is skipped for types whose
every field is a flat primitive (integer/float/decimal/string/char/bool/unit/enum),
which cannot recurse.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Test the reflection-free ToString deep-recursion guard

A 1,000,000-deep value's generated ToString throws a catchable
InsufficientExecutionStackException rather than hard-crashing the process.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* revert ToString additions to fsharp.core types

* Remove stale FSharp.Core release note for the reverted Result/Choice ToString

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Fix code formatting in IlxGen.fs (dotnet fantomas)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* don't use quoted name

* int version of reflectionfree-printing doc

* doc tweaks

* Link release note to the printing doc and cover anonymous records

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test backticks

* Share the ToString recursion guard with anonymous records

The guard lived in MakeBindingsForToStringAugmentation, which anonymous
records bypass: they are synthesized too late for type augmentation and
reach mkRecdToString from IlxGen instead. Deep nesting overflowed the
stack rather than raising InsufficientExecutionStackException.

Move it into mkToStringRecursionGuard, applied inside mkRecdToString and
mkUnionToString, so every caller of the body builders gets it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Add EmittedIL baselines for struct and anonymous record ToString

Struct records and unions read fields off the this pointer and switch on
the tag, and the anonymous record path is generated separately in IlxGen,
so each gets its own baseline.

The anonymous baseline omits the field reads: they name the anonymous
type, whose mangled name is not stable across compilations.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Fix empty anonymous record ToString rendering a doubled space

The open/close braces carry inner spaces ("{| " and " |}"); with no
fields they abut and render "{|  |}". Trim the leading space when the
field list is empty, matching %A's "{| |}".

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* tidy comment

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
(cherry picked from commit 17cb503)

* Run ilverify via the tool manifest instead of a hard-coded cache path (#20101)

(cherry picked from commit c00299f)

* Record spreads (#18927)

(cherry picked from commit f4b785f)

* Implement interpolated strings via String.Concat (#19971)

(cherry picked from commit d3403ca)

* Secure release-note checks for fork pull requests (#20081)

* Secure release-note checks for fork pull requests

* Address release-note workflow review feedback

(cherry picked from commit 647548b)

* Update test project to net11 (#20104)

* Update test project to net11

Internal CI was failing since the move to net11 because restoring this
test project had to suddenly be done via network call to nuget.org

* Update target framework and PDB path in tests

(cherry picked from commit 536800c)

* LexFilter: drop non-strict mode (#20106)

(cherry picked from commit ea778bb)

* Implement direct delegates (#19993)

(cherry picked from commit d89529c)

* Fix check_release_notes 403 by restoring pull-requests: write and making the comment non-fatal (#20198)

check_release_notes runs via pull_request_target, so GitHub executes the workflow from the default branch (main). Creating the informational PR comment requires pull-requests: write, but #20081 reduced the token to read, turning the check red with HTTP 403 on any PR that had to create (not update) the comment - e.g. Maestro/darc PR #20133. Restore pull-requests: write so the comment posts, and guard the comment step with continue-on-error plus try/catch so posting can never fail the release-notes verdict. Supersedes #20200.

(cherry picked from commit 92a3700)

* Adopt ordered multi-caret markers in FCS tests (no behaviour change) (#20082)

* Clean up goto-def tests: drop unused open System and redundant caret-anchor comments

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9

* Collapse copy-pasted completion sources with ordered {caretN} markers

Uses SourceContext.extractOrderedMarkedSources (multi-caret) to replace whole-source
[<Theory>] copies with one marked source in DotOff.ArraySliceNotation (3 copies) and
CurriedArguments.Regression (5 copies). No behaviour change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9

* Collapse copy-pasted completion sources in PatternMatching and Generics

Bug312557_2 (4 source copies -> 1 {caretN}) and Bug69673_1.CtrlSpaceForThis
(2-row Theory -> 1). No behaviour change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9

* Collapse Symbols 'Nested copy-and-update' 8 copied sources into one {caretN}

The 8 Facts shared an identical source copied per caret; now one {caret1..8}
source + a (field-name, range) cases list. No behaviour change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9

* BreakpointLocation: mark expected ranges with {selstart}/{selend} in source

Replaces magic ((line,col),(line,col)) tuples with {selstart}/{selend} markers
around the breakpoint span; the validation caret is inferred from {selend}
(SourceContext), so no {caret} needed. Compares against context.SelectedRange.
No behaviour change (all 6 tests green).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9
(cherry picked from commit 2038269)

* Record spreads: off-by-default shadowing warnings (#20206)

(cherry picked from commit 05af9c8)

* Remove always-on PrintfBinaryFormat language feature flag (#20202)

* Add RED tests for unconditional %B and rejected --disableLanguageFeature:PrintfBinaryFormat

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove always-on PrintfBinaryFormat language feature flag

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Correct langversion comment in disableLanguageFeature %B guard test

Fixes an inaccurate comment (claimed minimum accepted --langversion is 8.0)
flagged during expert review. %B is now unconditional across all langversions
after removing the PrintfBinaryFormat feature flag. Comment-only change.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Make %B guard test accurate: assert compile+run at supported langversion

The expert-review finding noted the %B guard test compiled under the default
(latest) langversion where %B was already accepted, giving no regression value
for the removed 6.0 gate. A sub-6.0 test is infeasible: the minimum supported
langversion is 8.0 (lower versions error with FS3880), already above the old
6.0 gate, so removing the PrintfBinaryFormat flag is a pure no-op for every
supported langversion. The guard test now clearly asserts the %B code path is
intact (compiles and runs, producing 10011), with the 3881 rejection test
covering removal of the feature name. Comment documents why no sub-6.0 test
exists.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add release notes for PrintfBinaryFormat flag removal

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Drop release notes and added guard tests: this is cleanup-only

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 5fa2950)

* Stabilize preview language features into F# 11.0 (#20199)

* Stabilize preview language features into F# 11.0

Move MethodOverloadsCache, ErrorOnMissingSignatureAttribute,
DirectDelegateConstruction, AccessProtectedBaseFieldFromClosure and
RecordSpreads from previewVersion to languageVersion110. FromEndSlicing
stays in preview in its own block. Also relocate the misplaced
ImplicitDIMCoverage into the F# 11.0 block and move the corresponding
release notes from .Language/preview.md into .Language/11.0.md.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

* Fill PR number in release note

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 84eefce)

* Implement `<inheritdoc>` XML documentation support for F# (#19188)

(cherry picked from commit 93659f3)

* Add support for `<include>` XML documentation tag (#19186)

* Add support for <include> XML documentation tag (#19186)

Implement support for expanding <include file="..." path="..."/> elements
in XML doc comments when generating documentation files via --doc.

(cherry picked from commit 5260b68)

* Fix false prompt-injection alarm on PR Tooling Safety Check bypass labels (#20130)

* Fix false prompt-injection alarm on PR Tooling Safety Check bypass labels

The threat-detection job is a separate LLM that only sees the workflow
description plus the agent's output, not the process steps. When the agent
correctly applies AI-Tooling-Check-Bypassed to a non-fork PR, the detector
misreads the bypass label as the agent being manipulated into skipping its
scan and raises a false prompt-injection alarm, aborting the run's label and
memory outputs. Give the detector context via threat-detection.prompt.

(cherry picked from commit 2c245dd)

* feat(Async): RunSynchronouslyImmediate (#19804)

(cherry picked from commit 9357a4f)

* Add ECMA-335 EnC metadata delta writer (#20019)

* Add ECMA-335 EnC metadata delta writer

Adds an internal, standalone ECMA-335 Edit-and-Continue metadata delta writer to
AbstractIL: delta #- table stream and heap construction (DeltaMetadataTables,
DeltaMetadataSerializer, DeltaTableLayout, DeltaIndexSizing), ECMA-335 II.24.2.6
coded-index encoding (DeltaMetadataEncoding), EncLog/EncMap emission, generation GUID
chaining, user-string and standalone-signature token calculators (IlxDeltaStreams),
and the coordinating writer (FSharpDeltaMetadataWriter) over a plain row-description
input model (DeltaMetadataTypes, ILDeltaHandles, ILMetadataHeaps).

The writer's inputs are row records (names, tokens, signatures, RVAs) plus heap
offsets; it has no dependency on any semantic diffing or session machinery. It
compiles with no in-tree consumer by design: the consumer is the F# hot reload work
in #19941, following the same upstreaming pattern as #20017 and #20018
(land isolated, test-covered infrastructure first, wire the feature in a later PR).

One line of ilwrite.fsi is touched to expose the pre-existing markerForUnicodeBytes
so the delta writer reuses the exact string-marker logic of the full writer. No
behavior change for any existing code path.

Tests (130): coded-index encodings asserted against the production definitions and
ECMA-335 II.24.2.6 order, System.Reflection.Metadata reader parity over emitted
deltas, EncLog/EncMap correctness, stream layout, heap and index sizing,
multi-generation heap-offset chaining asserted against computed expected values,
standalone-signature rows asserted at baseline+1 from a real seeded baseline, and
serializer failure paths.

(cherry picked from commit e9fdc37)

* Remove always-on StructActivePattern language feature flag (#20208)

(cherry picked from commit 2def18d)

* Remove always-on OpenTypeDeclaration language feature flag (#20209)

(cherry picked from commit e1ad05c)

* Allow closing '>' of multiline nested type arguments to align with the opener (#15171) (#20003)

(cherry picked from commit 34a3053)

* Record constructors (FS-1073) (#19974)

(cherry picked from commit 7e2cbf1)

* docs: update state-machine.md for changed workflow SHAs (#20234)

Incremental update:
- Fix LPSS safe-output label count (10 → 11)
- Update source-shas footer

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 4a929f3)

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 3039021 (#20108)

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 3036509

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 3036509

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 3038596

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 3038596

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 3038733

* Localized file check-in by OneLocBuild Task: Build definition ID 499: Build ID 3038733

---------

Co-authored-by: Copilot <copilot@github.com>
(cherry picked from commit ab2e066)

* Enable MSBuild node reuse defaults in eng/Build.ps1 and eng/build.sh (#20220)

Both entry points hard-disabled MSBuild node reuse for every build,
overriding Arcade's default (node reuse on locally, off on CI). The
overrides were added together in March 2019 (7a6448e, f4bd221)
as a workaround for FSharp.Build.dll version conflicts when the proto
compiler and the freshly built one are loaded into reused MSBuild nodes
during F#'s self-bootstrap.

This removes both overrides so Arcade's default applies, restoring node
reuse for the inner loop and unblocking MSBuild Server. Whether the 2019
FSharp.Build.dll conflict still reproduces is validated by full CI,
including the Windows EndToEndBuildTests job, which builds without -ci
and therefore now exercises node reuse end to end.

Fixes #20217

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 80b7e4e)

* Fix spurious FS0410 for tuple patternInput bindings (#4161) (#19947)

Skip accessibility checks on compiler-generated patternInput module bindings.
These temps are module-init scaffolding whose visibility does not reflect the
enclosing let-binding scope, which caused false FS0410 errors when tuple
deconstruction referenced private types in the same module.

Fixes #4161

(cherry picked from commit 853c500)

* Ignore lines starting with `#:` (RFC-1337) (#20212)

(cherry picked from commit 8111bff)

* feat(Async+Task+ValueTask): consistent helper modules (#19844)

(cherry picked from commit 4890294)

* Add hot reload baseline reading and recorded EnC state (#20026)

* Add Roslyn-format EnC CustomDebugInformation codec and portable PDB method CDI emission

Adds an internal AbstractIL module implementing, byte for byte, the three Portable PDB
CustomDebugInformation blob formats Roslyn persists per method for Edit and Continue
(EnC Local Slot Map, EnC Lambda and Closure Map, EnC State Machine State Map), with
serializers, deserializers, a portable PDB read-back helper, and an occurrence-key
packing helper for deterministic syntax-offset slots.

Plumbs an optional methodCustomDebugInfoRows side channel through the IL binary writer
options into the portable PDB generator so a compilation can attach CDI rows to named
methods. Names that do not identify exactly one method row are dropped. All existing
writer call sites pass an empty map, so emitted PDBs are byte-identical to before.

No in-tree caller populates the map yet; the consumer is the F# hot reload work in
#19941, following the same pattern as #20017 (land isolated, test-covered
infrastructure first, wire the feature later).

Tests: blob round-trips, Roslyn golden-byte encodings, cross-validation against
CDI blobs emitted by a real Roslyn compilation, fail-closed occurrence-key packing
(including an int32-overflow regression where a wrapped negative key previously
escaped the bound check), and end-to-end synthetic PDB emission proving correct
MethodDef parenting, zero rows for an empty map, and no rows for absent or
ambiguous names.

(cherry picked from commit 51dd37e)

* Optimizer: propagate NoDynamicInvocation (#20242)

(cherry picked from commit 23fa8d5)

* Support OverloadResolutionPriority and a most-concrete overload tiebreaker

Two opt-in (`--langversion:preview`) additions to method overload resolution.

- F# now honors `System.Runtime.CompilerServices.OverloadResolutionPriorityAttribute` (.NET 9): among the applicable candidates the highest-priority ones are considered first, matching C#. Priority is applied before betterness and scoped by the declaring type, so a library can steer F# callers to the overload it intends.

- When overload resolution would otherwise be ambiguous, the more concrete candidate is now preferred — `Option<'T>` over `'T`, `Task<'T>` over `'T`, `Result<'T, string>` over `Result<'T, 'E>` — across constructors, static and instance members, extension members, and optional/ParamArray tails. Candidates that are incomparable (more concrete at different positions, or differing only by an SRTP constraint) stay ambiguous rather than being resolved arbitrarily.

The tiebreaker is transparent: opt into `--warnon:3575` to see which concrete overload was selected and `--warnon:3576` for each generic overload it bypassed.

RFC FS-1338 (OverloadResolutionPriority) and RFC FS-1340 (most-concrete tiebreaker).

(cherry picked from commit 089835d)

* Remove always-on UseBindingValueDiscard language feature flag (#20162) (#20218)

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit c2b38d7)

* Remove always-on WildCardInForLoop language feature flag (#20221)

* Remove always-on WildCardInForLoop language feature flag

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Add release note for WildCardInForLoop flag removal

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Strip release-notes changes (NO_RELEASE_NOTES)

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit afe45bf)

* Remove always-on AndBang language feature flag (#20223)

* Collapse always-on AndBang enforcement gates in CheckComputationExpressions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove always-on AndBang language feature flag and regenerate xlf

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Fix Fantomas formatting after AndBang gate removal

The CheckCodeFormatting CI job failed because the collapsed AndBang gate
left a two-line let binding that Fantomas collapses to a single line.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 4990959)

* Remove always-on ImplicitYield language feature flag (#20230)

* Collapse ImplicitYield enforcement sites to always-enabled path

Remove the dead 'SupportsFeature LanguageFeature.ImplicitYield' guards, whose condition is always true for langversion >= 8.0, across the four Checking/Expressions files. Each guard is collapsed to its enabled branch and the dead branch/binding deleted. Behaviour is unchanged; the flag definition in LanguageFeatures.fs is intentionally left for a later sprint.

The now-unused 'cenv' parameter of YieldFree is renamed to '_cenv' (not removed): FSharp.Profiles.props enables --warnon:1182 with WarningsAsErrors, so an unused named binding is a hard build error, and removing the parameter would cascade through IsSimpleSemicolonSequenceElement, TryGetSimpleSemicolonSequenceOfComprehension and the SimpleSemicolonSequence active pattern (all out of scope). The '_' prefix preserves the exact signature and all positional call sites.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove ImplicitYield language feature flag definition

The ImplicitYield feature shipped in F# 4.7 and is permanently enabled
(minimum accepted --langversion is 8.0), making the flag dead configuration.
Remove the LanguageFeature.ImplicitYield union case, its features-map entry,
the GetFeatureString arm, the featureImplicitYield FSComp.txt string, and
regenerate the xlf localisation files. The always-on
SynExpr.SequentialOrImplicitYield AST machinery is untouched.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
(cherry picked from commit 9f19d64)

* Import: Don't walk non-F# assemblies when labelling trait constraint sources (#20090)

* Don't walk non-F# assemblies when labelling trait constraint sources

`addConstraintSources` (added in #16304, so that a failed member constraint
names the member it came from) is applied to every imported assembly, and
recurses through `e.ModuleOrNamespaceType` for every module and namespace
entity it finds.

For an assembly imported from IL there is nothing to find: the walk only
reads `AllValsAndMembers`, and `ImportILTypeDefs` gives every namespace and
type entity an empty val list; only an F# trait constraint produces a
`TyparConstraint.MayResolveMember` to label in the first place. Meanwhile the
recursion forces each namespace entity's `ModuleOrNamespaceType`, which
imports that namespace - so referencing an assembly ends up importing every
namespace in it, and reading every type definition, whether or not the code
touches it.

Skip the CCUs that aren't F#. FSharp.Core and F# references are still walked,
so the error messages are unchanged.

Measured with FSharpChecker.ParseAndCheckProject, keeping the results alive so
the imported assembly structures stay on the heap (averages of 3 runs, one per
process): a 486-reference F# project retains 1319.2 -> 952.1 MB (-27.8%), and
a 168-reference console project 77.7 -> 69.8 MB (-10.2%). Checking
FSharp.Compiler.Service itself (124 references, 397 sources) goes 2301.7 ->
2298.0 MB, i.e. within the noise at that size - what the imports cost there is
dwarfed by the trees of the project's own code.

(cherry picked from commit 623d80d)

* chore(Async.RunSynchronouslyImmediate): Stragglers re #19804 (#20245)

* chore(Async.RunSynchronouslyImmediate): Stragglers from #19804

- rename and sync clone impls withing VisualFSharp.slnx as per previous PR
- update RunImmediateExceptOnUI to delegate and follow naming

(cherry picked from commit 1852df8)

* Parser: recover on unfinished abstract members (#20070)

(cherry picked from commit ce9b62a)

* Remove always-on SingleUnderscorePattern language feature flag (#20222)

* Remove always-on SingleUnderscorePattern language feature flag

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Remove orphaned parsUnexpectedSymbolDot diagnostic resource

Its only two consumers were the parser guards removed when the always-on SingleUnderscorePattern language feature was deleted, leaving the FSComp.txt entry and 13 xlf trans-units unreachable. Regenerated xlf via UpdateXlf.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Tomas Grosup <tomas.grosup@microsoft.com>
(cherry picked from commit 22e1934)

* Add symbol and type highlighting to F# diagnostics (#20097)

(cherry picked from commit 208b7b4)

* Avoid per-instance lock object in InterruptibleLazy and DelayInitArrayMap (#20088)

* Avoid per-instance lock object in InterruptibleLazy and DelayInitArrayMap

Both types allocated a dedicated `syncObj = obj()` for their one-time
initialisation. These instances are internal and never locked externally,
and there are enough of them (one per lazy IL member, per ILTypeDefs /
ILMethodDefs, etc.) that the extra bare System.Object adds up to tens of MB
on a large project. Lock on `this` instead and drop the field.

Measured on a single-file FCS check against a project with ~486 references:
bare System.Object instances dropped from ~1,000,000 to ~29,000 (~-22 MB).

(cherry picked from commit 29718dd)

* Regression matrix: build unmodified repos against local FSharp.Core (#20240)

(cherry picked from commit ffa6ef1)

* IL: add ILPreNamespace, make ILPreTypeDef creation lazy (#20092)

* IL: add ILPreNamespace, make ILPreTypeDef creation lazy

(cherry picked from commit d3fab44)

* feat(Async): Add exception-unwrapping Await (#19785)

(cherry picked from commit 1e94cc5)

* IL: fix leaking binary view (#20250)

(cherry picked from commit 6ac056a)

* IL: use empty tables for members when possible (#20249)

(cherry picked from commit e09605c)

* Add regression test: #14454, IAsyncDisposable use in task CE in FSI (#20251)

(cherry picked from commit 5d6e4dc)

* Teach compiler driver to ignore unknown warning codes (#20246)

(cherry picked from commit 0640920)

* Remove always-on RelaxWhitespace language feature flag (#20226)

(cherry picked from commit d26c842)

* Add nullness regression test for recursive inline SRTP memoization (#20247)

(cherry picked from commit 492d417)

* Ship FSharp.Core with a net10.0 target framework (selective port of #20229)

Adapted from upstream PR #20229 for the 10.0.4xx servicing
packaging model. Ported: FSharpCoreShippedNetTargetFramework=net10.0 knob
(Directory.Build.props/.targets, eng/TargetFrameworks.props), FSharp.Core
fsproj/nuspec net10.0 asset shipment, FSharp.Core/*.fs(i) #if NETSTANDARD2_1
|| NET widening, FSharp.Compiler.fsproj dependent-project TFM wiring,
VerifyNetPackage.fsx package validation, ILVerify net10.0 baselines, and
FSharp.Core/FCS surface-area test updates.

Not ported: nothing was dropped from the upstream patch's actual net10.0
packaging content; the only upstream hunk excluded was an unrelated
Central-Package-Management NU1507 NoWarn block that only applies when
ManagePackageVersionsCentrally is set (introduced by the separately
excluded #20084 CPM migration, not by #20229 itself). No Arcade 11 or
.NET 11 SDK/TFM assumptions are introduced; the destination's existing
SDK pin, Arcade version, and servicing branding are preserved.

(cherry picked from commit 1928203, adapted)

* Flow release/10.0.4xx into rel/d18.11-ltsc for VS insertion (adapted from #20412)

Upstream PR #20412 ("Make release/dev18.11 flow into
rel/insiders") retargeted release/dev18.11's VS insertion branch from
rel/main to rel/insiders, and set FSharpReleaseBranchName from main to
release/dev18.11.

This branch (release/10.0.4xx) already has FSharpReleaseBranchName set
to its own servicing branch (release/10.0.4xx), so that part does not
apply here and is left untouched. The equivalent, required change for
this servicing branch is only the VS insertion target: route VS
insertion to rel/d18.11-ltsc instead of rel/insiders, matching the LTSC
servicing relationship between this F# branch and its VS branch. No
other branch, package, or servicing branding changed.

(cherry picked from commit ac4fecf, adapted)

* Fix ILVerify baseline filenames for net10.0 FSharp.Core asset (follow-up to #20229 adaptation)

The original upstream #20229 commit shipped the ILVerify baseline files
named with a literal 'netcoreapp' TFM segment
(ilverify_FSharp.Core_{Debug,Release}_netcoreapp.bsl), but ilverify.ps1
resolves the baseline filename dynamically from the
FSharpCoreShippedNetTargetFramework MSBuild property, which in this
destination resolves to 'net10.0' (not 'net11.0' as in later upstream
history, and not the literal string 'netcoreapp'). Renamed the two
empty (zero-error) baseline files to ilverify_FSharp.Core_{Debug,Release}_net10.0.bsl
so tests/ILVerify/ilverify.ps1 finds them. Verified directly via
'dotnet ilverify --sanity-checks --tokens' against the built Release
net10.0 FSharp.Core.dll: 'All Classes and Methods ... Verified.' with
zero errors, matching the (empty) baseline.

* Use the 43.13 FCS family for the 18.11 backport

Signal the upstream breaking tokenizer, tooltip and record AST contracts with a new FCS package family. Keep compiler and Core servicing versions unchanged. Add a red-green version guard and package-linked migration notes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ee897a29-d64c-4e85-954e-cded7192eea6

---------

Co-authored-by: Nat Elkins <nat@nelknet.com>
Co-authored-by: dotnet bot <dotnet-bot@dotnetfoundation.org>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Eugene Auduchinok <eugene.auduchinok@jetbrains.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: kerams <kerams@users.noreply.github.com>
Co-authored-by: Charles Roddie <charles.roddie@cantab.net>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Adam Boniecki <20281641+abonie@users.noreply.github.com>
Co-authored-by: Brian Rourke Boll <brianrourkeboll@users.noreply.github.com>
Co-authored-by: Charles Roddie <charles.roddie@summatic.co.uk>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Ruben Bartelink <ruben@bartelink.com>
Co-authored-by: Edgar Gonzalez <egonzalez@totallymoney.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Kirtikumar Anandrao Ramchandani <33368817+KirtiRamchandani@users.noreply.github.com>
Co-authored-by: Martin <29605222+Martin521@users.noreply.github.com>
Co-authored-by: Tomas Grosup <tomas.grosup@microsoft.com>
Copilot-Session: 9d20cd22-45dc-411f-9163-185fd6dd54d9
Copilot-Session: ee897a29-d64c-4e85-954e-cded7192eea6
Copilot-Session: 34374e7f-b8e0-4d82-89f8-d289e6cdd7f6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants