From 2c5ac7fe8c40e492bd0cf06527906809103c2556 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 3 Feb 2026 11:36:45 +0100 Subject: [PATCH 01/20] Update azure-pipelines-PR.yml --- azure-pipelines-PR.yml | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index 830df465998..793a33b3ecd 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -919,3 +919,19 @@ stages: buildScript: build.sh displayName: FsharpPlus_Net10_Linux useVmImage: $(UbuntuMachineQueueName) + - repo: TheAngryByrd/IcedTasks + commit: 863bf91cdee93d8c4c875bb5d321dd92eb20d5a9 + buildScript: dotnet build src/IcedTasks/IcedTasks.fsproj -c Release + displayName: IcedTasks_Build + - repo: TheAngryByrd/IcedTasks + commit: 863bf91cdee93d8c4c875bb5d321dd92eb20d5a9 + buildScript: dotnet test tests/IcedTasks.Tests/IcedTasks.Tests.fsproj -c Release + displayName: IcedTasks_Test + - repo: demystifyfp/FsToolkit.ErrorHandling + commit: 9cd957e335767df03e2fb0aa2f7b0fed782c5091 + buildScript: dotnet build FsToolkit.ErrorHandling.sln -c Release + displayName: FsToolkit_ErrorHandling_Build + - repo: demystifyfp/FsToolkit.ErrorHandling + commit: 9cd957e335767df03e2fb0aa2f7b0fed782c5091 + buildScript: dotnet test tests/FsToolkit.ErrorHandling.Tests/FsToolkit.ErrorHandling.Tests.fsproj -c Release + displayName: FsToolkit_ErrorHandling_Test From b18d0e26523955b07db510fa2e8eb9518ae38529 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Fri, 6 Feb 2026 20:29:52 +0100 Subject: [PATCH 02/20] Update azure-pipelines-PR.yml --- azure-pipelines-PR.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index 793a33b3ecd..b2e437d4e1b 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -921,17 +921,17 @@ stages: useVmImage: $(UbuntuMachineQueueName) - repo: TheAngryByrd/IcedTasks commit: 863bf91cdee93d8c4c875bb5d321dd92eb20d5a9 - buildScript: dotnet build src/IcedTasks/IcedTasks.fsproj -c Release + buildScript: build.cmd DotnetBuild displayName: IcedTasks_Build - repo: TheAngryByrd/IcedTasks commit: 863bf91cdee93d8c4c875bb5d321dd92eb20d5a9 - buildScript: dotnet test tests/IcedTasks.Tests/IcedTasks.Tests.fsproj -c Release + buildScript: build.cmd DotnetTest displayName: IcedTasks_Test - repo: demystifyfp/FsToolkit.ErrorHandling commit: 9cd957e335767df03e2fb0aa2f7b0fed782c5091 - buildScript: dotnet build FsToolkit.ErrorHandling.sln -c Release + buildScript: build.cmd Build displayName: FsToolkit_ErrorHandling_Build - repo: demystifyfp/FsToolkit.ErrorHandling commit: 9cd957e335767df03e2fb0aa2f7b0fed782c5091 - buildScript: dotnet test tests/FsToolkit.ErrorHandling.Tests/FsToolkit.ErrorHandling.Tests.fsproj -c Release + buildScript: build.cmd DotnetTest displayName: FsToolkit_ErrorHandling_Test From 6c34e210e565ec2a55d69113d91da2baed1f7060 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Fri, 6 Feb 2026 20:41:43 +0100 Subject: [PATCH 03/20] add opentk --- azure-pipelines-PR.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index b2e437d4e1b..1a034520ded 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -935,3 +935,7 @@ stages: commit: 9cd957e335767df03e2fb0aa2f7b0fed782c5091 buildScript: build.cmd DotnetTest displayName: FsToolkit_ErrorHandling_Test + - repo: opentk/opentk + commit: 60c20cca65a7df6e8335e8d6060d91b30909fbea + buildScript: dotnet build tests/OpenTK.Tests/OpenTK.Tests.fsproj tests/OpenTK.Tests.Integration/OpenTK.Tests.Integration.fsproj -c Release + displayName: OpenTK_FSharp_Build From c472cf1bd293a13da0d5b1a3daa1a9eb38479806 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 14:26:00 +0100 Subject: [PATCH 04/20] Fix: replace '.' in repo names for valid AzDO job identifiers The repo 'demystifyfp/FsToolkit.ErrorHandling' contains a period which is not allowed in Azure DevOps job identifiers (alphanumeric + underscore only). This caused YAML validation failure preventing the entire pipeline from starting. Add an additional replace() call for '.' -> '_'. --- eng/templates/regression-test-jobs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/templates/regression-test-jobs.yml b/eng/templates/regression-test-jobs.yml index 66c184138aa..30ad1456b5c 100644 --- a/eng/templates/regression-test-jobs.yml +++ b/eng/templates/regression-test-jobs.yml @@ -10,7 +10,7 @@ parameters: jobs: - ${{ each item in parameters.testMatrix }}: - - job: RegressionTest_${{ replace(item.displayName, '-', '_') }}_${{ replace(replace(item.repo, '/', '_'), '-', '_') }} + - job: RegressionTest_${{ replace(replace(item.displayName, '-', '_'), '.', '_') }}_${{ replace(replace(replace(item.repo, '/', '_'), '-', '_'), '.', '_') }} displayName: '${{ item.displayName }} Regression Test' dependsOn: ${{ parameters.dependsOn }} ${{ if item.useVmImage }}: From 2a0dcfd6c4ea2b1eb12f68d7131b792028409754 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 15:09:37 +0100 Subject: [PATCH 05/20] Fix regression test template for buildScript with arguments Three fixes: 1. Checkout step: split buildScript to check only the file name (first token), not the entire string including arguments 2. Build step: use 'cmd /c' on Windows for file-based scripts to handle arguments naturally; on Linux, chmod only the script file 3. OpenTK: use single project in dotnet build (MSB1008 error with multiple projects) --- azure-pipelines-PR.yml | 2 +- eng/templates/regression-test-jobs.yml | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index 1a034520ded..244cac7b1c4 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -937,5 +937,5 @@ stages: displayName: FsToolkit_ErrorHandling_Test - repo: opentk/opentk commit: 60c20cca65a7df6e8335e8d6060d91b30909fbea - buildScript: dotnet build tests/OpenTK.Tests/OpenTK.Tests.fsproj tests/OpenTK.Tests.Integration/OpenTK.Tests.Integration.fsproj -c Release + buildScript: dotnet build tests/OpenTK.Tests/OpenTK.Tests.fsproj -c Release displayName: OpenTK_FSharp_Build diff --git a/eng/templates/regression-test-jobs.yml b/eng/templates/regression-test-jobs.yml index 30ad1456b5c..d5b19149f2a 100644 --- a/eng/templates/regression-test-jobs.yml +++ b/eng/templates/regression-test-jobs.yml @@ -65,11 +65,13 @@ jobs: Write-Host "Build command is a built-in dotnet command: $buildScript" Write-Host "Skipping file existence check for built-in command" } else { - Write-Host "Verifying build script exists: $buildScript" - if (Test-Path $buildScript) { - Write-Host "Build script found: $buildScript" + # Extract just the script file name (first token) for existence check + $scriptFile = ($buildScript -split ' ', 2)[0] + Write-Host "Verifying build script exists: $scriptFile" + if (Test-Path $scriptFile) { + Write-Host "Build script found: $scriptFile" } else { - Write-Host "Build script not found: $buildScript" + Write-Host "Build script not found: $scriptFile" Write-Host "Available files in root:" Get-ChildItem exit 1 @@ -182,12 +184,13 @@ jobs: } } elseif ($IsWindows) { Write-Host "Executing file-based script: $buildScript" - & ".\$buildScript" 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { + cmd /c ".\$buildScript" 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { Process-BuildOutput $_ } } else { Write-Host "Executing file-based script: $buildScript" - chmod +x "$buildScript" + $scriptFile = ($buildScript -split ' ', 2)[0] + chmod +x "$scriptFile" bash -c "./$buildScript" 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { Process-BuildOutput $_ } From ac6d7535f4d1347ff4920338bf7aa6365cc98bb9 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 15:57:52 +0100 Subject: [PATCH 06/20] Fix regression test failures: add .NET 9.0 SDK, use direct dotnet commands 1. Add .NET 9.0.x SDK installation - IcedTasks' FAKE build project targets net9.0 and fails without the runtime 2. Switch IcedTasks and FsToolkit from FAKE build.cmd to direct dotnet build/test - FAKE's build.exe locks itself causing MSB3027 file lock errors and cascading FS0229 metadata read failures 3. Keep .NET 9.0 SDK for repos that may need it --- azure-pipelines-PR.yml | 8 ++++---- eng/templates/regression-test-jobs.yml | 7 +++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index 244cac7b1c4..535339e8304 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -921,19 +921,19 @@ stages: useVmImage: $(UbuntuMachineQueueName) - repo: TheAngryByrd/IcedTasks commit: 863bf91cdee93d8c4c875bb5d321dd92eb20d5a9 - buildScript: build.cmd DotnetBuild + buildScript: dotnet build IcedTasks.sln displayName: IcedTasks_Build - repo: TheAngryByrd/IcedTasks commit: 863bf91cdee93d8c4c875bb5d321dd92eb20d5a9 - buildScript: build.cmd DotnetTest + buildScript: dotnet test IcedTasks.sln displayName: IcedTasks_Test - repo: demystifyfp/FsToolkit.ErrorHandling commit: 9cd957e335767df03e2fb0aa2f7b0fed782c5091 - buildScript: build.cmd Build + buildScript: dotnet build FsToolkit.ErrorHandling.sln displayName: FsToolkit_ErrorHandling_Build - repo: demystifyfp/FsToolkit.ErrorHandling commit: 9cd957e335767df03e2fb0aa2f7b0fed782c5091 - buildScript: build.cmd DotnetTest + buildScript: dotnet test FsToolkit.ErrorHandling.sln displayName: FsToolkit_ErrorHandling_Test - repo: opentk/opentk commit: 60c20cca65a7df6e8335e8d6060d91b30909fbea diff --git a/eng/templates/regression-test-jobs.yml b/eng/templates/regression-test-jobs.yml index d5b19149f2a..a8da2bb6439 100644 --- a/eng/templates/regression-test-jobs.yml +++ b/eng/templates/regression-test-jobs.yml @@ -97,6 +97,13 @@ jobs: version: '8.0.x' installationPath: $(Pipeline.Workspace)/TestRepo/.dotnet + - task: UseDotNet@2 + displayName: Install .NET SDK 9.0.x for ${{ item.displayName }} + inputs: + packageType: sdk + version: '9.0.x' + installationPath: $(Pipeline.Workspace)/TestRepo/.dotnet + - task: UseDotNet@2 displayName: Install .NET SDK 10.0.100 for ${{ item.displayName }} inputs: From 037e7b06a5762201f45e20c9a2c408fc6eb50cec Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 16:40:33 +0100 Subject: [PATCH 07/20] Add dotnet tool restore step before build IcedTasks' Directory.Build.targets runs 'dotnet tool restore' during build. When MSBuild builds projects in parallel, concurrent tool restores fight over NuGet package files causing file lock errors. Pre-restoring tools eliminates the race condition. --- eng/templates/regression-test-jobs.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/eng/templates/regression-test-jobs.yml b/eng/templates/regression-test-jobs.yml index a8da2bb6439..423ebe4e635 100644 --- a/eng/templates/regression-test-jobs.yml +++ b/eng/templates/regression-test-jobs.yml @@ -149,6 +149,18 @@ jobs: Write-Host "===========================================" displayName: Report build environment for ${{ item.displayName }} + - pwsh: | + Set-Location $(Pipeline.Workspace)/TestRepo + if (Test-Path ".config/dotnet-tools.json") { + Write-Host "Restoring dotnet tools..." + dotnet tool restore + Write-Host "Tool restore exit code: $LASTEXITCODE" + } else { + Write-Host "No dotnet-tools.json found, skipping tool restore" + } + displayName: Restore dotnet tools for ${{ item.displayName }} + continueOnError: true + - pwsh: | Set-Location $(Pipeline.Workspace)/TestRepo Write-Host "============================================" From bb026d477bc7db3284be3d9b874928a35393a02d Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 18:03:17 +0100 Subject: [PATCH 08/20] Split OpenTK into two matrix entries to test both projects --- azure-pipelines-PR.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index 535339e8304..7d79e759fab 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -938,4 +938,8 @@ stages: - repo: opentk/opentk commit: 60c20cca65a7df6e8335e8d6060d91b30909fbea buildScript: dotnet build tests/OpenTK.Tests/OpenTK.Tests.fsproj -c Release - displayName: OpenTK_FSharp_Build + displayName: OpenTK_Tests_Build + - repo: opentk/opentk + commit: 60c20cca65a7df6e8335e8d6060d91b30909fbea + buildScript: dotnet build tests/OpenTK.Tests.Integration/OpenTK.Tests.Integration.fsproj -c Release + displayName: OpenTK_Integration_Build From 93c31cbab4e2c32369e2e1eaab22c7ce3f3c251d Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 18:39:04 +0100 Subject: [PATCH 09/20] Fix FS3356 false positive: only check static extension members for duplicate type name conflict Instance extension members compile with the extended type as the first IL parameter, so they can never produce duplicate IL method signatures even when the simple type name matches. Only static extension members lack this distinguishing parameter. This fixes a regression where libraries like IcedTasks that define instance (inline) extension members on builder types from different namespaces (e.g. Microsoft.FSharp.Control.TaskBuilderBase and IcedTasks.TaskBase.TaskBuilderBase) were incorrectly rejected. Regression introduced by PR #18821 (commit e948a688). --- src/Compiler/Checking/PostInferenceChecks.fs | 20 ++++--- .../Language/ExtensionMethodTests.fs | 54 +++++++++++++++++-- 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/src/Compiler/Checking/PostInferenceChecks.fs b/src/Compiler/Checking/PostInferenceChecks.fs index d3854d1ce27..e59a80b5cdf 100644 --- a/src/Compiler/Checking/PostInferenceChecks.fs +++ b/src/Compiler/Checking/PostInferenceChecks.fs @@ -2680,21 +2680,27 @@ let CheckEntityDefns cenv env tycons = // check modules //-------------------------------------------------------------------------- -/// Check for duplicate extension member names that would cause IL conflicts. -/// Extension members for types with the same simple name but different fully qualified names -/// will be emitted into the same IL container type, causing a duplicate member error. +/// Check for duplicate static extension member names that would cause IL conflicts. +/// Static extension members for types with the same simple name but different fully qualified names +/// compile to static methods in the same module IL type without a distinguishing first parameter, +/// so they can produce duplicate IL method signatures. +/// Instance extension members are safe because they compile with the extended type as the first +/// parameter, which differentiates the IL signatures. let CheckForDuplicateExtensionMemberNames (cenv: cenv) (vals: Val seq) = if cenv.reportErrors then - let extensionMembers = + let staticExtensionMembers = vals - |> Seq.filter (fun v -> v.IsExtensionMember && v.IsMember) + |> Seq.filter (fun v -> + v.IsExtensionMember + && v.IsMember + && not (v.IsInstanceMember)) |> Seq.toList - if not extensionMembers.IsEmpty then + if not staticExtensionMembers.IsEmpty then // Group by LogicalName which includes generic arity suffix (e.g., Expr`1 for Expr<'T>) // This matches how types are compiled to IL, so Expr and Expr<'T> are separate groups let groupedByLogicalName = - extensionMembers + staticExtensionMembers |> List.groupBy (fun v -> v.MemberApparentEntity.LogicalName) for (logicalName, members) in groupedByLogicalName do diff --git a/tests/FSharp.Compiler.ComponentTests/Language/ExtensionMethodTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/ExtensionMethodTests.fs index 956362312f8..3eb711097c2 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/ExtensionMethodTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/ExtensionMethodTests.fs @@ -697,7 +697,9 @@ module CompiledExtensions = ] [] - let ``Instance extension members for types with same simple name should error`` () = + let ``Instance extension members for types with same simple name should succeed`` () = + // Instance extension members compile with the extended type as the first IL parameter, + // so they can never produce duplicate IL signatures even with same simple type name. Fsx """ module Compiled @@ -712,10 +714,7 @@ module CompiledExtensions = member _.InstanceExtension() = () """ |> compile - |> shouldFail - |> withDiagnostics [ - (Error 3356, Line 11, Col 18, Line 11, Col 35, "Extension members extending types with the same simple name 'Task' but different fully qualified names cannot be defined in the same module. Consider defining these extensions in separate modules.") - ] + |> shouldSucceed [] let ``Extension members on generic types with same simple name should error`` () = @@ -806,3 +805,48 @@ module CompiledExtensions = (Error 3356, Line 12, Col 23, Line 12, Col 33, "Extension members extending types with the same simple name 'Task' but different fully qualified names cannot be defined in the same module. Consider defining these extensions in separate modules.") (Error 3356, Line 13, Col 23, Line 13, Col 33, "Extension members extending types with the same simple name 'Task' but different fully qualified names cannot be defined in the same module. Consider defining these extensions in separate modules.") ] + + [] + let ``Instance inline extension members on builder types with same simple name should succeed`` () = + // Regression test for IcedTasks-like pattern: instance (inline) extension members on + // computation expression builder types with the same simple name from different namespaces. + // Instance extension members compile with the extended type as the first IL parameter, + // so signatures never collide even when the simple type name is the same. + Fsx + """ +namespace NsA +type BuilderBase() = class end + +namespace NsB +type BuilderBase() = class end + +namespace Extensions +module M = + type NsA.BuilderBase with + member inline this.Bind(x: int, f) = f x + member inline this.ReturnFrom(x: int) = x + + type NsB.BuilderBase with + member inline this.Source(x: int) = x + member inline this.Bind(x: string, f) = f x + """ + |> compile + |> shouldSucceed + + [] + let ``Mixed static and instance extension members - only static should error`` () = + Fsx + """ +module Compiled + +type Task = { F: int } + +module CompiledExtensions = + type System.Threading.Tasks.Task with + static member StaticExt() = () + + type Task with + member _.InstanceExt() = () + """ + |> compile + |> shouldSucceed From 204d2fdf19444650071b967d38076b7050276147 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 18:40:34 +0100 Subject: [PATCH 10/20] Support multi-command buildScript with ';;' separator The regression test template now supports multiple commands in a single matrix entry using ';;' as a separator. Commands run sequentially and the job fails on the first non-zero exit code. Example: buildScript: dotnet build A.fsproj ;; dotnet build B.fsproj Reverts OpenTK back to a single matrix entry with both test projects. --- azure-pipelines-PR.yml | 8 +-- eng/templates/regression-test-jobs.yml | 94 ++++++++++++++++---------- 2 files changed, 59 insertions(+), 43 deletions(-) diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index 7d79e759fab..507f6f79ffe 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -937,9 +937,5 @@ stages: displayName: FsToolkit_ErrorHandling_Test - repo: opentk/opentk commit: 60c20cca65a7df6e8335e8d6060d91b30909fbea - buildScript: dotnet build tests/OpenTK.Tests/OpenTK.Tests.fsproj -c Release - displayName: OpenTK_Tests_Build - - repo: opentk/opentk - commit: 60c20cca65a7df6e8335e8d6060d91b30909fbea - buildScript: dotnet build tests/OpenTK.Tests.Integration/OpenTK.Tests.Integration.fsproj -c Release - displayName: OpenTK_Integration_Build + 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 diff --git a/eng/templates/regression-test-jobs.yml b/eng/templates/regression-test-jobs.yml index 423ebe4e635..2032c351731 100644 --- a/eng/templates/regression-test-jobs.yml +++ b/eng/templates/regression-test-jobs.yml @@ -1,5 +1,10 @@ # Template for F# Compiler Regression Tests # Tests third-party F# projects with the freshly built compiler +# +# buildScript notation: +# - Single command: buildScript: dotnet build MySolution.sln +# - Multiple commands: buildScript: dotnet build A.fsproj ;; dotnet build B.fsproj +# Commands separated by ';;' run sequentially; the job fails on the first non-zero exit code. parameters: - name: testMatrix @@ -61,20 +66,22 @@ jobs: Get-ChildItem -Name $buildScript = '${{ item.buildScript }}' - if ($buildScript -like "dotnet*") { - Write-Host "Build command is a built-in dotnet command: $buildScript" - Write-Host "Skipping file existence check for built-in command" - } else { - # Extract just the script file name (first token) for existence check - $scriptFile = ($buildScript -split ' ', 2)[0] - Write-Host "Verifying build script exists: $scriptFile" - if (Test-Path $scriptFile) { - Write-Host "Build script found: $scriptFile" + # Support ';;' separator for multiple commands — validate each command's script file + $commands = $buildScript -split ';;' | ForEach-Object { $_.Trim() } | Where-Object { $_ } + foreach ($cmd in $commands) { + if ($cmd -like "dotnet*") { + Write-Host "Built-in dotnet command, skipping file check: $cmd" } else { - Write-Host "Build script not found: $scriptFile" - Write-Host "Available files in root:" - Get-ChildItem - exit 1 + $scriptFile = ($cmd -split ' ', 2)[0] + Write-Host "Verifying build script exists: $scriptFile" + if (Test-Path $scriptFile) { + Write-Host "Build script found: $scriptFile" + } else { + Write-Host "Build script not found: $scriptFile" + Write-Host "Available files in root:" + Get-ChildItem + exit 1 + } } } displayName: Checkout ${{ item.displayName }} at specific commit @@ -171,7 +178,6 @@ jobs: Write-Host "============================================" Write-Host "" - $buildScript = '${{ item.buildScript }}' $errorLogPath = "$(Pipeline.Workspace)/build-errors.log" $fullLogPath = "$(Pipeline.Workspace)/build-full.log" @@ -189,42 +195,56 @@ jobs: Add-Content -Path $errorLogPath -Value $line } } - - if ($buildScript -like "dotnet*") { - Write-Host "Executing built-in command: $buildScript" - if ($IsWindows) { - cmd /c $buildScript 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { + + function Run-Command { + param([string]$cmd) + if ($cmd -like "dotnet*") { + Write-Host "Executing built-in command: $cmd" + if ($IsWindows) { + cmd /c $cmd 2>&1 | Tee-Object -FilePath $fullLogPath -Append | ForEach-Object { + Process-BuildOutput $_ + } + } else { + bash -c "$cmd" 2>&1 | Tee-Object -FilePath $fullLogPath -Append | ForEach-Object { + Process-BuildOutput $_ + } + } + } elseif ($IsWindows) { + Write-Host "Executing file-based script: $cmd" + cmd /c ".\$cmd" 2>&1 | Tee-Object -FilePath $fullLogPath -Append | ForEach-Object { Process-BuildOutput $_ } } else { - bash -c "$buildScript" 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { + Write-Host "Executing file-based script: $cmd" + $scriptFile = ($cmd -split ' ', 2)[0] + chmod +x "$scriptFile" + bash -c "./$cmd" 2>&1 | Tee-Object -FilePath $fullLogPath -Append | ForEach-Object { Process-BuildOutput $_ } } - } elseif ($IsWindows) { - Write-Host "Executing file-based script: $buildScript" - cmd /c ".\$buildScript" 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { - Process-BuildOutput $_ - } - } else { - Write-Host "Executing file-based script: $buildScript" - $scriptFile = ($buildScript -split ' ', 2)[0] - chmod +x "$scriptFile" - bash -c "./$buildScript" 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { - Process-BuildOutput $_ + return $LASTEXITCODE + } + + # Support ';;' separator for multiple commands + $commands = ('${{ item.buildScript }}' -split ';;') | ForEach-Object { $_.Trim() } | Where-Object { $_ } + + foreach ($cmd in $commands) { + $exitCode = Run-Command $cmd + if ($exitCode -ne 0) { + Write-Host "" + Write-Host "============================================" + Write-Host "Command failed: $cmd" + Write-Host "Exit code: $exitCode" + Write-Host "============================================" + exit $exitCode } } - $exitCode = $LASTEXITCODE Write-Host "" Write-Host "============================================" Write-Host "Build completed for ${{ item.displayName }}" - Write-Host "Exit code: $exitCode" + Write-Host "Exit code: 0" Write-Host "============================================" - - if ($exitCode -ne 0) { - exit $exitCode - } displayName: Build ${{ item.displayName }} with local F# compiler env: LocalFSharpCompilerPath: $(Pipeline.Workspace)/FSharpCompiler From 85f39306ade3f969a1608bad479652e644aff190 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 19:04:12 +0100 Subject: [PATCH 11/20] Fix FS0229: B-stream misalignment in TypedTreePickle when langFeatureNullness disabled When langFeatureNullness is false, p_ty2 conditionally skipped writing nullness B-bytes for type tags 1-4, but u_ty unconditionally reads them. This caused B-stream misalignment when constraint data (e.g. NotSupportsNull from BCL types) was also written to B-stream, leading to FS0229 errors when consuming metadata. Fix: Always write a B-byte (0 = AmbivalentToNull) for each type tag, regardless of langFeatureNullness. This keeps streams aligned. Also adds ImportTests.fs to the test project (was missing since migration) and adds two regression tests for the B-stream misalignment scenario. --- src/Compiler/TypedTree/TypedTreePickle.fs | 11 ++ .../FSharp.Compiler.ComponentTests.fsproj | 1 + .../Import/ImportTests.fs | 108 ++++++++++++++++++ 3 files changed, 120 insertions(+) diff --git a/src/Compiler/TypedTree/TypedTreePickle.fs b/src/Compiler/TypedTree/TypedTreePickle.fs index 8a61809ab06..81deb4e2037 100644 --- a/src/Compiler/TypedTree/TypedTreePickle.fs +++ b/src/Compiler/TypedTree/TypedTreePickle.fs @@ -2437,11 +2437,16 @@ let _ = p_tys l st | TType_app(ERefNonLocal nleref, [], nullness) -> + // Always write nullness byte to B stream to keep it aligned with unconditional reads in u_ty. + // Other data (e.g. typar constraints) is also written to stream B unconditionally, + // so skipping nullness bytes would cause stream B misalignment when langFeatureNullness = false. if st.oglobals.langFeatureNullness then match nullness.Evaluate() with | NullnessInfo.WithNull -> p_byteB 9 st | NullnessInfo.WithoutNull -> p_byteB 10 st | NullnessInfo.AmbivalentToNull -> p_byteB 11 st + else + p_byteB 0 st p_byte 1 st p_simpletyp nleref st @@ -2452,6 +2457,8 @@ let _ = | NullnessInfo.WithNull -> p_byteB 12 st | NullnessInfo.WithoutNull -> p_byteB 13 st | NullnessInfo.AmbivalentToNull -> p_byteB 14 st + else + p_byteB 0 st p_byte 2 st p_tcref "typ" tc st @@ -2463,6 +2470,8 @@ let _ = | NullnessInfo.WithNull -> p_byteB 15 st | NullnessInfo.WithoutNull -> p_byteB 16 st | NullnessInfo.AmbivalentToNull -> p_byteB 17 st + else + p_byteB 0 st p_byte 3 st // Note, the "this" argument may be found in the domain position of a function type, so propagate the isStructThisArgPos value @@ -2475,6 +2484,8 @@ let _ = | NullnessInfo.WithNull -> p_byteB 18 st | NullnessInfo.WithoutNull -> p_byteB 19 st | NullnessInfo.AmbivalentToNull -> p_byteB 20 st + else + p_byteB 0 st p_byte 4 st p_tpref r st diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index f236ca6599d..63150ad3a7d 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -267,6 +267,7 @@ + diff --git a/tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs b/tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs index 8bd3625faf3..b0611adee03 100644 --- a/tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs @@ -935,3 +935,111 @@ let v = new y() |> compile |> shouldSucceed |> ignore + + // Regression test: B-stream misalignment when pickling metadata with langFeatureNullness=false + // When a library is compiled with LangVersion 8.0 (no nullness), the type nullness B-stream bytes + // must still be written to keep the stream aligned with unconditional reads and constraint data. + // Without the fix, the reader hits NotSupportsNull constraint bytes (0x01) when expecting type + // nullness values, causing FS0229 "u_ty - 4/B". + [] + let ``Referencing library compiled with LangVersion 8 should not produce FS0229 B-stream error`` () = + let fsLib = + FSharp """ +module LibA + +type Result<'T, 'E> = + | Ok of 'T + | Error of 'E + +let bind (f: 'T -> Result<'U, 'E>) (r: Result<'T, 'E>) : Result<'U, 'E> = + match r with + | Ok x -> f x + | Error e -> Error e + +let map (f: 'T -> 'U) (r: Result<'T, 'E>) : Result<'U, 'E> = + bind (f >> Ok) r + +type Builder() = + member _.Return(x: 'T) : Result<'T, 'E> = Ok x + member _.ReturnFrom(x: Result<'T, 'E>) : Result<'T, 'E> = x + member _.Bind(m: Result<'T, 'E>, f: 'T -> Result<'U, 'E>) : Result<'U, 'E> = bind f m + +let builder = Builder() + +let inline combine<'T, 'U, 'E when 'T : equality and 'U : comparison> + (r1: Result<'T, 'E>) (r2: Result<'U, 'E>) : Result<'T * 'U, 'E> = + match r1, r2 with + | Ok t, Ok u -> Ok(t, u) + | Error e, _ | _, Error e -> Error e + """ + |> withName "LibA" + |> asLibrary + |> withLangVersion80 + + FSharp """ +module LibB + +open LibA + +let test() = + let r1 = Ok 42 + let r2 = Ok "hello" + combine r1 r2 + """ + |> asLibrary + |> withReferences [fsLib] + |> compile + |> shouldSucceed + |> ignore + + [] + let ``Referencing library with many generic types compiled at LangVersion 8 should not produce FS0229`` () = + let fsLib = + FSharp """ +module Lib + +type Wrapper<'T> = { Value: 'T } +type Pair<'T, 'U> = { First: 'T; Second: 'U } +type Triple<'T, 'U, 'V> = { A: 'T; B: 'U; C: 'V } + +let wrap (x: 'T) : Wrapper<'T> = { Value = x } +let pair (x: 'T) (y: 'U) : Pair<'T, 'U> = { First = x; Second = y } +let triple (x: 'T) (y: 'U) (z: 'V) : Triple<'T, 'U, 'V> = { A = x; B = y; C = z } + +let mapWrapper (f: 'T -> 'U) (w: Wrapper<'T>) : Wrapper<'U> = { Value = f w.Value } +let mapPair (f: 'T -> 'T2) (g: 'U -> 'U2) (p: Pair<'T, 'U>) : Pair<'T2, 'U2> = + { First = f p.First; Second = g p.Second } + +type ChainBuilder() = + member _.Return(x: 'T) : Wrapper<'T> = wrap x + member _.Bind(m: Wrapper<'T>, f: 'T -> Wrapper<'U>) : Wrapper<'U> = f m.Value + +let chain = ChainBuilder() + +let inline constrained<'T when 'T : equality> (x: 'T) (y: 'T) = x = y +let inline constrained2<'T, 'U when 'T : equality and 'U : comparison> (x: 'T) (y: 'U) = + (x, y) + """ + |> withName "GenericLib" + |> asLibrary + |> withLangVersion80 + + FSharp """ +module Consumer + +open Lib + +let test() = + let w = wrap 42 + let mapped = mapWrapper (fun x -> x + 1) w + let p = pair "hello" 42 + let t = triple 1 "two" 3.0 + let eq = constrained 1 1 + let c = chain { return 42 } + (mapped, p, t, eq, c) + """ + |> asLibrary + |> withReferences [fsLib] + |> compile + |> shouldSucceed + |> ignore From 2a623985db96902362a51610d449b9f2c881b7bb Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 19:05:07 +0100 Subject: [PATCH 12/20] Add documentation for FS0229 B-stream misalignment regression --- .../regression-fs0229-bstream-misalignment.md | 140 ++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 docs/regression-fs0229-bstream-misalignment.md diff --git a/docs/regression-fs0229-bstream-misalignment.md b/docs/regression-fs0229-bstream-misalignment.md new file mode 100644 index 00000000000..0711b6feb0a --- /dev/null +++ b/docs/regression-fs0229-bstream-misalignment.md @@ -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 '': + The data read from the stream is inconsistent, reading past end of resource, + u_ty - 4/B OR u_ty - 1/B, byte = +``` + +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 `8.0` 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) From 4806039ac28b1fa4ffc9ed69257c2154437512b5 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 19:09:58 +0100 Subject: [PATCH 13/20] Add release notes for FS0229 and FS3356 fixes --- docs/release-notes/.FSharp.Compiler.Service/10.0.300.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md index c247da5870b..4eddd5c7aef 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md @@ -1,5 +1,8 @@ ### Fixed +* Fix FS0229 B-stream misalignment when reading metadata from assemblies compiled with LangVersion < 9.0. ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) +* Fix FS3356 false positive for instance extension members with same name on different types. ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) + ### Added ### Changed From 5561c9952525091f63f868975fde4f24d3178391 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 21:12:15 +0100 Subject: [PATCH 14/20] Fix FS3356 tests: use static members for IL collision tests, allow instance members The duplicate extension member check (FS3356) only applies to static extension members because they lack the extended type as a first IL parameter, causing signature collisions. Instance extension members are safe because the extended type differentiates the IL signatures. Updated the existing tests to use static members (the actual IL collision case) and added companion tests showing instance members on same-named types are allowed. --- .../DuplicateExtensionMemberTests.fs | 64 +++++++++++++++++-- 1 file changed, 58 insertions(+), 6 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/DuplicateExtensionMemberTests.fs b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/DuplicateExtensionMemberTests.fs index dd53e92951a..c0021c55ebc 100644 --- a/tests/FSharp.Compiler.ComponentTests/ErrorMessages/DuplicateExtensionMemberTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/ErrorMessages/DuplicateExtensionMemberTests.fs @@ -8,7 +8,10 @@ open FSharp.Test.Compiler module ``Duplicate Extension Members`` = [] - let ``Same type name from different namespaces should error``() = + let ``Same type name from different namespaces should error for static members``() = + // Static extension members on types with same simple name but different namespaces + // produce duplicate IL method signatures because the extended type's namespace is not + // encoded in the IL method name/signature for static extensions. FSharp """ namespace NS1 @@ -22,15 +25,40 @@ namespace NS3 module Extensions = type NS1.Task with - member x.Foo() = 1 + static member Foo() = 1 type NS2.Task with - member x.Bar() = 2 + static member Bar() = 2 """ |> typecheck |> shouldFail |> withDiagnosticMessageMatches "Extension members extending types with the same simple name 'Task'" + [] + let ``Same type name from different namespaces should be allowed for instance members``() = + // Instance extension members are safe because the extended type becomes the first + // parameter in IL, differentiating the signatures. + FSharp """ +namespace NS1 + +type Task = class end + +namespace NS2 + +type Task = class end + +namespace NS3 + +module Extensions = + type NS1.Task with + member x.Foo() = 1 + + type NS2.Task with + member x.Bar() = 2 + """ + |> typecheck + |> shouldSucceed + [] let ``Generic and non-generic types with same base name should be allowed``() = // This tests that Expr and Expr<'T> are allowed since they have different LogicalNames @@ -53,7 +81,8 @@ module Extensions = |> shouldSucceed [] - let ``Same generic type name from different namespaces should error``() = + let ``Same generic type name from different namespaces should error for static members``() = + // Same IL collision issue as non-generic, but with generic types. FSharp """ namespace NS1 @@ -67,15 +96,38 @@ namespace NS3 module Extensions = type NS1.Container<'T> with - member x.Foo() = 1 + static member Foo() = 1 type NS2.Container<'T> with - member x.Bar() = 2 + static member Bar() = 2 """ |> typecheck |> shouldFail |> withDiagnosticMessageMatches "Extension members extending types with the same simple name 'Container`1'" + [] + let ``Same generic type name from different namespaces should be allowed for instance members``() = + FSharp """ +namespace NS1 + +type Container<'T> = class end + +namespace NS2 + +type Container<'T> = class end + +namespace NS3 + +module Extensions = + type NS1.Container<'T> with + member x.Foo() = 1 + + type NS2.Container<'T> with + member x.Bar() = 2 + """ + |> typecheck + |> shouldSucceed + [] let ``Extensions on same type should be allowed``() = FSharp """ From 0408122065a7c57b772fc720e1f890bcd9b41464 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Mon, 9 Feb 2026 22:05:44 +0100 Subject: [PATCH 15/20] Fix pre-existing ImportTests warning 52 on net472 --- tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs b/tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs index b0611adee03..1ddccfd9dd5 100644 --- a/tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Import/ImportTests.fs @@ -679,6 +679,7 @@ let updateAge () = """ |> asLibrary |> withReferences [fsLib] + |> ignoreWarnings |> compile |> shouldSucceed |> ignore From 52af8768dc204d58351c354452517ef135f5617e Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 10 Feb 2026 10:34:43 +0100 Subject: [PATCH 16/20] Link release notes to originating PRs --- docs/release-notes/.FSharp.Compiler.Service/10.0.300.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md index 4eddd5c7aef..cc417b8fd81 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.300.md @@ -1,7 +1,7 @@ ### Fixed -* Fix FS0229 B-stream misalignment when reading metadata from assemblies compiled with LangVersion < 9.0. ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) -* Fix FS3356 false positive for instance extension members with same name on different types. ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) +* Fix FS0229 B-stream misalignment when reading metadata from assemblies compiled with LangVersion < 9.0, introduced by [#17706](https://github.com/dotnet/fsharp/pull/17706). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) +* Fix FS3356 false positive for instance extension members with same name on different types, introduced by [#18821](https://github.com/dotnet/fsharp/pull/18821). ([PR #19260](https://github.com/dotnet/fsharp/pull/19260)) ### Added From d770dd5e85c919371000771f6e9eda4b70e51479 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 10 Feb 2026 10:55:18 +0100 Subject: [PATCH 17/20] Extract p_nullnessB helper to reduce repetition in p_ty2 --- src/Compiler/TypedTree/TypedTreePickle.fs | 52 ++++++++--------------- 1 file changed, 17 insertions(+), 35 deletions(-) diff --git a/src/Compiler/TypedTree/TypedTreePickle.fs b/src/Compiler/TypedTree/TypedTreePickle.fs index 81deb4e2037..7017570ddac 100644 --- a/src/Compiler/TypedTree/TypedTreePickle.fs +++ b/src/Compiler/TypedTree/TypedTreePickle.fs @@ -2415,6 +2415,19 @@ let u_tyar_spec st = let u_tyar_specs = (u_list u_tyar_spec) +/// Write nullness information to stream B for a type. +/// Always writes exactly one byte to keep stream B aligned with unconditional reads in u_ty. +/// Other data (e.g. typar constraints) is also written to stream B unconditionally, +/// so skipping nullness bytes would cause stream B misalignment when langFeatureNullness = false. +let p_nullnessB baseTag (nullness: Nullness) st = + if st.oglobals.langFeatureNullness then + match nullness.Evaluate() with + | NullnessInfo.WithNull -> p_byteB baseTag st + | NullnessInfo.WithoutNull -> p_byteB (baseTag + 1) st + | NullnessInfo.AmbivalentToNull -> p_byteB (baseTag + 2) st + else + p_byteB 0 st + let _ = fill_p_ty2 (fun isStructThisArgPos ty st -> let ty = stripTyparEqns ty @@ -2437,56 +2450,25 @@ let _ = p_tys l st | TType_app(ERefNonLocal nleref, [], nullness) -> - // Always write nullness byte to B stream to keep it aligned with unconditional reads in u_ty. - // Other data (e.g. typar constraints) is also written to stream B unconditionally, - // so skipping nullness bytes would cause stream B misalignment when langFeatureNullness = false. - if st.oglobals.langFeatureNullness then - match nullness.Evaluate() with - | NullnessInfo.WithNull -> p_byteB 9 st - | NullnessInfo.WithoutNull -> p_byteB 10 st - | NullnessInfo.AmbivalentToNull -> p_byteB 11 st - else - p_byteB 0 st - + p_nullnessB 9 nullness st p_byte 1 st p_simpletyp nleref st | 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 - + p_nullnessB 12 nullness st p_byte 2 st p_tcref "typ" tc st p_tys tinst st | TType_fun(d, r, nullness) -> - if st.oglobals.langFeatureNullness then - match nullness.Evaluate() with - | NullnessInfo.WithNull -> p_byteB 15 st - | NullnessInfo.WithoutNull -> p_byteB 16 st - | NullnessInfo.AmbivalentToNull -> p_byteB 17 st - else - p_byteB 0 st - + p_nullnessB 15 nullness st p_byte 3 st // Note, the "this" argument may be found in the domain position of a function type, so propagate the isStructThisArgPos value p_ty2 isStructThisArgPos d st p_ty r st | TType_var(r, nullness) -> - if st.oglobals.langFeatureNullness then - match nullness.Evaluate() with - | NullnessInfo.WithNull -> p_byteB 18 st - | NullnessInfo.WithoutNull -> p_byteB 19 st - | NullnessInfo.AmbivalentToNull -> p_byteB 20 st - else - p_byteB 0 st - + p_nullnessB 18 nullness st p_byte 4 st p_tpref r st From bb6675d7c813cb3a73362270197c1da8fc380f70 Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Tue, 10 Feb 2026 11:04:31 +0100 Subject: [PATCH 18/20] Add inline comments for B-stream nullness tag constants --- src/Compiler/TypedTree/TypedTreePickle.fs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Compiler/TypedTree/TypedTreePickle.fs b/src/Compiler/TypedTree/TypedTreePickle.fs index 7017570ddac..b5bd769de7f 100644 --- a/src/Compiler/TypedTree/TypedTreePickle.fs +++ b/src/Compiler/TypedTree/TypedTreePickle.fs @@ -2450,25 +2450,25 @@ let _ = p_tys l st | TType_app(ERefNonLocal nleref, [], nullness) -> - p_nullnessB 9 nullness st + p_nullnessB 9 nullness st // B tags: 9=WithNull, 10=WithoutNull, 11=Ambivalent p_byte 1 st p_simpletyp nleref st | TType_app(tc, tinst, nullness) -> - p_nullnessB 12 nullness st + p_nullnessB 12 nullness st // B tags: 12=WithNull, 13=WithoutNull, 14=Ambivalent p_byte 2 st p_tcref "typ" tc st p_tys tinst st | TType_fun(d, r, nullness) -> - p_nullnessB 15 nullness st + p_nullnessB 15 nullness st // B tags: 15=WithNull, 16=WithoutNull, 17=Ambivalent p_byte 3 st // Note, the "this" argument may be found in the domain position of a function type, so propagate the isStructThisArgPos value p_ty2 isStructThisArgPos d st p_ty r st | TType_var(r, nullness) -> - p_nullnessB 18 nullness st + p_nullnessB 18 nullness st // B tags: 18=WithNull, 19=WithoutNull, 20=Ambivalent p_byte 4 st p_tpref r st From d51d0916a091486921452bde64ebaeb09554e250 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Feb 2026 11:41:27 +0100 Subject: [PATCH 19/20] Add .github/skills/ to .fantomasignore (#19268) --- .fantomasignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.fantomasignore b/.fantomasignore index c2677d44fa4..1b373a01b2e 100644 --- a/.fantomasignore +++ b/.fantomasignore @@ -13,6 +13,7 @@ artifacts/ # For some reason, it tries to format files from remotes (Processing ./.git/refs/remotes//FSComp.fsi) .git/ +.github/skills/ # Explicitly unformatted implementation src/Compiler/Checking/AccessibilityLogic.fs From ceed0ceb684dbd2a6972d617b669cab3600ef66f Mon Sep 17 00:00:00 2001 From: Tomas Grosup Date: Wed, 11 Feb 2026 14:33:37 +0100 Subject: [PATCH 20/20] improving file specific instructions, adding postmortem skill --- .github/instructions/CodeGen.instructions.md | 6 ++ .../instructions/DebugEmit.instructions.md | 6 ++ .github/instructions/FSComp.instructions.md | 6 ++ .../instructions/FSharpCore.instructions.md | 6 ++ .github/instructions/LSP.instructions.md | 6 ++ .../instructions/Optimizer.instructions.md | 6 ++ .../instructions/SyntaxTree.instructions.md | 7 ++ .../TypedTreePickle.instructions.md | 73 ++++++++++++++++++ .github/skills/postmortem/SKILL.md | 75 +++++++++++++++++++ docs/postmortems/README.md | 5 ++ .../regression-fs0229-bstream-misalignment.md | 0 11 files changed, 196 insertions(+) create mode 100644 .github/instructions/CodeGen.instructions.md create mode 100644 .github/instructions/DebugEmit.instructions.md create mode 100644 .github/instructions/FSComp.instructions.md create mode 100644 .github/instructions/FSharpCore.instructions.md create mode 100644 .github/instructions/LSP.instructions.md create mode 100644 .github/instructions/Optimizer.instructions.md create mode 100644 .github/instructions/SyntaxTree.instructions.md create mode 100644 .github/instructions/TypedTreePickle.instructions.md create mode 100644 .github/skills/postmortem/SKILL.md create mode 100644 docs/postmortems/README.md rename docs/{ => postmortems}/regression-fs0229-bstream-misalignment.md (100%) diff --git a/.github/instructions/CodeGen.instructions.md b/.github/instructions/CodeGen.instructions.md new file mode 100644 index 00000000000..379901aeeef --- /dev/null +++ b/.github/instructions/CodeGen.instructions.md @@ -0,0 +1,6 @@ +--- +applyTo: + - "src/Compiler/CodeGen/**/*.{fs,fsi}" +--- + +Read `docs/representations.md`. diff --git a/.github/instructions/DebugEmit.instructions.md b/.github/instructions/DebugEmit.instructions.md new file mode 100644 index 00000000000..699ceba4513 --- /dev/null +++ b/.github/instructions/DebugEmit.instructions.md @@ -0,0 +1,6 @@ +--- +applyTo: + - "src/Compiler/AbstractIL/ilwritepdb.{fs,fsi}" +--- + +Read `docs/debug-emit.md`. diff --git a/.github/instructions/FSComp.instructions.md b/.github/instructions/FSComp.instructions.md new file mode 100644 index 00000000000..104f99bef11 --- /dev/null +++ b/.github/instructions/FSComp.instructions.md @@ -0,0 +1,6 @@ +--- +applyTo: + - "src/Compiler/FSComp.txt" +--- + +Read and follow `docs/diagnostics.md`. diff --git a/.github/instructions/FSharpCore.instructions.md b/.github/instructions/FSharpCore.instructions.md new file mode 100644 index 00000000000..16971df3903 --- /dev/null +++ b/.github/instructions/FSharpCore.instructions.md @@ -0,0 +1,6 @@ +--- +applyTo: + - "src/FSharp.Core/**/*.{fs,fsi}" +--- + +Read and follow `docs/fsharp-core-notes.md`. diff --git a/.github/instructions/LSP.instructions.md b/.github/instructions/LSP.instructions.md new file mode 100644 index 00000000000..8b7a3b29471 --- /dev/null +++ b/.github/instructions/LSP.instructions.md @@ -0,0 +1,6 @@ +--- +applyTo: + - "src/FSharp.Compiler.LanguageServer/**/*.{fs,fsi}" +--- + +Read `docs/lsp.md`. diff --git a/.github/instructions/Optimizer.instructions.md b/.github/instructions/Optimizer.instructions.md new file mode 100644 index 00000000000..d1ffb0bd014 --- /dev/null +++ b/.github/instructions/Optimizer.instructions.md @@ -0,0 +1,6 @@ +--- +applyTo: + - "src/Compiler/Optimize/**/*.{fs,fsi}" +--- + +Read `docs/optimizations.md`. diff --git a/.github/instructions/SyntaxTree.instructions.md b/.github/instructions/SyntaxTree.instructions.md new file mode 100644 index 00000000000..666713e1b8b --- /dev/null +++ b/.github/instructions/SyntaxTree.instructions.md @@ -0,0 +1,7 @@ +--- +applyTo: + - "src/Compiler/SyntaxTree/SyntaxTree.{fs,fsi}" + - "src/Compiler/pars.fsy" +--- + +Read `docs/changing-the-ast.md`. diff --git a/.github/instructions/TypedTreePickle.instructions.md b/.github/instructions/TypedTreePickle.instructions.md new file mode 100644 index 00000000000..135110d9a4f --- /dev/null +++ b/.github/instructions/TypedTreePickle.instructions.md @@ -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`. diff --git a/.github/skills/postmortem/SKILL.md b/.github/skills/postmortem/SKILL.md new file mode 100644 index 00000000000..035862c07bb --- /dev/null +++ b/.github/skills/postmortem/SKILL.md @@ -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. diff --git a/docs/postmortems/README.md b/docs/postmortems/README.md new file mode 100644 index 00000000000..f545a92baaf --- /dev/null +++ b/docs/postmortems/README.md @@ -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. diff --git a/docs/regression-fs0229-bstream-misalignment.md b/docs/postmortems/regression-fs0229-bstream-misalignment.md similarity index 100% rename from docs/regression-fs0229-bstream-misalignment.md rename to docs/postmortems/regression-fs0229-bstream-misalignment.md