From d5b2bff62e480fad48ed48645dc5c3b9060d6781 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 20 Sep 2026 19:25:42 +0200 Subject: [PATCH] Note when a shared field needs option instead of voption The voption guidance said no field is ever a reason to pick option, including ones shared between threads. That is wrong for a lock-free field: option is always one atomic-sized reference, voption is a struct that can span more than one word and tear on an unsynchronized write. PR #20523 hit this for a Position field a text-view listener writes and the project options reactor reads. Co-Authored-By: Claude Sonnet 5 --- .github/instructions/FSharp.instructions.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/instructions/FSharp.instructions.md b/.github/instructions/FSharp.instructions.md index 60a6a053c08..cd0b73d76cc 100644 --- a/.github/instructions/FSharp.instructions.md +++ b/.github/instructions/FSharp.instructions.md @@ -25,7 +25,8 @@ When the IDE's F# semantic tools are unavailable, use the `F#` MCP server (`.mcp ## Values and types -- `voption` – `ValueSome`/`ValueNone` – over `option`; it is this compiler's option type. Fields, members, parameters and values shared between threads included: none of them is a reason to pick `option`. Exception: when an API hands you `'T option` and has no `voption` counterpart, unwrap with `Option.defaultValue`/`Option.defaultWith` directly – do not insert `ValueOption.ofOption` just to switch modules. +- `voption` – `ValueSome`/`ValueNone` – over `option`; it is this compiler's option type. Exception: when an API hands you `'T option` and has no `voption` counterpart, unwrap with `Option.defaultValue`/`Option.defaultWith` directly – do not insert `ValueOption.ofOption` just to switch modules. +- A field one thread writes without a lock and another reads needs an *atomic-sized* write, not `voption`'s style. `'T option` is always a single reference (atomic on any width); `'T voption` is a struct that grows with `'T` and can span more than one word – for a `'T` already close to a full word (like `Position`'s `[]` `int64`, where the voption tag pads it past one), a torn write is possible. Use `'T option` for such a field specifically, and only for it – a local binding built from it, never touched by another thread, stays `voption` as always. - The mirror case, an API that *takes* `'T option` (an optional argument `?caret = …`, a field typed `IDisposable option`): stay in `ValueOption` through the whole chain and convert once, last – `x |> ValueOption.bind _.Position |> ValueOption.toOption`, never `x |> ValueOption.toOption |> Option.bind _.Position`. - `vsintegration` has `voption`-returning counterparts of the FSharp.Core collection functions, suffixed `V`, in `FSharp.Editor/Common/Extensions.fs`: `Seq.tryHeadV`/`tryFindV`/`tryFindIndexV`/`tryPickV`/`chooseV`, `Array.tryHeadV`/`tryFindV`/`tryPickV`/`chooseV`, `List.tryFindV`, `ImmutableArray.tryHeadV`. Reach for those rather than the `option`-returning original. The module is `[]` and compiles before the rest of `FSharp.Editor`, so a file in the `Microsoft.VisualStudio.FSharp.Editor` namespace needs no `open` for them. `src/Compiler` has no equivalents. - `struct ('T1 * 'T2)` tuples and `[]` types on allocation-sensitive paths.