Skip to content

Commit 1ff7cb1

Browse files
authored
Fix checking nested fields for records (#18964)
1 parent c1b13e3 commit 1ff7cb1

File tree

5 files changed

+78
-8
lines changed

5 files changed

+78
-8
lines changed

docs/release-notes/.FSharp.Compiler.Service/11.0.0.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22

33
* Scripts: Fix resolving the dotnet host path when an SDK directory is specified. ([PR #18960](https://github.com/dotnet/fsharp/pull/18960))
44
* Fix excessive StackGuard thread jumping ([PR #18971](https://github.com/dotnet/fsharp/pull/18971))
5+
* Checking: Fix checking nested fields for records and anonymous ([PR #18964](https://github.com/dotnet/fsharp/pull/18964))
56
* Fix name is bound multiple times is not reported in 'as' pattern ([PR #18984](https://github.com/dotnet/fsharp/pull/18984))
6-
* Type relations cache: handle potentially "infinite" types ([PR #19010](https://github.com/dotnet/fsharp/pull/19010))
7+
* Type relations cache: handle potentially "infinite" types ([PR #19010](https://github.com/dotnet/fsharp/pull/19010))
78

89
### Added
910

src/Compiler/Checking/CheckRecordSyntaxHelpers.fs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,12 +156,26 @@ let TransformAstForNestedUpdates (cenv: TcFileState) (env: TcEnv) overallTy (lid
156156
(accessIds, outerFieldId),
157157
Some(synExprRecd (recdExprCopyInfo (fields |> List.map fst) withExpr) outerFieldId rest exprBeingAssigned)
158158

159+
/// This name is used when a complex expression is bound for use as a binding in a copy-and-update expression.
160+
/// For example, in `{ f () with ... }`, `f ()` is replaced by `let bind@ = f ()`
161+
let BindIdText = "bind@"
162+
163+
/// Finding the 'bind@' identifier is the only way to detect that an expression has already been bound.
164+
let inline (|IsSimpleOrBoundExpr|_|) (withExprOpt: (SynExpr * BlockSeparator) option) =
165+
match withExprOpt with
166+
| None -> true
167+
| Some(expr, _) ->
168+
match expr with
169+
| SynExpr.LongIdent(_, lIds, _, _) -> lIds.LongIdent |> List.exists (fun id -> id.idText = BindIdText)
170+
| SynExpr.Ident _ -> true
171+
| _ -> false
172+
159173
/// When the original expression in copy-and-update is more complex than `{ x with ... }`, like `{ f () with ... }`,
160174
/// we bind it first, so that it's not evaluated multiple times during a nested update
161175
let BindOriginalRecdExpr (withExpr: SynExpr * BlockSeparator) mkRecdExpr =
162176
let originalExpr, blockSep = withExpr
163177
let mOrigExprSynth = originalExpr.Range.MakeSynthetic()
164-
let id = mkSynId mOrigExprSynth "bind@"
178+
let id = mkSynId mOrigExprSynth BindIdText
165179
let withExpr = SynExpr.Ident id, blockSep
166180

167181
let binding =

src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,9 @@ val TransformAstForNestedUpdates<'a> :
1919
withExpr: SynExpr * (range * 'a) ->
2020
(Ident list * Ident) * SynExpr option
2121

22+
val BindIdText: string
23+
24+
val inline (|IsSimpleOrBoundExpr|_|): withExprOpt: (SynExpr * BlockSeparator) option -> bool
25+
2226
val BindOriginalRecdExpr:
2327
withExpr: SynExpr * BlockSeparator -> mkRecdExpr: ((SynExpr * BlockSeparator) option -> SynExpr) -> SynExpr

src/Compiler/Checking/Expressions/CheckExpressions.fs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5895,9 +5895,8 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE
58955895
TcExprTuple cenv overallTy env tpenv (isExplicitStruct, args, m)
58965896

58975897
| SynExpr.AnonRecd (isStruct, withExprOpt, unsortedFieldExprs, mWholeExpr, trivia) ->
5898-
match withExprOpt with
5899-
| None
5900-
| Some(SynExpr.Ident _, _) ->
5898+
match withExprOpt with
5899+
| None | IsSimpleOrBoundExpr ->
59015900
TcNonControlFlowExpr env <| fun env ->
59025901
TcPossiblyPropagatingExprLeafThenConvert (fun ty -> isAnonRecdTy g ty || isTyparTy g ty) cenv overallTy env mWholeExpr (fun overallTy ->
59035902
TcAnonRecdExpr cenv overallTy env tpenv (isStruct, withExprOpt, unsortedFieldExprs, mWholeExpr)
@@ -5929,10 +5928,9 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE
59295928
let binds = unionBindingAndMembers binds members
59305929
TcExprObjectExpr cenv overallTy env tpenv (synObjTy, argopt, binds, extraImpls, mNewExpr, m)
59315930

5932-
| SynExpr.Record (inherits, withExprOpt, synRecdFields, mWholeExpr) ->
5931+
| SynExpr.Record (inherits, withExprOpt, synRecdFields, mWholeExpr) ->
59335932
match withExprOpt with
5934-
| None
5935-
| Some(SynExpr.Ident _, _) ->
5933+
| None | IsSimpleOrBoundExpr ->
59365934
TcNonControlFlowExpr env <| fun env ->
59375935
TcExprRecord cenv overallTy env tpenv (inherits, withExprOpt, synRecdFields, mWholeExpr)
59385936
| Some withExpr ->

tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,3 +480,56 @@ if actual <> expected then
480480
|> withLangVersion80
481481
|> compileExeAndRun
482482
|> verifyOutput "once"
483+
484+
[<Fact>]
485+
let ``N-Nested copy-and-update works when the starting expression is not a simple identifier``() =
486+
FSharp """
487+
module CopyAndUpdateTests
488+
type SubSubTest = {
489+
Z: int
490+
}
491+
492+
type SubTest = {
493+
Y: SubSubTest
494+
}
495+
496+
type Test = {
497+
X: SubTest
498+
}
499+
500+
let getTest () =
501+
{ X = { Y = { Z = 0 } } }
502+
503+
[<EntryPoint>]
504+
let main argv =
505+
let a = {
506+
getTest () with
507+
X.Y.Z = 1
508+
}
509+
printfn "%i" a.X.Y.Z |> ignore
510+
0
511+
"""
512+
|> typecheck
513+
|> shouldSucceed
514+
|> verifyOutput "1"
515+
516+
[<Fact>]
517+
let ``N-Nested, anonymous copy-and-update works when the starting expression is not a simple identifier``() =
518+
FSharp """
519+
module CopyAndUpdateTests
520+
521+
let getTest () =
522+
{| X = {| Y = {| Z = 0 |} |} |}
523+
524+
[<EntryPoint>]
525+
let main argv =
526+
let a = {|
527+
getTest () with
528+
X.Y.Z = 1
529+
|}
530+
printfn "%i" a.X.Y.Z |> ignore
531+
0
532+
"""
533+
|> typecheck
534+
|> shouldSucceed
535+
|> verifyOutput "1"

0 commit comments

Comments
 (0)