Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested Record Field Copy and Update #14821

Merged
merged 41 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
f746733
Nested record copy and update
kerams Feb 25, 2023
c3b5c0b
Add completions support
kerams Feb 26, 2023
76e1c4e
Add initial support for anonymous records
kerams Feb 27, 2023
2f843bb
Extend anonymous record support
kerams Feb 27, 2023
c9a2d29
Format
kerams Feb 27, 2023
7af7cbc
Improve Intellisense for anonymous records
kerams Feb 28, 2023
04cff96
Guard nested copy-and-update with a languafe feature
kerams Feb 28, 2023
2a5024f
Fix tests
kerams Feb 28, 2023
5da0738
Merge branch 'main' of https://github.com/dotnet/fsharp into nest
kerams Feb 28, 2023
f145126
Refactor
kerams Feb 28, 2023
0081cea
Refactor
kerams Feb 28, 2023
f973574
Format
kerams Feb 28, 2023
670f5c5
Fix resolution priority
kerams Feb 28, 2023
71abfae
Fix and refactor
kerams Mar 1, 2023
496d369
Merge branch 'main' of https://github.com/dotnet/fsharp into nest
kerams Mar 1, 2023
d2eced1
Refactor
kerams Mar 1, 2023
39ea7c6
Add first batch of tests
kerams Mar 1, 2023
95236e7
Add field qualification test
kerams Mar 2, 2023
f9938ac
Fix updates on recursive records
kerams Mar 2, 2023
d79c431
Add more tests
kerams Mar 2, 2023
09fa90d
Add more tests
kerams Mar 2, 2023
684c454
Improve Intellisense
kerams Mar 3, 2023
28b3631
Format
kerams Mar 3, 2023
c23718d
Fix surface area
kerams Mar 3, 2023
ce76288
Merge branch 'main' of https://github.com/dotnet/fsharp into nest
kerams Mar 3, 2023
08f44b3
Refactor
kerams Mar 3, 2023
139833c
Add diagnostic tests
kerams Mar 4, 2023
cf26fbb
Deduplicate items in nested field update tooltips
kerams Mar 4, 2023
679155e
Refactor
kerams Mar 7, 2023
0f49fb5
Merge branch 'main' of https://github.com/dotnet/fsharp into nest
kerams Mar 7, 2023
6cccd10
Refactor
kerams Mar 11, 2023
4eb5c90
Merge branch 'main' of https://github.com/dotnet/fsharp into nest
kerams Mar 11, 2023
6f6997a
Refactor
kerams Mar 13, 2023
b6894c5
Address comments
kerams Mar 14, 2023
ce4b1b7
Merge branch 'main' of https://github.com/dotnet/fsharp into nest
kerams Mar 14, 2023
0868ffc
Ooops
kerams Mar 14, 2023
a3067c5
Merge branch 'main' of https://github.com/dotnet/fsharp into nest
kerams Mar 15, 2023
4fc7c3c
Address comments
kerams Mar 15, 2023
151f3d8
Merge branch 'main' of https://github.com/dotnet/fsharp into nest
kerams Mar 17, 2023
a8cae6f
Add test with dotted field
kerams Mar 17, 2023
ec113d4
Merge branch 'main' into nest
T-Gro Mar 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Compiler/Checking/CheckBasics.fs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ type TcEnv =

// Do we lay down an implicit debug point?
eIsControlFlow: bool

/// Type checking an expanded nested copy-and-update record expression
eIsInNestedCopyAndUpdate: bool
}

member tenv.DisplayEnv = tenv.eNameResEnv.DisplayEnv
Expand Down
3 changes: 3 additions & 0 deletions src/Compiler/Checking/CheckBasics.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ type TcEnv =
eLambdaArgInfos: ArgReprInfo list list

eIsControlFlow: bool

/// Type checking an expanded nested copy-and-update record expression
eIsInNestedCopyAndUpdate: bool
}

member DisplayEnv: DisplayEnv
Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -5226,7 +5226,8 @@ let emptyTcEnv g =
eCtorInfo = None
eCallerMemberName = None
eLambdaArgInfos = []
eIsControlFlow = false }
eIsControlFlow = false
eIsInNestedCopyAndUpdate = false }

let CreateInitialTcEnv(g, amap, scopem, assemblyName, ccus) =
(emptyTcEnv g, ccus) ||> List.collectFold (fun env (ccu, autoOpens, internalsVisible) ->
Expand Down
179 changes: 146 additions & 33 deletions src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1852,6 +1852,94 @@ let BuildFieldMap (cenv: cenv) env isPartial ty (flds: ((Ident list * Ident) * '
| _ -> error(Error(FSComp.SR.tcRecordFieldInconsistentTypes(), m)))
tinst, tcref, fldsmap, List.rev rfldsList

/// Merges updates to nested record fields on the same level in record copy-and-update
let GroupUpdatesToNestedFields (fields: (bool * (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
| (aIsNestedUpdate, lidwid, Some (SynExpr.Record (baseInfo, copyInfo, aFlds, m))), (bIsNestedUpdate, _, Some (SynExpr.Record (recordFields = bFlds))) ->
let reducedRecd = (aIsNestedUpdate || bIsNestedUpdate, lidwid, Some(SynExpr.Record (baseInfo, copyInfo, aFlds @ bFlds, m)))
groupIfNested (reducedRecd :: res) ys
| (aIsNestedUpdate, lidwid, Some (SynExpr.AnonRecd (isStruct, copyInfo, aFlds, m, trivia))), (bIsNestedUpdate, _, Some (SynExpr.AnonRecd (recordFields = bFlds))) ->
let reducedRecd = (aIsNestedUpdate || bIsNestedUpdate, 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
let TransformAstForNestedUpdates cenv env overallTy (lid: LongIdent) v withExpr =
let recdExprCopyInfo ids withExpr id =
let upToId origSepRng id lidwd =
let rec buildLid res (id: Ident) =
function
| [] -> res
| (h: Ident) :: t -> if h.idText = id.idText then h :: res else buildLid (h :: res) id t

let rec combineIds =
function
| [] | [_] -> []
| id1::id2::rest -> (id1, id2) :: (id2 :: rest |> combineIds)

let calcLidSeparatorRanges 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, lid |> combineIds |> calcLidSeparatorRanges)

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)) // TODO: id.idRange should be the range of the next separator
| _ -> None

let rec synExprRecd copyInfo (id: Ident) fields v =
match fields with
| [] -> failwith "unreachable"
vzarytovskii marked this conversation as resolved.
Show resolved Hide resolved
| (fldId, isAnon) :: rest ->
// todo the unit
let nestedField = if rest.IsEmpty then Option.defaultValue (mkSynUnit range0) v else synExprRecd copyInfo fldId rest v

if isAnon then
// The correct structness will later be taken from the anynymous type, which already exists
SynExpr.AnonRecd(false, copyInfo id, [ ([ fldId ], None, nestedField) ], id.idRange, { OpeningBraceRange = range0 })
else
SynExpr.Record(None, copyInfo id, [ SynExprRecordField((LongIdentWithDots ([ fldId ], []), true), None, Some nestedField, None) ], id.idRange)

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

match access, flds with
| [], [] -> None
| accessIds, [] -> Some (false, List.frontAndBack accessIds, v)
| accessIds, [ (fldId, _) ] -> Some (false, List.frontAndBack (accessIds @ [ fldId ]), v)
| accessIds, (fldId, _) :: rest ->
// todo remove the other some
Some (true, (accessIds, fldId), Some (synExprRecd (recdExprCopyInfo (flds |> List.map fst) withExpr) fldId rest v))

let rec ApplyUnionCaseOrExn (makerForUnionCase, makerForExnTag) m (cenv: cenv) env overallTy item =
let g = cenv.g
let ad = env.eAccessRights
Expand Down Expand Up @@ -6565,7 +6653,7 @@ 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
if oldFldsList.IsEmpty && not env.eIsInNestedCopyAndUpdate then
let enabledByLangFeature = g.langVersion.SupportsFeature LanguageFeature.WarningWhenCopyAndUpdateRecordChangesAllFields
warning(ErrorEnabledWithLanguageFeature(FSComp.SR.tcCopyAndUpdateRecordChangesAllFields(fullDisplayTextOfTyconRef tcref), m, enabledByLangFeature))

Expand Down Expand Up @@ -7291,52 +7379,58 @@ 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 fldsList, containsNestedUpdates =
let flds =
[
synRecdFields
|> List.choose (fun (SynExprRecordField (fieldName = (synLongId, isOk); expr = v)) ->
// 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 with
| Some withExpr ->
match synLongId.LongIdent with
| [] -> None
| [ id ] -> Some (false, ([], id), v)
| lid -> TransformAstForNestedUpdates cenv env overallTy lid v withExpr
| _ -> Some (false, List.frontAndBack synLongId.LongIdent, v))

let flds = if hasOrigExpr then GroupUpdatesToNestedFields flds else flds

match flds with
| [] -> []
| [] -> [], false
| _ ->
let tinst, tcref, _, fldsList = BuildFieldMap cenv env hasOrigExpr overallTy flds mWholeExpr
let tinst, tcref, _, fldsList = BuildFieldMap cenv env hasOrigExpr overallTy (flds |> List.map (fun (_, x, y) -> x, y)) mWholeExpr
let gtyp = mkAppTy tcref tinst
UnifyTypes cenv env mWholeExpr overallTy gtyp

[ for n, v in fldsList do
match v with
| Some v -> yield n, v
| None -> () ]
| None -> () ], flds |> List.exists (fun (isNestedUpdate, _, _) -> isNestedUpdate)
kerams marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -7375,7 +7469,8 @@ and TcRecdExpr cenv (overallTy: TType) env tpenv (inherits, withExprOpt, synRecd
errorR(InternalError("Unexpected failure in getting super type", mWholeExpr))
None, tpenv

let expr, tpenv = TcRecordConstruction cenv overallTy env tpenv withExprInfoOpt overallTy fldsList mWholeExpr
let envinner = if containsNestedUpdates then { env with eIsInNestedCopyAndUpdate = true } else env
let expr, tpenv = TcRecordConstruction cenv overallTy envinner tpenv withExprInfoOpt overallTy fldsList mWholeExpr

let expr =
match superInitExprOpt with
Expand All @@ -7390,7 +7485,7 @@ 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)
|> List.iter (fun (label, count) ->
if count > 1 then error (Error (FSComp.SR.tcAnonRecdDuplicateFieldId(label), mWholeExpr)))

Expand All @@ -7405,7 +7500,7 @@ and TcNewAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, unsortedField

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 (fieldId, _, _) -> fieldId[0]) |> List.toArray
let anonInfo, sortedFieldTys = UnifyAnonRecdTypeAndInferCharacteristics env.eContextInfo cenv env.DisplayEnv mWholeExpr overallTy isStruct unsortedFieldIds

// Sort into canonical order
Expand All @@ -7419,8 +7514,9 @@ and TcNewAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, unsortedField
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))
let m = rangeOfLid fieldId
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 @@ -7445,7 +7541,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 @@ -7454,6 +7549,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.choose (fun (lid, _, e) ->
match lid with
| [] -> None
| [ id ] -> Some (false, ([], id), Some e) // todo remove options
| lid -> TransformAstForNestedUpdates cenv env origExprTy lid (Some e) (origExpr, (range0, range0)))
|> GroupUpdatesToNestedFields

let unsortedFieldSynExprsGiven = unsortedFieldIdsAndSynExprsGiven |> List.map (fun (_, _, e) -> e.Value) //todo

let origExprIsStruct =
match tryDestAnonRecdTy g origExprTy with
| ValueSome (anonInfo, _) -> evalTupInfoIsStruct anonInfo.TupInfo
Expand All @@ -7469,7 +7576,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 Expand Up @@ -7520,6 +7627,12 @@ and TcCopyAndUpdateAnonRecdExpr cenv (overallTy: TType) env tpenv (isStruct, ori

// Check the expressions in unsorted order
let unsortedFieldExprsGiven, tpenv =
let env =
if unsortedFieldIdsAndSynExprsGiven |> List.exists (fun (isNestedUpdate, _, _) -> isNestedUpdate) then
{ env with eIsInNestedCopyAndUpdate = true }
else
env

TcExprsWithFlexes cenv env mWholeExpr tpenv flexes unsortedFieldTysGiven unsortedFieldSynExprsGiven

let unsortedFieldExprsGiven = unsortedFieldExprsGiven |> List.toArray
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
Loading