diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 9509ac72a3..dc135c64eb 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,7 +1,7 @@ ### Fixed * Suppress hover/symbol resolution for wildcard `_` patterns inside `member _.…` bodies that incorrectly showed `val _: T` tooltip. ([PR #19760](https://github.com/dotnet/fsharp/pull/19760)) -* Deduplicate format specifier locations in computation expressions so editor tooling no longer reports duplicate entries for the same `%` specifier. ([Issue #16419](https://github.com/dotnet/fsharp/issues/16419), [PR #19791](https://github.com/dotnet/fsharp/pull/19791)) +* Fix the source of duplicate format specifier locations in `seq { }` computation expressions with implicit yield: the body was speculatively type-checked twice (once as a statement, once as a yielded expression), causing the sink to be notified twice per `%` specifier. The second pass now reuses the cached first-pass result. ([Issue #16419](https://github.com/dotnet/fsharp/issues/16419), [PR #19791](https://github.com/dotnet/fsharp/pull/19791)) * Reject non-function bindings for single-case and partial active pattern names with FS1209, matching the existing multi-case behavior. ([PR #19763](https://github.com/dotnet/fsharp/pull/19763)) * Fix FS0421 "The address of the variable cannot be used at this point" incorrectly raised for the discard pattern `let _ = &expr` when `let x = &expr` compiles. ([Issue #18841](https://github.com/dotnet/fsharp/issues/18841), [PR #19811](https://github.com/dotnet/fsharp/pull/19811)) * Honor `--nowarn` and `--warnaserror` for warnings emitted during command-line option parsing ([Issue #19576](https://github.com/dotnet/fsharp/issues/19576), [PR #19776](https://github.com/dotnet/fsharp/pull/19776)) diff --git a/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs b/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs index 86bf635d58..838de01418 100644 --- a/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckSequenceExpressions.fs @@ -439,7 +439,23 @@ let TcSequenceExpression (cenv: TcFileState) env tpenv comp (overallTy: OverallT let genResultTy = NewInferenceType g let mExpr = expr.Range UnifyTypes cenv env mExpr genOuterTy (mkSeqTy cenv.g genResultTy) - let expr, tpenv = TcExprFlex cenv flex true genResultTy env tpenv comp + + // Cache the result of the first type-check so that re-checking 'comp' below as a yielded + // expression reuses it instead of running side-effecting checks (e.g. format string parsing) + // a second time. See also TcExprSequentialOrImplicitYield which uses the same cache. + let cachedExpr = + match expr with + | Expr.DebugPoint(_, e) -> e + | _ -> expr + + env.eCachedImplicitYieldExpressions.Add(comp.Range, (comp, _ty, cachedExpr)) + + let expr, tpenv = + try + TcExprFlex cenv flex true genResultTy env tpenv comp + finally + env.eCachedImplicitYieldExpressions.Remove comp.Range + let exprTy = tyOfExpr cenv.g expr AddCxTypeMustSubsumeType env.eContextInfo env.DisplayEnv cenv.css mExpr NoTrace genResultTy exprTy diff --git a/src/Compiler/Checking/NameResolution.fs b/src/Compiler/Checking/NameResolution.fs index 36eedebfbe..898dc78aa7 100644 --- a/src/Compiler/Checking/NameResolution.fs +++ b/src/Compiler/Checking/NameResolution.fs @@ -2185,8 +2185,6 @@ type TcResultsSinkImpl(tcGlobals, ?sourceText: ISourceText) = let capturedOpenDeclarations = ResizeArray() let capturedFormatSpecifierLocations = ResizeArray<_>() - let capturedFormatSpecifierRanges = HashSet() - let capturedNameResolutionIdentifiers = HashSet { new IEqualityComparer<_> with @@ -2291,8 +2289,7 @@ type TcResultsSinkImpl(tcGlobals, ?sourceText: ISourceText) = capturedMethodGroupResolutions.Add(CapturedNameResolution(itemMethodGroup, [], occurrenceType, nenv, ad, m)) member sink.NotifyFormatSpecifierLocation(m, numArgs) = - if capturedFormatSpecifierRanges.Add(m) then - capturedFormatSpecifierLocations.Add((m, numArgs)) + capturedFormatSpecifierLocations.Add((m, numArgs)) member sink.NotifyRelatedSymbolUse(m, item, kind) = if allowedRange m then diff --git a/tests/FSharp.Compiler.Service.Tests/EditorTests.fs b/tests/FSharp.Compiler.Service.Tests/EditorTests.fs index f9c1da195f..a765412901 100644 --- a/tests/FSharp.Compiler.Service.Tests/EditorTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/EditorTests.fs @@ -11,6 +11,13 @@ open FSharp.Compiler.Tokenization #nowarn "1182" // Unused bindings when ignored parsed results etc. +/// Virtual source filename used by tests that don't need a real on-disk path. +/// Path.Combine keeps this OS-neutral for any test that compares the resulting range filename. +let private testFile = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "Test.fsx") + +/// Parses and type-checks an inline source snippet against the shared `testFile` identifier. +let private parseAndCheck (source: string) = parseAndCheckScript (testFile, source) + let stringMethods = [ "Chars"; "Clone"; "CompareTo"; "Contains"; "CopyTo"; "EndsWith"; @@ -541,18 +548,35 @@ let _ = debug "[LanguageService] Type checking fails for '%s' with content=%A an (4, 82, 4, 84, 1); (4, 108, 4, 110, 1)|] -[] -let ``Format specifier locations not duplicated in CE`` () = - let input = "let _ = seq { sprintf \"%d\" 1 }" - let file = "/home/user/Test.fsx" - let _parseResult, typeCheckResults = parseAndCheckScript(file, input) - - let locations = typeCheckResults.GetFormatSpecifierLocationsAndArity() - let percentD = - locations - |> Array.filter (fun (r, _) -> r.StartColumn = 23) - - Assert.Equal(1, percentD.Length) +// Regression for issue #16419: in `seq { e }` with implicit-yield, the body 'e' was +// type-checked twice (once as a statement via TryTcStmt, once as a yielded expression). +// Both passes used to notify the sink, leading to duplicate format-specifier entries +// when 'e' contained a printf-style format string. +[] +[] +[] +[] +let ``Format specifier locations are not duplicated in seq computation expression`` (source: string, expectedCount: int) = + let _, typeCheckResults = parseAndCheck source + let locs = typeCheckResults.GetFormatSpecifierLocationsAndArity() + Assert.Equal(expectedCount, locs.Length) + +// Validates that caching the implicit-yield first-pass typecheck does not break +// expected-type-driven inference (subsumption, type-directed conversion, +// nullness flex, overload resolution). +[] +[ = seq { 1 }")>] +[ = seq { yield 1 }")>] +[ = seq { \"hi\" }")>] +[ = seq { \"hi\" }")>] +[ = seq { T.M(1) }")>] +let ``Implicit-yield in seq preserves expected-type-driven inference`` (source: string) = + let _, typeCheckResults = parseAndCheck source + let errors = + typeCheckResults.Diagnostics + |> Array.filter (fun d -> d.Severity = FSharp.Compiler.Diagnostics.FSharpDiagnosticSeverity.Error) + |> Array.map (fun d -> d.Message) + Assert.Equal<_ seq>(Array.empty, errors) #if ASSUME_PREVIEW_FSHARP_CORE []