Confusing merge behaviour in `std.record.{update,remove}`

[solson]

Confusing merge behaviour in `std.record.{update,remove}`

[yannham]

Thanks for reporting. This is definitely a bug. I think the original design guideline was that only merge could impact recursive fields, and as soon as you use a record as a "normal" dictionary (std.record.{update,remove,insert}), you don't get the recursive behavior (otherwise, you should just use merge instead - although remove can't be emulated with merge, but this is on purpose).

Following this guideline, I believe the right result should be the following:

nickel> std.record.update "a" 2 { a = 1, b = a }
{ a = 2, b = 1, }

nickel> std.record.update "a" 2 { a = 1, b = a } & {}
{ a = 2, b = 1, }

nickel> std.record.remove "a" { a = 1, b = a }
{ b = 1, }

nickel> std.record.remove "a" { a = 1, b = a } & {}
{ b = 1, }

I see two ways to achieve that:

1. We "freeze" the recursive record before any remove primop. We don't really care about insert: insert alone can't mess with existing dependencies, it's either remove alone or the combination of remove and insert that are problematic. This way, there's no more confusion: prior to a remove, the recursive fixpoint is computed once and then it's just a normal data dictionary like in Python with no more recursive references.
2. The drawback of the previous solution is that it makes it impossible to override or recompute any other value in the original record after a remove, even if it doesn't have anything to do with the removed value: (std.record.remove "a" {a = 1, b = c, c = 2}) & {c | force = 3} would give {b = 2, c = 3}, which might be surprising. We can freeze only the value of the field that is removed instead, and let other fields keep their recursive/overriding capabilities.

I think 1. is a simpler mental model: you don't have to think about what fields were removed or not, as soon as you use one remove operation, all recursive fields are frozen. 2. is probably more flexible, though.