From 9e733193d4aaede819ca997e740870ca0ea346fb Mon Sep 17 00:00:00 2001 From: evgTSV Date: Sat, 4 Oct 2025 01:21:34 +0500 Subject: [PATCH 1/3] Fix: correct nested field checking for records/anonymous records, add tests, minor refactoring --- .../Checking/CheckRecordSyntaxHelpers.fs | 4 +- .../Checking/CheckRecordSyntaxHelpers.fsi | 2 + .../Checking/Expressions/CheckExpressions.fs | 18 ++++++ .../Language/CopyAndUpdateTests.fs | 56 +++++++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs index 861ad5587c6..fefeafa023c 100644 --- a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs +++ b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs @@ -156,12 +156,14 @@ let TransformAstForNestedUpdates (cenv: TcFileState) (env: TcEnv) overallTy (lid (accessIds, outerFieldId), Some(synExprRecd (recdExprCopyInfo (fields |> List.map fst) withExpr) outerFieldId rest exprBeingAssigned) +let BindIdText = "bind@" + /// When the original expression in copy-and-update is more complex than `{ x with ... }`, like `{ f () with ... }`, /// we bind it first, so that it's not evaluated multiple times during a nested update let BindOriginalRecdExpr (withExpr: SynExpr * BlockSeparator) mkRecdExpr = let originalExpr, blockSep = withExpr let mOrigExprSynth = originalExpr.Range.MakeSynthetic() - let id = mkSynId mOrigExprSynth "bind@" + let id = mkSynId mOrigExprSynth BindIdText let withExpr = SynExpr.Ident id, blockSep let binding = diff --git a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi index 4e4f40d7504..93cbac45bdf 100644 --- a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi +++ b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi @@ -19,5 +19,7 @@ val TransformAstForNestedUpdates<'a> : withExpr: SynExpr * (range * 'a) -> (Ident list * Ident) * SynExpr option +val BindIdText: string + val BindOriginalRecdExpr: withExpr: SynExpr * BlockSeparator -> mkRecdExpr: ((SynExpr * BlockSeparator) option -> SynExpr) -> SynExpr diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 6979f20f0e0..06327c92613 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -5902,6 +5902,16 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE TcPossiblyPropagatingExprLeafThenConvert (fun ty -> isAnonRecdTy g ty || isTyparTy g ty) cenv overallTy env mWholeExpr (fun overallTy -> TcAnonRecdExpr cenv overallTy env tpenv (isStruct, withExprOpt, unsortedFieldExprs, mWholeExpr) ) + | Some(SynExpr.LongIdent (_, lId, _, _), _) as Some withExpr -> + let lIds = lId.LongIdent + if lIds |> List.tryFind (fun id -> id.idText = BindIdText) |> _.IsSome then + TcNonControlFlowExpr env <| fun env -> + TcPossiblyPropagatingExprLeafThenConvert (fun ty -> isAnonRecdTy g ty || isTyparTy g ty) cenv overallTy env mWholeExpr (fun overallTy -> + TcAnonRecdExpr cenv overallTy env tpenv (isStruct, withExprOpt, unsortedFieldExprs, mWholeExpr) + ) + else + BindOriginalRecdExpr withExpr (fun withExpr -> SynExpr.AnonRecd (isStruct, withExpr, unsortedFieldExprs, mWholeExpr, trivia)) + |> TcExpr cenv overallTy env tpenv | Some withExpr -> BindOriginalRecdExpr withExpr (fun withExpr -> SynExpr.AnonRecd (isStruct, withExpr, unsortedFieldExprs, mWholeExpr, trivia)) |> TcExpr cenv overallTy env tpenv @@ -5935,6 +5945,14 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE | Some(SynExpr.Ident _, _) -> TcNonControlFlowExpr env <| fun env -> TcExprRecord cenv overallTy env tpenv (inherits, withExprOpt, synRecdFields, mWholeExpr) + | Some(SynExpr.LongIdent (_, lId, _, _), _) as Some withExpr -> + let lIds = lId.LongIdent + if lIds |> List.tryFind (fun id -> id.idText = "bind@") |> _.IsSome then + TcNonControlFlowExpr env <| fun env -> + TcExprRecord cenv overallTy env tpenv (inherits, withExprOpt, synRecdFields, mWholeExpr) + else + BindOriginalRecdExpr withExpr (fun withExpr -> SynExpr.Record (inherits, withExpr, synRecdFields, mWholeExpr)) + |> TcExpr cenv overallTy env tpenv | Some withExpr -> BindOriginalRecdExpr withExpr (fun withExpr -> SynExpr.Record (inherits, withExpr, synRecdFields, mWholeExpr)) |> TcExpr cenv overallTy env tpenv diff --git a/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs index 1e29223eed4..0b1a617ff39 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs @@ -480,3 +480,59 @@ if actual <> expected then |> withLangVersion80 |> compileExeAndRun |> verifyOutput "once" + +[] +let ``N-Nested copy-and-update works when the starting expression is not a simple identifier``() = + FSharp """ +module CopyAndUpdateTests +type SubSubTest = { + Z: int +} + +type SubTest = { + Y: SubSubTest +} + +type Test = { + X: SubTest +} + +let getTest () = + { X = { Y = { Z = 0 } } } + +[] +let main argv = + let a = { + getTest () with + X.Y.Z = 1 + } + printfn "%i" a.X.Y.Z |> ignore + 0 + """ + |> withLangVersion80 + |> typecheck + |> shouldSucceed + |> verifyOutput "1" + +[] +let ``N-Nested, anonymous copy-and-update works when the starting expression is not a simple identifier``() = + FSharp """ +module CopyAndUpdateTests + +let getTest () = + {| X = {| Y = {| Z = 0 |} |} |} + +[] +let main argv = + let a = 1 + let a = {| + getTest () with + X.Y.Z = 1 + |} + printfn "%i" a.X.Y.Z |> ignore + 0 + """ + |> withLangVersion80 + |> typecheck + |> shouldSucceed + |> verifyOutput "1" \ No newline at end of file From 95dbb46eb01520df114f941b1a3c157eea087077 Mon Sep 17 00:00:00 2001 From: evgTSV Date: Sat, 4 Oct 2025 13:02:05 +0500 Subject: [PATCH 2/3] Refactor record expr handling --- .../Checking/CheckRecordSyntaxHelpers.fs | 13 +++++++ .../Checking/CheckRecordSyntaxHelpers.fsi | 3 ++ .../Checking/Expressions/CheckExpressions.fs | 34 ++++--------------- .../Language/CopyAndUpdateTests.fs | 1 - 4 files changed, 23 insertions(+), 28 deletions(-) diff --git a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs index fefeafa023c..4dd6986a546 100644 --- a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs +++ b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs @@ -158,6 +158,19 @@ let TransformAstForNestedUpdates (cenv: TcFileState) (env: TcEnv) overallTy (lid let BindIdText = "bind@" +let IsNoneOrSimpleOrBindedExpr (withExprOpt: (SynExpr * BlockSeparator) option) = + match withExprOpt with + | None -> true + | Some (expr, _) -> + match expr with + | SynExpr.LongIdent (_, lIds, _, _) -> + lIds.LongIdent + |> List.tryFind (fun id -> id.idText = BindIdText) + |> _.IsSome + + | SynExpr.Ident _ -> true + | _ -> false + /// When the original expression in copy-and-update is more complex than `{ x with ... }`, like `{ f () with ... }`, /// we bind it first, so that it's not evaluated multiple times during a nested update let BindOriginalRecdExpr (withExpr: SynExpr * BlockSeparator) mkRecdExpr = diff --git a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi index 93cbac45bdf..fc0dc8f5ee2 100644 --- a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi +++ b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi @@ -21,5 +21,8 @@ val TransformAstForNestedUpdates<'a> : val BindIdText: string +val IsNoneOrSimpleOrBindedExpr: + withExprOpt: (SynExpr * BlockSeparator) option -> bool + val BindOriginalRecdExpr: withExpr: SynExpr * BlockSeparator -> mkRecdExpr: ((SynExpr * BlockSeparator) option -> SynExpr) -> SynExpr diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 06327c92613..1bff42cbe86 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -5895,24 +5895,13 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE TcExprTuple cenv overallTy env tpenv (isExplicitStruct, args, m) | SynExpr.AnonRecd (isStruct, withExprOpt, unsortedFieldExprs, mWholeExpr, trivia) -> - match withExprOpt with - | None - | Some(SynExpr.Ident _, _) -> + if IsNoneOrSimpleOrBindedExpr withExprOpt then TcNonControlFlowExpr env <| fun env -> TcPossiblyPropagatingExprLeafThenConvert (fun ty -> isAnonRecdTy g ty || isTyparTy g ty) cenv overallTy env mWholeExpr (fun overallTy -> TcAnonRecdExpr cenv overallTy env tpenv (isStruct, withExprOpt, unsortedFieldExprs, mWholeExpr) ) - | Some(SynExpr.LongIdent (_, lId, _, _), _) as Some withExpr -> - let lIds = lId.LongIdent - if lIds |> List.tryFind (fun id -> id.idText = BindIdText) |> _.IsSome then - TcNonControlFlowExpr env <| fun env -> - TcPossiblyPropagatingExprLeafThenConvert (fun ty -> isAnonRecdTy g ty || isTyparTy g ty) cenv overallTy env mWholeExpr (fun overallTy -> - TcAnonRecdExpr cenv overallTy env tpenv (isStruct, withExprOpt, unsortedFieldExprs, mWholeExpr) - ) - else - BindOriginalRecdExpr withExpr (fun withExpr -> SynExpr.AnonRecd (isStruct, withExpr, unsortedFieldExprs, mWholeExpr, trivia)) - |> TcExpr cenv overallTy env tpenv - | Some withExpr -> + else + let withExpr = withExprOpt.Value BindOriginalRecdExpr withExpr (fun withExpr -> SynExpr.AnonRecd (isStruct, withExpr, unsortedFieldExprs, mWholeExpr, trivia)) |> TcExpr cenv overallTy env tpenv @@ -5939,21 +5928,12 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE let binds = unionBindingAndMembers binds members TcExprObjectExpr cenv overallTy env tpenv (synObjTy, argopt, binds, extraImpls, mNewExpr, m) - | SynExpr.Record (inherits, withExprOpt, synRecdFields, mWholeExpr) -> - match withExprOpt with - | None - | Some(SynExpr.Ident _, _) -> + | SynExpr.Record (inherits, withExprOpt, synRecdFields, mWholeExpr) -> + if IsNoneOrSimpleOrBindedExpr withExprOpt then TcNonControlFlowExpr env <| fun env -> TcExprRecord cenv overallTy env tpenv (inherits, withExprOpt, synRecdFields, mWholeExpr) - | Some(SynExpr.LongIdent (_, lId, _, _), _) as Some withExpr -> - let lIds = lId.LongIdent - if lIds |> List.tryFind (fun id -> id.idText = "bind@") |> _.IsSome then - TcNonControlFlowExpr env <| fun env -> - TcExprRecord cenv overallTy env tpenv (inherits, withExprOpt, synRecdFields, mWholeExpr) - else - BindOriginalRecdExpr withExpr (fun withExpr -> SynExpr.Record (inherits, withExpr, synRecdFields, mWholeExpr)) - |> TcExpr cenv overallTy env tpenv - | Some withExpr -> + else + let withExpr = withExprOpt.Value BindOriginalRecdExpr withExpr (fun withExpr -> SynExpr.Record (inherits, withExpr, synRecdFields, mWholeExpr)) |> TcExpr cenv overallTy env tpenv diff --git a/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs index 0b1a617ff39..c31b6a3c574 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs @@ -524,7 +524,6 @@ let getTest () = [] let main argv = - let a = 1 let a = {| getTest () with X.Y.Z = 1 From 0296f68ec90273be7d9995513c97ac8348ddaf22 Mon Sep 17 00:00:00 2001 From: evgTSV Date: Sat, 11 Oct 2025 00:18:01 +0500 Subject: [PATCH 3/3] Removed explicit lang version since current default is sufficient. Fix typo in func name. --- src/Compiler/Checking/CheckRecordSyntaxHelpers.fs | 2 +- src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi | 2 +- src/Compiler/Checking/Expressions/CheckExpressions.fs | 4 ++-- .../Language/CopyAndUpdateTests.fs | 2 -- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs index 4dd6986a546..66f33323621 100644 --- a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs +++ b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fs @@ -158,7 +158,7 @@ let TransformAstForNestedUpdates (cenv: TcFileState) (env: TcEnv) overallTy (lid let BindIdText = "bind@" -let IsNoneOrSimpleOrBindedExpr (withExprOpt: (SynExpr * BlockSeparator) option) = +let IsNoneOrSimpleOrBoundExpr (withExprOpt: (SynExpr * BlockSeparator) option) = match withExprOpt with | None -> true | Some (expr, _) -> diff --git a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi index fc0dc8f5ee2..7fa70c941dc 100644 --- a/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi +++ b/src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi @@ -21,7 +21,7 @@ val TransformAstForNestedUpdates<'a> : val BindIdText: string -val IsNoneOrSimpleOrBindedExpr: +val IsNoneOrSimpleOrBoundExpr: withExprOpt: (SynExpr * BlockSeparator) option -> bool val BindOriginalRecdExpr: diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 1bff42cbe86..344f07b190f 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -5895,7 +5895,7 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE TcExprTuple cenv overallTy env tpenv (isExplicitStruct, args, m) | SynExpr.AnonRecd (isStruct, withExprOpt, unsortedFieldExprs, mWholeExpr, trivia) -> - if IsNoneOrSimpleOrBindedExpr withExprOpt then + if IsNoneOrSimpleOrBoundExpr withExprOpt then TcNonControlFlowExpr env <| fun env -> TcPossiblyPropagatingExprLeafThenConvert (fun ty -> isAnonRecdTy g ty || isTyparTy g ty) cenv overallTy env mWholeExpr (fun overallTy -> TcAnonRecdExpr cenv overallTy env tpenv (isStruct, withExprOpt, unsortedFieldExprs, mWholeExpr) @@ -5929,7 +5929,7 @@ and TcExprUndelayed (cenv: cenv) (overallTy: OverallTy) env tpenv (synExpr: SynE TcExprObjectExpr cenv overallTy env tpenv (synObjTy, argopt, binds, extraImpls, mNewExpr, m) | SynExpr.Record (inherits, withExprOpt, synRecdFields, mWholeExpr) -> - if IsNoneOrSimpleOrBindedExpr withExprOpt then + if IsNoneOrSimpleOrBoundExpr withExprOpt then TcNonControlFlowExpr env <| fun env -> TcExprRecord cenv overallTy env tpenv (inherits, withExprOpt, synRecdFields, mWholeExpr) else diff --git a/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs b/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs index c31b6a3c574..b579c8b0188 100644 --- a/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs +++ b/tests/FSharp.Compiler.ComponentTests/Language/CopyAndUpdateTests.fs @@ -509,7 +509,6 @@ let main argv = printfn "%i" a.X.Y.Z |> ignore 0 """ - |> withLangVersion80 |> typecheck |> shouldSucceed |> verifyOutput "1" @@ -531,7 +530,6 @@ let main argv = printfn "%i" a.X.Y.Z |> ignore 0 """ - |> withLangVersion80 |> typecheck |> shouldSucceed |> verifyOutput "1" \ No newline at end of file