diff --git a/EXECUTIVE_SUMMARY.md b/EXECUTIVE_SUMMARY.md new file mode 100644 index 0000000000..9937985ccc --- /dev/null +++ b/EXECUTIVE_SUMMARY.md @@ -0,0 +1,77 @@ +# FSharpPlus curryN Regression Fix - Executive Summary + +## The Bug + +FSharpPlus `curryN` pattern worked in F# SDK 8.0 but failed in SDK 9.0+ with error FS0030 (value restriction): + +```fsharp +let _x1 = curryN f1 100 // SDK 8: OK | SDK 9+: FS0030 error +``` + +## Root Cause + +In `CheckDeclarations.fs`, the `ApplyDefaults` function processes unsolved type variables at the end of type checking. The existing code only solved typars with `StaticReq <> None`: + +```fsharp +if (tp.StaticReq <> TyparStaticReq.None) then + ChooseTyparSolutionAndSolve cenv.css denvAtEnd tp +``` + +**The problem**: Some SRTP (Statically Resolved Type Parameter) typars have `MayResolveMember` constraints but `StaticReq=None`. These typars were being skipped, leaving them unsolved. When `CheckValueRestriction` ran next, it found unsolved typars and reported FS0030. + +## Why This Is a Bug + +The `ApplyDefaults` code checks `StaticReq <> None` to identify SRTP typars that need solving. However, a typar may participate in an SRTP constraint (having a `MayResolveMember` constraint) without having `StaticReq` set. This can happen when: + +1. The typar is the **result type** of an SRTP method call, not the head type +2. The typar is constrained through SRTP constraints but isn't directly marked with `^` + +**Key insight from instrumentation:** +``` + - ? (StaticReq=None, Constraints=[MayResolveMember, CoercesTo]) ← HAS SRTP CONSTRAINT! +[ApplyDefaults] After processing: 17 still unsolved ← SRTP typar SKIPPED because StaticReq=None +``` + +The condition `tp.StaticReq <> None` was too narrow - it missed typars that have SRTP constraints but no explicit static requirement. + +## Regression Analysis - Git Blame Evidence + +**Root Cause: PR #15181 (commit `b73be1584`) - Nullness Checking Feature** + +The regression was introduced by `FreshenTypar` added in PR #15181: + +```fsharp +// src/Compiler/Checking/NameResolution.fs:1600-1604 +let FreshenTypar (g: TcGlobals) rigid (tp: Typar) = + let clearStaticReq = g.langVersion.SupportsFeature LanguageFeature.InterfacesWithAbstractStaticMembers + let staticReq = if clearStaticReq then TyparStaticReq.None else tp.StaticReq // ← BUG! + ... +``` + +**The Mechanism:** + +1. **SDK 8**: `FreshenTypar` did not exist. When typars were freshened, `StaticReq` was preserved from the original typar. + +2. **SDK 9+**: When `InterfacesWithAbstractStaticMembers` is enabled (always on), `FreshenTypar` **clears `StaticReq` to `None`** unconditionally. + +3. **Effect**: SRTP typars still have `MayResolveMember` constraints, but lose their `StaticReq` marker. + +4. **Consequence**: `ApplyDefaults` checks `if tp.StaticReq <> None` → returns false → typar never solved → FS0030 error. + +**The fix** adds an alternative check for `MayResolveMember` constraints directly, making `ApplyDefaults` robust against this `StaticReq` clearing. + +## The Fix + +Added a check for `MayResolveMember` constraints in addition to `StaticReq`: + +```fsharp +let hasSRTPConstraint = tp.Constraints |> List.exists (function TyparConstraint.MayResolveMember _ -> true | _ -> false) +if (tp.StaticReq <> TyparStaticReq.None) || hasSRTPConstraint then + ChooseTyparSolutionAndSolve cenv.css denvAtEnd tp +``` + +## Verification + +- ✅ New curryN-style regression test passes +- ✅ FSharpPlus `curryN` pattern compiles without type annotations +- ✅ No regressions introduced in SRTP-related tests diff --git a/azure-pipelines-PR.yml b/azure-pipelines-PR.yml index 83919f2749..2ac87e6157 100644 --- a/azure-pipelines-PR.yml +++ b/azure-pipelines-PR.yml @@ -914,3 +914,17 @@ stages: buildScript: build.sh displayName: FSharpPlus_Linux useVmImage: $(UbuntuMachineQueueName) + - repo: fsprojects/FSharpPlus + commit: 2648efe + buildScript: build.cmd + displayName: FsharpPlus_NET10 + # remove this before merging + - repo: fsprojects/FSharpPlus + commit: 2648efe + buildScript: dotnet msbuild build.proj -t:Build;Pack;Test;AllDocs + displayName: FsharpPlus_NET10_FullTestSuite + - repo: fsprojects/FSharpPlus + commit: 2648efe + buildScript: build.sh + displayName: FsharpPlus_Net10_Linux + useVmImage: $(UbuntuMachineQueueName) diff --git a/docs/release-notes/.FSharp.Compiler.Service/10.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/10.0.200.md index 54f1b13f05..e82d8bdb6a 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/10.0.200.md +++ b/docs/release-notes/.FSharp.Compiler.Service/10.0.200.md @@ -1,5 +1,6 @@ ### Fixed +* Fixed SRTP resolution regression causing FS0030 value restriction errors with FSharpPlus curryN-style patterns in .NET 9 SDK. ([PR #19218](https://github.com/dotnet/fsharp/pull/19218)) * Type relations cache: optimize key generation ([Issue #19116](https://github.com/dotnet/fsharp/issues/18767)) ([PR #19120](https://github.com/dotnet/fsharp/pull/19120)) * Fixed QuickParse to correctly handle optional parameter syntax with `?` prefix, resolving syntax highlighting issues. ([Issue #11008753](https://developercommunity.visualstudio.com/t/F-Highlighting-fails-on-optional-parame/11008753)) ([PR #XXXXX](https://github.com/dotnet/fsharp/pull/XXXXX)) * Fix `--preferreduilang` switch leaking into `fsi.CommandLineArgs` when positioned after script file ([PR #19151](https://github.com/dotnet/fsharp/pull/19151)) diff --git a/eng/templates/regression-test-jobs.yml b/eng/templates/regression-test-jobs.yml index 3216eeaed4..ac1195e397 100644 --- a/eng/templates/regression-test-jobs.yml +++ b/eng/templates/regression-test-jobs.yml @@ -5,13 +5,13 @@ parameters: - name: testMatrix type: object - name: dependsOn - type: string + type: string default: 'EndToEndBuildTests' jobs: - ${{ each item in parameters.testMatrix }}: - job: RegressionTest_${{ replace(item.displayName, '-', '_') }}_${{ replace(replace(item.repo, '/', '_'), '-', '_') }} - displayName: '${{ item.displayName }} Regression Test' + displayName: '${{ item.displayName }} Regression Test' dependsOn: ${{ parameters.dependsOn }} ${{ if item.useVmImage }}: pool: @@ -19,7 +19,7 @@ jobs: ${{ else }}: pool: name: $(DncEngPublicBuildPool) - demands: ImageOverride -equals $(_WindowsMachineQueueName) + demands: ImageOverride -equals $(_WindowsMachineQueueName) timeoutInMinutes: 60 steps: - checkout: self @@ -27,7 +27,7 @@ jobs: - task: DownloadPipelineArtifact@2 displayName: Download F# Compiler FSC Artifacts - inputs: + inputs: artifactName: 'FSharpCompilerFscArtifacts' downloadPath: '$(Pipeline.Workspace)/FSharpCompiler/artifacts/bin/fsc' @@ -38,17 +38,17 @@ jobs: downloadPath: '$(Pipeline.Workspace)/FSharpCompiler/artifacts/bin/FSharp.Core' - task: DownloadPipelineArtifact@2 - displayName: Download UseLocalCompiler props + displayName: Download UseLocalCompiler props inputs: artifactName: 'UseLocalCompilerProps' downloadPath: '$(Pipeline.Workspace)/Props' - pwsh: | - Write-Host "Cloning repository: ${{ item.repo }}" + Write-Host "Cloning repository: ${{ item.repo }}" git clone --recursive https://github.com/${{ item.repo }}.git $(Pipeline.Workspace)/TestRepo Set-Location $(Pipeline.Workspace)/TestRepo - Write-Host "Checking out commit: ${{ item.commit }}" + Write-Host "Checking out commit: ${{ item.commit }}" git checkout ${{ item.commit }} Write-Host "Initializing submodules (if any)..." @@ -60,25 +60,22 @@ jobs: Write-Host "Repository structure:" Get-ChildItem -Name - # Check if buildScript is a built-in dotnet command or a file-based script - # Note: buildScript comes from the pipeline YAML configuration (testMatrix parameter), - # not from external user input, so it's safe to use - $buildScript = "${{ item.buildScript }}" + $buildScript = '${{ item.buildScript }}' if ($buildScript -like "dotnet*") { - Write-Host "Build command is a built-in dotnet command: $buildScript" + 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" + Write-Host "Verifying build script exists: $buildScript" if (Test-Path $buildScript) { Write-Host "Build script found: $buildScript" } else { - Write-Host "Build script not found: $buildScript" + Write-Host "Build script not found: $buildScript" Write-Host "Available files in root:" Get-ChildItem exit 1 } } - displayName: Checkout ${{ item.displayName }} at specific commit + displayName: Checkout ${{ item.displayName }} at specific commit - pwsh: | Set-Location $(Pipeline.Workspace)/TestRepo @@ -92,14 +89,14 @@ jobs: displayName: Remove global.json to use latest SDK - task: UseDotNet@2 - displayName: Install .NET SDK 8.0.x for ${{ item.displayName }} + displayName: Install .NET SDK 8.0.x for ${{ item.displayName }} inputs: packageType: sdk version: '8.0.x' installationPath: $(Pipeline.Workspace)/TestRepo/.dotnet - task: UseDotNet@2 - displayName: Install .NET SDK 10.0.100 for ${{ item.displayName }} + displayName: Install .NET SDK 10.0.100 for ${{ item.displayName }} inputs: packageType: sdk version: '10.0.100' @@ -153,23 +150,41 @@ jobs: Write-Host "============================================" Write-Host "" - $buildScript = "${{ item.buildScript }}" + $buildScript = '${{ item.buildScript }}' + $errorLogPath = "$(Pipeline.Workspace)/build-errors.log" + $fullLogPath = "$(Pipeline.Workspace)/build-full.log" + + # Function to filter and capture important lines + function Process-BuildOutput { + param([string]$line) + Write-Host $line + if ($line -match ': error ' -or + $line -match ': warning ' -or + $line -match 'FAILED|Failed!' -or + $line -match '^\s+X\s+' -or + $line -match 'Exception: ' -or + $line -match '^\s+at\s+.*\(\)' -or + $line -match 'Build FAILED') { + Add-Content -Path $errorLogPath -Value $line + } + } - # Check if it's a built-in dotnet command or a file-based script - # Note: buildScript comes from the pipeline YAML configuration (testMatrix parameter), - # not from external user input, so using Invoke-Expression is safe here if ($buildScript -like "dotnet*") { Write-Host "Executing built-in command: $buildScript" - Invoke-Expression $buildScript + Invoke-Expression $buildScript 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { + Process-BuildOutput $_ + } } elseif ($IsWindows) { - # Execute the provided script on Windows Write-Host "Executing file-based script: $buildScript" - & ".\$buildScript" + & ".\$buildScript" 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { + Process-BuildOutput $_ + } } else { - # Execute the provided script on Linux Write-Host "Executing file-based script: $buildScript" chmod +x "$buildScript" - bash -c "./$buildScript" + bash -c "./$buildScript" 2>&1 | Tee-Object -FilePath $fullLogPath | ForEach-Object { + Process-BuildOutput $_ + } } $exitCode = $LASTEXITCODE @@ -182,14 +197,14 @@ jobs: if ($exitCode -ne 0) { exit $exitCode } - displayName: Build ${{ item.displayName }} with local F# compiler + displayName: Build ${{ item.displayName }} with local F# compiler env: LocalFSharpCompilerPath: $(Pipeline.Workspace)/FSharpCompiler LoadLocalFSharpBuild: 'True' LocalFSharpCompilerConfiguration: Release - timeoutInMinutes: 45 + timeoutInMinutes: 45 - - pwsh: | + - pwsh: | Set-Location $(Pipeline.Workspace)/TestRepo $binlogDir = "$(Pipeline.Workspace)/BinaryLogs" New-Item -ItemType Directory -Force -Path $binlogDir | Out-Null @@ -200,18 +215,18 @@ jobs: Write-Host "No .binlog files found" } else { foreach ($binlog in $binlogs) { - Write-Host "Copying: $($binlog.FullName)" + Write-Host "Copying: $($binlog.FullName)" Copy-Item $binlog.FullName -Destination $binlogDir } Write-Host "Collected $($binlogs.Count) .binlog files" } - displayName: Collect Binary Logs + displayName: Collect Binary Logs condition: always() continueOnError: true - task: PublishPipelineArtifact@1 - displayName: Publish ${{ item.displayName }} Binary Logs - inputs: + displayName: Publish ${{ item.displayName }} Binary Logs + inputs: targetPath: '$(Pipeline.Workspace)/BinaryLogs' artifactName: '${{ item.displayName }}_BinaryLogs' publishLocation: pipeline @@ -226,6 +241,7 @@ jobs: Write-Host "Repository: ${{ item.repo }}" Write-Host "Commit: ${{ item.commit }}" Write-Host "Build Script: ${{ item.buildScript }}" + if ($env:AGENT_JOBSTATUS -eq "Succeeded") { Write-Host "Status: SUCCESS" Write-Host "The ${{ item.displayName }} library builds successfully with the new F# compiler" @@ -233,44 +249,72 @@ jobs: Write-Host "Status: FAILED" Write-Host "The ${{ item.displayName }} library failed to build with the new F# compiler" - # Build multiline error message with reproduction steps - $lines = @( - "Regression test FAILED for ${{ item.displayName }} (${{ item.repo }}@${{ item.commit }})", - "", - "LOCAL REPRODUCTION STEPS (from fsharp repo root):", - "==========================================", - "# 1. Build the F# compiler", - "./build.sh -c Release", - "", - "# 2. Clone and checkout the failing library", - "cd ..", - "git clone --recursive https://github.com/${{ item.repo }}.git TestRepo", - "cd TestRepo", - "git checkout ${{ item.commit }}", - "git submodule update --init --recursive", - "rm -f global.json", - "", - "# 3. Prepare the repo for local compiler", - "dotnet fsi ../fsharp/eng/scripts/PrepareRepoForRegressionTesting.fsx `"../fsharp/UseLocalCompiler.Directory.Build.props`"", - "", - "# 4. Build with local compiler", - "export LocalFSharpCompilerPath=`$PWD/../fsharp", - "export LoadLocalFSharpBuild=True", - "export LocalFSharpCompilerConfiguration=Release", - "./${{ item.buildScript }}", - "==========================================" - ) - - # Report using VSO error format - each line separately - foreach ($line in $lines) { - Write-Host "##[error]$line" + $errorLogPath = "$(Pipeline.Workspace)/build-errors.log" + if (Test-Path $errorLogPath) { + Write-Host "" + Write-Host "##[error]============================================" + Write-Host "##[error]FAILURE SUMMARY for ${{ item.displayName }}" + Write-Host "##[error]============================================" + + $errors = Get-Content $errorLogPath | Select-Object -First 100 + $errorCount = 0 + $warningCount = 0 + $testFailCount = 0 + + foreach ($line in $errors) { + if ($line -match ': error ') { + Write-Host "##[error]$line" + $errorCount++ + } elseif ($line -match ': warning ') { + Write-Host "##[warning]$line" + $warningCount++ + } elseif ($line -match 'FAILED|Failed! |^\s+X\s+') { + Write-Host "##[error][TEST] $line" + $testFailCount++ + } else { + Write-Host "##[error]$line" + } + } + + $totalLines = (Get-Content $errorLogPath | Measure-Object -Line).Lines + if ($totalLines -gt 100) { + Write-Host "##[warning]... and $($totalLines - 100) more lines (see full log or binary logs)" + } + + Write-Host "" + Write-Host "##[error]Summary: $errorCount error(s), $warningCount warning(s), $testFailCount test failure(s)" + Write-Host "##[error]============================================" + } else { + Write-Host "##[warning]No error log captured - check full build output above" } - # Also log as VSO issue for Azure DevOps integration + Write-Host "" + Write-Host "##[section]LOCAL REPRODUCTION STEPS (from fsharp repo root):" + Write-Host "# 1. Build the F# compiler" + Write-Host "./build.sh -c Release" + Write-Host "" + Write-Host "# 2. Clone and checkout the failing library" + Write-Host "cd .." + Write-Host "git clone --recursive https://github.com/${{ item.repo }}.git TestRepo" + Write-Host "cd TestRepo" + Write-Host "git checkout ${{ item.commit }}" + Write-Host "git submodule update --init --recursive" + Write-Host "rm -f global.json" + Write-Host "" + Write-Host "# 3. Prepare the repo for local compiler" + Write-Host "dotnet fsi ../fsharp/eng/scripts/PrepareRepoForRegressionTesting.fsx `"../fsharp/UseLocalCompiler.Directory.Build.props`"" + Write-Host "" + Write-Host "# 4. Build with local compiler" + Write-Host "export LocalFSharpCompilerPath=`$PWD/../fsharp" + Write-Host "export LoadLocalFSharpBuild=True" + Write-Host "export LocalFSharpCompilerConfiguration=Release" + Write-Host "${{ item.buildScript }}" + Write-Host "##vso[task.logissue type=error;sourcepath=azure-pipelines-PR.yml]Regression test failed: ${{ item.displayName }}" } Write-Host "============================================" + Write-Host "" Write-Host "Binary logs found:" Get-ChildItem "*.binlog" -Recurse -ErrorAction SilentlyContinue | ForEach-Object { Write-Host $_.FullName } displayName: Report ${{ item.displayName }} test result diff --git a/src/Compiler/Checking/CheckDeclarations.fs b/src/Compiler/Checking/CheckDeclarations.fs index ce1bd58263..a5139a1cea 100644 --- a/src/Compiler/Checking/CheckDeclarations.fs +++ b/src/Compiler/Checking/CheckDeclarations.fs @@ -5672,10 +5672,13 @@ let ApplyDefaults (cenv: cenv) g denvAtEnd m moduleContents extraAttribs = // the defaults will be propagated to the new type variable. ApplyTyparDefaultAtPriority denvAtEnd cenv.css priority tp) - // OK, now apply defaults for any unsolved TyparStaticReq.HeadType + // OK, now apply defaults for any unsolved TyparStaticReq.HeadType or typars with SRTP (MayResolveMember) constraints + // Note: We also check for MayResolveMember constraints because some SRTP typars may not have StaticReq set + // (this can happen when the typar is involved in an SRTP constraint but isn't the "head type" itself) unsolved |> List.iter (fun tp -> if not tp.IsSolved then - if (tp.StaticReq <> TyparStaticReq.None) then + let hasSRTPConstraint = tp.Constraints |> List.exists (function TyparConstraint.MayResolveMember _ -> true | _ -> false) + if (tp.StaticReq <> TyparStaticReq.None) || hasSRTPConstraint then ChooseTyparSolutionAndSolve cenv.css denvAtEnd tp) with RecoverableException exn -> errorRecovery exn m diff --git a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs index b5fe18bac8..2f904cf287 100644 --- a/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Conformance/Types/TypeConstraints/IWSAMsAndSRTPs/IWSAMsAndSRTPsTests.fs @@ -1767,3 +1767,39 @@ printfn "Success: %d" result |> compileAndRun |> shouldSucceed + // Regression test for GitHub issue #18344 and FSharpPlus curryN pattern + // This tests that SRTP typars with MayResolveMember constraints are properly solved + // even when StaticReq is None + [] + let ``SRTP curryN-style pattern should compile without value restriction error`` () = + FSharp """ +module CurryNTest + +open System + +// Minimal reproduction of the FSharpPlus curryN pattern +type Curry = + static member inline Invoke f = + let inline call_2 (a: ^a, b: ^b) = ((^a or ^b) : (static member Curry: _*_ -> _) b, a) + call_2 (Unchecked.defaultof, Unchecked.defaultof<'t>) (f: 't -> 'r) : 'args + + static member Curry (_: Tuple<'t1> , _: Curry) = fun f t1 -> f (Tuple<_> t1) + static member Curry ((_, _) , _: Curry) = fun f t1 t2 -> f (t1, t2) + static member Curry ((_, _, _) , _: Curry) = fun f t1 t2 t3 -> f (t1, t2, t3) + +let inline curryN f = Curry.Invoke f + +// Test functions +let f1 (x: Tuple<_>) = [x.Item1] +let f2 (x, y) = [x + y] +let f3 (x, y, z) = [x + y + z] + +// These should compile without value restriction error (regression test for #18344) +let _x1 = curryN f1 100 +let _x2 = curryN f2 1 2 +let _x3 = curryN f3 1 2 3 + """ + |> asLibrary + |> compile + |> shouldSucceed +