Skip to content

Commit

Permalink
Nested Record Field Copy and Update (#14821)
Browse files Browse the repository at this point in the history
Co-authored-by: Tomas Grosup <tomasgrosup@microsoft.com>
  • Loading branch information
kerams and T-Gro authored Mar 20, 2023
1 parent db5aca9 commit a6563cd
Show file tree
Hide file tree
Showing 42 changed files with 1,087 additions and 93 deletions.
83 changes: 50 additions & 33 deletions src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ open FSharp.Compiler
open FSharp.Compiler.AbstractIL.IL
open FSharp.Compiler.AccessibilityLogic
open FSharp.Compiler.AttributeChecking
open FSharp.Compiler.CompilerGlobalState
open FSharp.Compiler.CheckBasics
open FSharp.Compiler.CheckRecordSyntaxHelpers
open FSharp.Compiler.ConstraintSolver
open FSharp.Compiler.DiagnosticsLogger
open FSharp.Compiler.Features
Expand Down Expand Up @@ -6568,7 +6568,9 @@ and TcRecordConstruction (cenv: cenv) (overallTy: TType) env tpenv withExprInfoO
if not (Zset.subset ns2 ns1) then
error(MissingFields(Zset.elements (Zset.diff ns2 ns1), m))
| _ ->
if oldFldsList.IsEmpty then
// `TransformAstForNestedUpdates` creates record constructions with synthetic ranges.
// Don't emit the warning for nested field updates, because it does not really make sense.
if oldFldsList.IsEmpty && not m.IsSynthetic then
let enabledByLangFeature = g.langVersion.SupportsFeature LanguageFeature.WarningWhenCopyAndUpdateRecordChangesAllFields
warning(ErrorEnabledWithLanguageFeature(FSComp.SR.tcCopyAndUpdateRecordChangesAllFields(fullDisplayTextOfTyconRef tcref), m, enabledByLangFeature))

Expand Down Expand Up @@ -7294,37 +7296,40 @@ and TcAssertExpr cenv overallTy env (m: range) tpenv x =

TcExpr cenv overallTy env tpenv callDiagnosticsExpr

and TcRecdExpr cenv (overallTy: TType) env tpenv (inherits, withExprOpt, synRecdFields, mWholeExpr) =

and TcRecdExpr cenv overallTy env tpenv (inherits, withExprOpt, synRecdFields, mWholeExpr) =
let g = cenv.g

let requiresCtor = (GetCtorShapeCounter env = 1) // Get special expression forms for constructors
let haveCtor = Option.isSome inherits

let withExprOpt, tpenv =
match withExprOpt with
| None -> None, tpenv
| Some (origExpr, _) ->
match inherits with
| Some (_, _, mInherits, _, _) -> error(Error(FSComp.SR.tcInvalidRecordConstruction(), mInherits))
| None ->
let withExpr, tpenv = TcExpr cenv (MustEqual overallTy) env tpenv origExpr
Some withExpr, tpenv
let withExprOptChecked, tpenv =
match withExprOpt with
| None -> None, tpenv
| Some (origExpr, _) ->
match inherits with
| Some (_, _, mInherits, _, _) -> error(Error(FSComp.SR.tcInvalidRecordConstruction(), mInherits))
| None ->
let withExpr, tpenv = TcExpr cenv (MustEqual overallTy) env tpenv origExpr
Some withExpr, tpenv

let hasOrigExpr = withExprOpt.IsSome
let hasOrigExpr = withExprOptChecked.IsSome

let fldsList =
let flds =
[
synRecdFields
|> List.map (fun (SynExprRecordField (fieldName = (synLongId, isOk); expr = exprBeingAssigned)) ->
// if we met at least one field that is not syntactically correct - raise ReportedError to transfer control to the recovery routine
for SynExprRecordField(fieldName=(synLongId, isOk); expr=v) in synRecdFields do
if not isOk then
// raising ReportedError None transfers control to the closest errorRecovery point but do not make any records into log
// we assume that parse errors were already reported
raise (ReportedError None)
if not isOk then
// raising ReportedError None transfers control to the closest errorRecovery point but do not make any records into log
// we assume that parse errors were already reported
raise (ReportedError None)

yield (List.frontAndBack synLongId.LongIdent, v)
]
match withExprOpt, synLongId.LongIdent, exprBeingAssigned with
| _, [ id ], _ -> ([], id), exprBeingAssigned
| Some withExpr, lid, Some exprBeingAssigned -> TransformAstForNestedUpdates cenv env overallTy lid exprBeingAssigned withExpr
| _ -> List.frontAndBack synLongId.LongIdent, exprBeingAssigned)

let flds = if hasOrigExpr then GroupUpdatesToNestedFields flds else flds

match flds with
| [] -> []
Expand All @@ -7339,7 +7344,7 @@ and TcRecdExpr cenv (overallTy: TType) env tpenv (inherits, withExprOpt, synRecd
| None -> () ]

let withExprInfoOpt =
match withExprOpt with
match withExprOptChecked with
| None -> None
| Some withExpr ->
let withExprAddrVal, withExprAddrValExpr = mkCompGenLocal mWholeExpr "inputRecord" (if isStructTy g overallTy then mkByrefTy g overallTy else overallTy)
Expand Down Expand Up @@ -7393,22 +7398,22 @@ and TcAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, optOrigSynExpr,

// Check for duplicate field IDs
unsortedFieldIdsAndSynExprsGiven
|> List.countBy (fun (fId, _, _) -> fId.idText)
|> List.countBy (fun (fId, _, _) -> textOfLid fId.LongIdent)
|> List.iter (fun (label, count) ->
if count > 1 then error (Error (FSComp.SR.tcAnonRecdDuplicateFieldId(label), mWholeExpr)))

match optOrigSynExpr with
| None ->
TcNewAnonRecdExpr cenv overallTy env tpenv (isStruct, unsortedFieldIdsAndSynExprsGiven, mWholeExpr)

| Some (origExpr, _) ->
TcCopyAndUpdateAnonRecdExpr cenv overallTy env tpenv (isStruct, origExpr, unsortedFieldIdsAndSynExprsGiven, mWholeExpr)
| Some orig ->
TcCopyAndUpdateAnonRecdExpr cenv overallTy env tpenv (isStruct, orig, unsortedFieldIdsAndSynExprsGiven, mWholeExpr)

and TcNewAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, unsortedFieldIdsAndSynExprsGiven, mWholeExpr) =

let g = cenv.g
let unsortedFieldSynExprsGiven = unsortedFieldIdsAndSynExprsGiven |> List.map (fun (_, _, fieldExpr) -> fieldExpr)
let unsortedFieldIds = unsortedFieldIdsAndSynExprsGiven |> List.map (fun (fieldId, _, _) -> fieldId) |> List.toArray
let unsortedFieldIds = unsortedFieldIdsAndSynExprsGiven |> List.map (fun (synLongIdent, _, _) -> synLongIdent.LongIdent[0]) |> List.toArray
let anonInfo, sortedFieldTys = UnifyAnonRecdTypeAndInferCharacteristics env.eContextInfo cenv env.DisplayEnv mWholeExpr overallTy isStruct unsortedFieldIds

// Sort into canonical order
Expand All @@ -7421,9 +7426,10 @@ and TcNewAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, unsortedField
let sigma = sortedIndexedArgs |> List.map fst |> List.toArray
let sortedFieldExprs = sortedIndexedArgs |> List.map snd

sortedFieldExprs |> List.iteri (fun j (fieldId, _, _) ->
let item = Item.AnonRecdField(anonInfo, sortedFieldTys, j, fieldId.idRange)
CallNameResolutionSink cenv.tcSink (fieldId.idRange, env.NameEnv, item, emptyTyparInst, ItemOccurence.Use, env.eAccessRights))
sortedFieldExprs |> List.iteri (fun j (synLongIdent, _, _) ->
let m = rangeOfLid synLongIdent.LongIdent
let item = Item.AnonRecdField(anonInfo, sortedFieldTys, j, m)
CallNameResolutionSink cenv.tcSink (m, env.NameEnv, item, emptyTyparInst, ItemOccurence.Use, env.eAccessRights))

let unsortedFieldTys =
sortedFieldTys
Expand All @@ -7437,7 +7443,7 @@ and TcNewAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, unsortedField

mkAnonRecd g mWholeExpr anonInfo unsortedFieldIds unsortedCheckedArgs unsortedFieldTys, tpenv

and TcCopyAndUpdateAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, origExpr, unsortedFieldIdsAndSynExprsGiven, mWholeExpr) =
and TcCopyAndUpdateAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, (origExpr, blockSeparator), unsortedFieldIdsAndSynExprsGiven, mWholeExpr) =
// The fairly complex case '{| origExpr with X = 1; Y = 2 |}'
// The origExpr may be either a record or anonymous record.
// The origExpr may be either a struct or not.
Expand All @@ -7448,7 +7454,6 @@ and TcCopyAndUpdateAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, ori
// Unlike in the case of record type copy-and-update {| a with X = 1 |} does not force a.X to exist or have had type 'int'

let g = cenv.g
let unsortedFieldSynExprsGiven = unsortedFieldIdsAndSynExprsGiven |> List.map (fun (_, _, e) -> e)
let origExprTy = NewInferenceType g
let origExprChecked, tpenv = TcExpr cenv (MustEqual origExprTy) env tpenv origExpr
let oldv, oldve = mkCompGenLocal mWholeExpr "inputRecord" origExprTy
Expand All @@ -7457,6 +7462,18 @@ and TcCopyAndUpdateAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, ori
if not (isAppTy g origExprTy || isAnonRecdTy g origExprTy) then
error (Error (FSComp.SR.tcCopyAndUpdateNeedsRecordType(), mOrigExpr))

// Expand expressions with respect to potential nesting
let unsortedFieldIdsAndSynExprsGiven =
unsortedFieldIdsAndSynExprsGiven
|> List.map (fun (synLongIdent, _, exprBeingAssigned) ->
match synLongIdent.LongIdent with
| [] -> error(Error(FSComp.SR.nrUnexpectedEmptyLongId(), mWholeExpr))
| [ id ] -> ([], id), Some exprBeingAssigned
| lid -> TransformAstForNestedUpdates cenv env origExprTy lid exprBeingAssigned (origExpr, blockSeparator))
|> GroupUpdatesToNestedFields

let unsortedFieldSynExprsGiven = unsortedFieldIdsAndSynExprsGiven |> List.choose snd

let origExprIsStruct =
match tryDestAnonRecdTy g origExprTy with
| ValueSome (anonInfo, _) -> evalTupInfoIsStruct anonInfo.TupInfo
Expand All @@ -7472,7 +7489,7 @@ and TcCopyAndUpdateAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, ori
/// - Choice2Of2 for a binding coming from the original expression
let unsortedIdAndExprsAll =
[|
for id, _, e in unsortedFieldIdsAndSynExprsGiven do
for (_, id), e in unsortedFieldIdsAndSynExprsGiven do
yield (id, Choice1Of2 e)
match tryDestAnonRecdTy g origExprTy with
| ValueSome (anonInfo, tinst) ->
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/CheckExpressions.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,7 @@ val BuildFieldMap:
env: TcEnv ->
isPartial: bool ->
ty: TType ->
((Ident list * Ident) * 'T) list ->
flds: ((Ident list * Ident) * 'T) list ->
m: range ->
TypeInst * TyconRef * Map<string, 'T> * (string * 'T) list

Expand Down
140 changes: 140 additions & 0 deletions src/Compiler/Checking/CheckRecordSyntaxHelpers.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.

module internal FSharp.Compiler.CheckRecordSyntaxHelpers

open FSharp.Compiler.CheckBasics
open FSharp.Compiler.DiagnosticsLogger
open FSharp.Compiler.Features
open FSharp.Compiler.NameResolution
open FSharp.Compiler.Syntax
open FSharp.Compiler.SyntaxTreeOps
open FSharp.Compiler.Text.Position
open FSharp.Compiler.Text.Range
open FSharp.Compiler.TypedTree

/// Merges updates to nested record fields on the same level in record copy-and-update.
///
/// `TransformAstForNestedUpdates` expands `{ x with A.B = 10; A.C = "" }`
///
/// into
///
/// { x with
/// A = { x.A with B = 10 };
/// A = { x.A with C = "" }
/// }
///
/// which we here convert to
///
/// { x with A = { x.A with B = 10; C = "" } }
let GroupUpdatesToNestedFields (fields: ((Ident list * Ident) * SynExpr option) list) =
let rec groupIfNested res xs =
match xs with
| [] -> res
| x :: [] -> x :: res
| x :: y :: ys ->
match x, y with
| (lidwid, Some (SynExpr.Record (baseInfo, copyInfo, aFlds, m))), (_, Some (SynExpr.Record (recordFields = bFlds))) ->
let reducedRecd =
(lidwid, Some(SynExpr.Record(baseInfo, copyInfo, aFlds @ bFlds, m)))

groupIfNested (reducedRecd :: res) ys
| (lidwid, Some (SynExpr.AnonRecd (isStruct, copyInfo, aFlds, m, trivia))), (_, Some (SynExpr.AnonRecd (recordFields = bFlds))) ->
let reducedRecd =
(lidwid, Some(SynExpr.AnonRecd(isStruct, copyInfo, aFlds @ bFlds, m, trivia)))

groupIfNested (reducedRecd :: res) ys
| _ -> groupIfNested (x :: res) (y :: ys)

fields
|> List.groupBy (fun ((_, field), _) -> field.idText)
|> List.collect (fun (_, fields) ->
if fields.Length < 2 then
fields
else
groupIfNested [] fields)

/// Expands a long identifier into nested copy-and-update expressions.
///
/// `{ x with A.B = 0 }` becomes `{ x with A = { x.A with B = 0 } }`
let TransformAstForNestedUpdates (cenv: TcFileState) env overallTy (lid: LongIdent) exprBeingAssigned withExpr =
let recdExprCopyInfo ids withExpr id =
let upToId origSepRng id lidwd =
let rec buildLid res (id: Ident) =
function
| [] -> res
| (h: Ident) :: t ->
// Mark these hidden field accesses as synthetic so that they don't make it
// into the name resolution sink.
let h = ident (h.idText, h.idRange.MakeSynthetic())

if equals h.idRange id.idRange then
h :: res
else
buildLid (h :: res) id t

let calcLidSeparatorRanges origSepRng lid =
match lid with
| []
| [ _ ] -> [ origSepRng ]
| _ :: t ->
origSepRng
:: List.map (fun (s: Ident, e: Ident) -> mkRange s.idRange.FileName s.idRange.End e.idRange.Start) t

let lid = buildLid [] id lidwd |> List.rev

(lid, List.pairwise lid |> calcLidSeparatorRanges origSepRng)

let totalRange (origId: Ident) (id: Ident) =
mkRange origId.idRange.FileName origId.idRange.End id.idRange.Start

let rangeOfBlockSeperator (id: Ident) =
let idEnd = id.idRange.End
let blockSeperatorStartCol = idEnd.Column
let blockSeperatorEndCol = blockSeperatorStartCol + 4
let blockSeperatorStartPos = mkPos idEnd.Line blockSeperatorStartCol
let blockSeporatorEndPos = mkPos idEnd.Line blockSeperatorEndCol

mkRange id.idRange.FileName blockSeperatorStartPos blockSeporatorEndPos

match withExpr with
| SynExpr.Ident origId, (sepRange, _) ->
let lid, rng = upToId sepRange id (origId :: ids)
Some(SynExpr.LongIdent(false, LongIdentWithDots(lid, rng), None, totalRange origId id), (rangeOfBlockSeperator id, None))
| _ -> None

let rec synExprRecd copyInfo (id: Ident) fields exprBeingAssigned =
match fields with
| [] -> failwith "unreachable"
| (fieldId, anonInfo) :: rest ->
let nestedField =
if rest.IsEmpty then
exprBeingAssigned
else
synExprRecd copyInfo fieldId rest exprBeingAssigned

let m = id.idRange.MakeSynthetic()

match anonInfo with
| Some {
AnonRecdTypeInfo.TupInfo = TupInfo.Const isStruct
} ->
let fields = [ LongIdentWithDots([ fieldId ], []), None, nestedField ]
SynExpr.AnonRecd(isStruct, copyInfo id, fields, m, { OpeningBraceRange = range0 })
| _ ->
let fields =
[
SynExprRecordField((LongIdentWithDots([ fieldId ], []), true), None, Some nestedField, None)
]

SynExpr.Record(None, copyInfo id, fields, m)

let access, fields =
ResolveNestedField cenv.tcSink cenv.nameResolver env.eNameResEnv env.eAccessRights overallTy lid

match access, fields with
| _, [] -> failwith "unreachable"
| accessIds, [ (fieldId, _) ] -> (accessIds, fieldId), Some exprBeingAssigned
| accessIds, (fieldId, _) :: rest ->
checkLanguageFeatureAndRecover cenv.g.langVersion LanguageFeature.NestedCopyAndUpdate (rangeOfLid lid)

(accessIds, fieldId), Some(synExprRecd (recdExprCopyInfo (fields |> List.map fst) withExpr) fieldId rest exprBeingAssigned)
20 changes: 20 additions & 0 deletions src/Compiler/Checking/CheckRecordSyntaxHelpers.fsi
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information.

module internal FSharp.Compiler.CheckRecordSyntaxHelpers

open FSharp.Compiler.CheckBasics
open FSharp.Compiler.Syntax
open FSharp.Compiler.Text
open FSharp.Compiler.TypedTree

val GroupUpdatesToNestedFields:
fields: ((Ident list * Ident) * SynExpr option) list -> ((Ident list * Ident) * SynExpr option) list

val TransformAstForNestedUpdates<'a> :
cenv: TcFileState ->
env: TcEnv ->
overallTy: TType ->
lid: LongIdent ->
exprBeingAssigned: SynExpr ->
withExpr: SynExpr * (range * 'a) ->
(Ident list * Ident) * SynExpr option
Loading

0 comments on commit a6563cd

Please sign in to comment.