Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2c5ac7f
Update azure-pipelines-PR.yml
T-Gro Feb 3, 2026
b18d0e2
Update azure-pipelines-PR.yml
T-Gro Feb 6, 2026
6c34e21
add opentk
T-Gro Feb 6, 2026
db6d174
Merge branch 'main' into more-regression-libs
T-Gro Feb 7, 2026
ab1a08f
Merge branch 'main' into more-regression-libs
T-Gro Feb 9, 2026
c472cf1
Fix: replace '.' in repo names for valid AzDO job identifiers
T-Gro Feb 9, 2026
2a0dcfd
Fix regression test template for buildScript with arguments
T-Gro Feb 9, 2026
ac6d753
Fix regression test failures: add .NET 9.0 SDK, use direct dotnet com…
T-Gro Feb 9, 2026
037e7b0
Add dotnet tool restore step before build
T-Gro Feb 9, 2026
bb026d4
Split OpenTK into two matrix entries to test both projects
T-Gro Feb 9, 2026
93c31cb
Fix FS3356 false positive: only check static extension members for du…
T-Gro Feb 9, 2026
204d2fd
Support multi-command buildScript with ';;' separator
T-Gro Feb 9, 2026
85f3930
Fix FS0229: B-stream misalignment in TypedTreePickle when langFeature…
T-Gro Feb 9, 2026
2a62398
Add documentation for FS0229 B-stream misalignment regression
T-Gro Feb 9, 2026
4806039
Add release notes for FS0229 and FS3356 fixes
T-Gro Feb 9, 2026
5561c99
Fix FS3356 tests: use static members for IL collision tests, allow in…
T-Gro Feb 9, 2026
0408122
Fix pre-existing ImportTests warning 52 on net472
T-Gro Feb 9, 2026
52af876
Link release notes to originating PRs
T-Gro Feb 10, 2026
d770dd5
Extract p_nullnessB helper to reduce repetition in p_ty2
T-Gro Feb 10, 2026
bb6675d
Add inline comments for B-stream nullness tag constants
T-Gro Feb 10, 2026
7ac791e
Merge branch 'main' into more-regression-libs
T-Gro Feb 10, 2026
d51d091
Add .github/skills/ to .fantomasignore (#19268)
Copilot Feb 11, 2026
ceed0ce
improving file specific instructions, adding postmortem skill
T-Gro Feb 11, 2026
c7a0053
Merge branch 'main' into more-regression-libs
T-Gro Feb 11, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .fantomasignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ artifacts/

# For some reason, it tries to format files from remotes (Processing ./.git/refs/remotes/<remote>/FSComp.fsi)
.git/
.github/skills/

# Explicitly unformatted implementation
src/Compiler/Checking/AccessibilityLogic.fs
Expand Down
6 changes: 6 additions & 0 deletions .github/instructions/CodeGen.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
applyTo:
- "src/Compiler/CodeGen/**/*.{fs,fsi}"
---

Read `docs/representations.md`.
6 changes: 6 additions & 0 deletions .github/instructions/DebugEmit.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
applyTo:
- "src/Compiler/AbstractIL/ilwritepdb.{fs,fsi}"
---

Read `docs/debug-emit.md`.
6 changes: 6 additions & 0 deletions .github/instructions/FSComp.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
applyTo:
- "src/Compiler/FSComp.txt"
---

Read and follow `docs/diagnostics.md`.
6 changes: 6 additions & 0 deletions .github/instructions/FSharpCore.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
applyTo:
- "src/FSharp.Core/**/*.{fs,fsi}"
---

Read and follow `docs/fsharp-core-notes.md`.
6 changes: 6 additions & 0 deletions .github/instructions/LSP.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
applyTo:
- "src/FSharp.Compiler.LanguageServer/**/*.{fs,fsi}"
---

Read `docs/lsp.md`.
6 changes: 6 additions & 0 deletions .github/instructions/Optimizer.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
applyTo:
- "src/Compiler/Optimize/**/*.{fs,fsi}"
---

Read `docs/optimizations.md`.
7 changes: 7 additions & 0 deletions .github/instructions/SyntaxTree.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
applyTo:
- "src/Compiler/SyntaxTree/SyntaxTree.{fs,fsi}"
- "src/Compiler/pars.fsy"
---

Read `docs/changing-the-ast.md`.
73 changes: 73 additions & 0 deletions .github/instructions/TypedTreePickle.instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
---
applyTo:
- "src/Compiler/TypedTree/TypedTreePickle.{fs,fsi}"
- "src/Compiler/Driver/CompilerImports.{fs,fsi}"
---

# Pickling: Binary Compatibility Rules

F# embeds metadata into compiled DLLs as binary resources. When project B references project A's DLL, the compiler deserializes that metadata to reconstruct type information. **The compiler that wrote the metadata and the compiler that reads it may be different versions.** This is the central constraint on every change you make here.

## The Compatibility Contract

A library compiled with F# SDK version X will be consumed by projects using SDK version Y. Both directions must work:

- **Forward compatibility**: A newer compiler must read metadata written by an older compiler.
- **Backward compatibility**: An older compiler must read metadata written by a newer compiler.

This means:

1. **Never remove, reorder, or reinterpret** existing serialized data. DLLs compiled with older formats exist in the wild permanently.
2. **Additions must be invisible to old readers.** New data goes in stream B, where readers that don't know about it get `0` (the default sentinel) past end-of-stream. New readers detect presence via a tag byte they write unconditionally.
3. **Tag values are forever.** Once a byte value means something in a reader's `match`, that meaning cannot change. Old DLLs encode that value with the old semantics.

## Reading and Writing Must Be Perfectly Aligned

The format uses two parallel byte streams. Every `p_*` (write) function has a corresponding `u_*` (read) function. They must produce and consume the **exact same byte sequence** under **every possible code path** — including paths gated by feature flags, language versions, or target frameworks that your current build may not exercise.

The dangerous scenario: a write is conditional on some flag, but the corresponding read is unconditional (or vice versa). One skipped byte shifts every subsequent read in that stream, producing `FS0229` errors that manifest far from the actual bug — often only when a specific combination of compiler versions and project configurations is used.

Branching on the **shape of the data being serialized** (e.g., which `TType` case, how many type parameters) is normal and correct — the reader sees the same data and branches the same way. The byte count varies with the data, and that's fine.

What is **not safe** is varying the byte count based on **compiler configuration** — feature flags, `LangVersion`, target framework, or any other setting that lives in the compiler process but is not encoded in the stream. The reader has no access to the writer's settings; it only sees bytes. If a flag causes the writer to skip a byte, the reader will consume whatever byte happens to be next, and every subsequent read shifts.

When reviewing a change, ask: "Does the number of bytes written depend on anything the reader cannot reconstruct from the stream itself?" If yes, the change will break cross-version compatibility.

## Reasoning About Evolution

Before making a change, think through these scenarios:

1. **You write new data. An old compiler reads the DLL.** The old reader doesn't know about your new bytes. Will it silently get `0` defaults and behave correctly, or will it crash or misinterpret data?

2. **An old compiler wrote the DLL. Your new reader processes it.** Your new reader expects data that isn't there. Does it handle the missing-data case (tag value `0`, end-of-stream) gracefully?

3. **Feature flags and language versions.** A single compiler binary may write different data depending on `LangVersion` or feature toggles. The reader processing that DLL has no access to the writer's flags — it only sees bytes. Every flag-dependent write path must still produce a byte-aligned stream that any reader can consume.

4. **Multi-TFM projects.** A solution may compile `netstandard2.0` (with `LangVersion=8.0`) and `net9.0` (with `LangVersion=preview`). The `net9.0` build references the `netstandard2.0` output. Both DLLs were produced by the same compiler binary but with different settings. This is where conditional writes break.

## Testing With the CompilerCompat Suite

If your change alters what gets serialized, **add coverage for it** in the CompilerCompat projects. Add the new type, constraint, or API shape to `CompilerCompatLib/Library.fs` and a corresponding consumer in `CompilerCompatApp/Program.fs`. This ensures your specific change is exercised across compiler versions, not just the pre-existing test surface.

Then run the cross-version compatibility tests:

```bash
dotnet fsi tests/FSharp.Compiler.ComponentTests/CompilerCompatibilityTests.fsx
```

This suite (`tests/projects/CompilerCompat/`) compiles a library with one compiler version, packs it as a NuGet package, then builds a consuming application with a different compiler version. The application references the library as a package — not a project reference — so the consuming compiler must deserialize the library's pickled metadata from the DLL, exercising the real import path.

It tests both directions:

| Scenario | Question answered |
|---|---|
| Library: released SDK → App: your local build | Can your new reader handle the old format? |
| Library: your local build → App: released SDK | Can old readers handle your new format? |
| Same, across .NET major versions (e.g., 9 ↔ current) | Does it hold across SDK generations? |

Also run the import regression tests:
```bash
dotnet test tests/FSharp.Compiler.ComponentTests -c Release --filter "FullyQualifiedName~Import" /p:BUILDING_USING_DOTNET=true
```

For a detailed example of what goes wrong when these rules are violated, see `docs/postmortems/regression-fs0229-bstream-misalignment.md`.
75 changes: 75 additions & 0 deletions .github/skills/postmortem/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
---
name: postmortem
description: Write a postmortem for a regression that escaped to production, broke real users, and traces back to a design flaw worth documenting for future implementors. Only invoke after confirming no existing postmortem or doc covers the same root cause.
---

# Postmortem Writing

## When to Invoke This Skill

All of the following must be true:

1. **Production escape.** The bug shipped in a released SDK or NuGet package. Internal-only or caught-in-CI issues do not qualify.
2. **User-visible breakage.** Real users hit the bug — build failures, runtime crashes, silent wrong behavior. Not a cosmetic or tooling-only issue.
3. **Non-obvious root cause.** The bug traces back to a design assumption, invariant violation, or interaction between independently-correct changes that is worth explaining to future contributors.
4. **Not already documented.** Check `docs/postmortems/` for existing write-ups covering the same root cause. Check `.github/instructions/` for rules that already encode the lesson. If covered, stop.

Do **not** write a postmortem for:
- Typos, simple off-by-one errors, or straightforward logic bugs.
- Bugs caught by CI before merge.
- Issues where the fix is obvious from the diff alone.

## What to Learn Before Writing

Before writing a single line, answer these questions:

1. **How did the bug reach users?** Trace the path: which PR introduced it, which release shipped it, why CI didn't catch it. Understanding the gap in coverage is often more valuable than the fix.
2. **What made it hard to diagnose?** Was the error message misleading? Did the symptom appear far from the cause? Did it only reproduce under specific configurations?
3. **What design assumption was violated?** Every qualifying postmortem has one. A format invariant, a compatibility contract, a threading assumption. Name it precisely.
4. **What would have prevented it?** A test? A code review checklist item? A compiler warning? An agentic instruction? This becomes the actionable outcome.

## Postmortem Structure

Write the file in `docs/postmortems/` with a descriptive filename (e.g., `regression-fs0229-bstream-misalignment.md`).

Use this outline:

### Summary
Two to three sentences. What broke, who was affected, what the root cause was.

### Error Manifestation
What did users actually see? Include the exact error message or observable behavior. Someone searching for this error should find this doc.

### Root Cause
Explain the design assumption that was violated. Keep it high-level enough that someone unfamiliar with the specific code can follow. Use short code snippets only if they clarify the mechanism — not to show the full diff.

### Why It Escaped
How did this get past code review, CI, and testing? Be specific: "The test suite only exercised single-TFM builds" is useful. "Testing was insufficient" is not.

### Fix
Brief description of what changed and why it restores the invariant. Link to the PR.

### Timeline
Table of relevant PRs/dates showing how the bug was introduced, exposed, and fixed. Include latent periods where the bug existed but was masked.

### Prevention
What has been or should be added to prevent recurrence: tests, agentic instructions, CI changes, code review checklists. Link to the specific artifacts (e.g., the `.github/instructions/` file that encodes the lesson).

## After Writing

1. **Identify the trigger paths.** Determine which source files, when changed, would risk repeating this class of bug. Be specific — e.g., `src/Compiler/TypedTree/TypedTreePickle.{fs,fsi}`, not "the compiler". These are the files where a future contributor needs to see the lesson.

2. **Create or update an instruction file.** Check `.github/instructions/` for an existing instruction file whose `applyTo` covers those paths. If one exists, add a reference to your postmortem. If none exists, create one with an `applyTo` scoped to exactly those paths:

```yaml
---
applyTo:
- "src/Compiler/Path/To/File.{fs,fsi}"
---
```

The instruction file should encode the **generalized rule** (not the incident details). Link the postmortem as a "see also" for deeper context. The postmortem explains *why the rule exists*; the instruction file tells agents *what to do* when editing those files.

3. **Do not create instructions without path scoping.** A postmortem lesson that applies "everywhere" is too vague to be actionable. If you can't name the files where the lesson matters, the postmortem may not meet the threshold for this skill.

4. **Update `docs/postmortems/README.md`** if it maintains an index.
20 changes: 20 additions & 0 deletions azure-pipelines-PR.yml
Original file line number Diff line number Diff line change
Expand Up @@ -919,3 +919,23 @@ stages:
buildScript: build.sh
displayName: FsharpPlus_Net10_Linux
useVmImage: $(UbuntuMachineQueueName)
- repo: TheAngryByrd/IcedTasks
commit: 863bf91cdee93d8c4c875bb5d321dd92eb20d5a9
buildScript: dotnet build IcedTasks.sln
displayName: IcedTasks_Build
- repo: TheAngryByrd/IcedTasks
commit: 863bf91cdee93d8c4c875bb5d321dd92eb20d5a9
buildScript: dotnet test IcedTasks.sln
displayName: IcedTasks_Test
- repo: demystifyfp/FsToolkit.ErrorHandling
commit: 9cd957e335767df03e2fb0aa2f7b0fed782c5091
buildScript: dotnet build FsToolkit.ErrorHandling.sln
displayName: FsToolkit_ErrorHandling_Build
- repo: demystifyfp/FsToolkit.ErrorHandling
commit: 9cd957e335767df03e2fb0aa2f7b0fed782c5091
buildScript: dotnet test FsToolkit.ErrorHandling.sln
displayName: FsToolkit_ErrorHandling_Test
- repo: opentk/opentk
commit: 60c20cca65a7df6e8335e8d6060d91b30909fbea
buildScript: dotnet build tests/OpenTK.Tests/OpenTK.Tests.fsproj -c Release ;; dotnet build tests/OpenTK.Tests.Integration/OpenTK.Tests.Integration.fsproj -c Release
displayName: OpenTK_FSharp_Build
5 changes: 5 additions & 0 deletions docs/postmortems/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Postmortems

Detailed write-ups of bugs that were hard to diagnose, had non-obvious root causes, or taught us something worth preserving. Each document captures the symptoms, root cause, fix, and timeline so that future contributors can recognize similar patterns early.

These are referenced from [agentic instructions](../../.github/instructions/) and serve as deeper reading — the instructions tell you *what* to do, the postmortems explain *why* the rules exist.
140 changes: 140 additions & 0 deletions docs/postmortems/regression-fs0229-bstream-misalignment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
# Regression: FS0229 B-Stream Misalignment in TypedTreePickle

## Summary

A metadata unpickling regression causes `FS0229` errors when the F# compiler (post-nullness-checking merge) reads metadata from assemblies compiled with `LangVersion < 9.0`. The root cause is a stream alignment bug in `TypedTreePickle.fs` where the secondary metadata stream ("B-stream") gets out of sync between writer and reader.

## Error Manifestation

```
error FS0229: Error reading/writing metadata for assembly '<AssemblyName>':
The data read from the stream is inconsistent, reading past end of resource,
u_ty - 4/B OR u_ty - 1/B, byte = <N>
```

This error occurs when consuming metadata from an assembly where:
1. The assembly was compiled by the current compiler (which writes B-stream data)
2. The compilation used `LangVersion 8.0` or earlier (which disables `langFeatureNullness`)
3. The assembly references BCL types whose type parameters carry `NotSupportsNull` or `AllowsRefStruct` constraints

**Affected real-world library**: [FsToolkit.ErrorHandling](https://github.com/demystifyfp/FsToolkit.ErrorHandling), which uses `<LangVersion>8.0</LangVersion>` for `netstandard2.0`/`netstandard2.1` TFMs.

## Root Cause

### Dual-Stream Metadata Format

F# compiler metadata uses two serialization streams:
- **Stream A** (main): Type tags, type constructor references, type parameter references, etc.
- **Stream B** (secondary): Nullness information per type + newer constraint data (e.g., `NotSupportsNull`, `AllowsRefStruct`)

These streams are written in parallel during pickling and read in parallel during unpickling. The invariant is: **every byte written to stream B by the writer must have a corresponding read in the reader**.

### The Bug

In `p_ty2` (the type pickle function), nullness information is written to stream B **conditionally**:

```fsharp
// BEFORE FIX (buggy)
| TType_app(tc, tinst, nullness) ->
if st.oglobals.langFeatureNullness then
match nullness.Evaluate() with
| NullnessInfo.WithNull -> p_byteB 12 st
| NullnessInfo.WithoutNull -> p_byteB 13 st
| NullnessInfo.AmbivalentToNull -> p_byteB 14 st
// No else branch - B-stream byte skipped when langFeatureNullness = false!
p_byte 2 st
p_tcref "typ" tc st
p_tys tinst st
```

But in `u_ty` (the type unpickle function), the B-stream byte is read **unconditionally**:

```fsharp
| 2 ->
let tagB = u_byteB st // Always reads, regardless of langFeatureNullness at compile time
let tcref = u_tcref st
let tinst = u_tys st
match tagB with
| 0 -> TType_app(tcref, tinst, KnownAmbivalentToNull)
| 12 -> TType_app(tcref, tinst, KnownWithNull)
...
```

This affects type tags 1 (TType_app no args), 2 (TType_app), 3 (TType_fun), and 4 (TType_var).

Meanwhile, `p_tyar_constraints` **unconditionally** writes constraint data to B-stream:

```fsharp
let p_tyar_constraints cxs st =
let cxs1, cxs2 = cxs |> List.partition (function
| TyparConstraint.NotSupportsNull _ | TyparConstraint.AllowsRefStruct _ -> false
| _ -> true)
p_list p_tyar_constraint cxs1 st
p_listB p_tyar_constraintB cxs2 st // Always writes to B, regardless of langFeatureNullness
```

### Misalignment Cascade

When `langFeatureNullness = false`:

1. Writer processes types → skips B-bytes for each type tag 1-4
2. Writer processes type parameter constraints → writes `NotSupportsNull` data to B-stream (value `0x01`)
3. Reader processes types → reads B-stream expecting nullness tags → gets constraint data instead
4. Constraint byte `0x01` is not a valid nullness tag (valid values: 0, 9-20) → `ufailwith "u_ty - 4/B"` or similar

The misalignment cascades: once one byte is read from the wrong position, all subsequent B-stream reads are shifted.

## Fix

Added `else p_byteB 0 st` to all four type cases in `p_ty2`, ensuring a B-byte is always written regardless of `langFeatureNullness`:

```fsharp
// AFTER FIX
| TType_app(tc, tinst, nullness) ->
if st.oglobals.langFeatureNullness then
match nullness.Evaluate() with
| NullnessInfo.WithNull -> p_byteB 12 st
| NullnessInfo.WithoutNull -> p_byteB 13 st
| NullnessInfo.AmbivalentToNull -> p_byteB 14 st
else
p_byteB 0 st // Keep B-stream aligned
p_byte 2 st
p_tcref "typ" tc st
p_tys tinst st
```

Value `0` means "no nullness info / AmbivalentToNull" and is already handled by all reader match cases.

## Timeline

| Date | PR | Change |
|------|-----|--------|
| Jul 2024 | [#15181](https://github.com/dotnet/fsharp/pull/15181) | Nullness checking: introduced B-stream for nullness bytes, conditional write in `p_ty2` |
| Aug 2024 | [#15310](https://github.com/dotnet/fsharp/pull/15310) | Nullness checking applied to codebase |
| Sep 2024 | [#17706](https://github.com/dotnet/fsharp/pull/17706) | `AllowsRefStruct`: added constraint data to B-stream unconditionally via `p_listB` |

The bug was latent from #15181 but only manifested when #17706 added unconditional B-stream writes for constraints. Before #17706, the B-stream was empty when `langFeatureNullness = false`, so the reader's unconditional reads would hit the end-of-stream sentinel (returning 0) harmlessly. After #17706, constraint data appeared in the B-stream even without nullness, causing the misalignment.

## Regression Tests

Two tests added in `tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs`:

1. **Basic test**: Compiles a library with `LangVersion=8.0` containing generic types with BCL constraints (e.g., `IEquatable<'T>`), then references it from another compilation and verifies no FS0229 error.

2. **Stress test**: Multiple type parameters with various constraint patterns, function types, and nested generics — all compiled at `LangVersion=8.0` and successfully consumed.

## Reproduction

To reproduce the original bug (before fix):

1. Clone [FsToolkit.ErrorHandling](https://github.com/demystifyfp/FsToolkit.ErrorHandling)
2. Inject the pre-fix compiler via `UseLocalCompiler.Directory.Build.props`
3. Build `netstandard2.0` TFM (uses `LangVersion=8.0`)
4. Build `net9.0` TFM that references the `netstandard2.0` output
5. The `net9.0` build fails with `FS0229: u_ty - 4/B`

## Files Changed

- `src/Compiler/TypedTree/TypedTreePickle.fs` — Added `else p_byteB 0 st` to four locations in `p_ty2`
- `tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs` — Two regression tests
- `tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj` — Added `ImportTests.fs` include (was missing since migration)
Loading
Loading