In Batteries we haveupdate, which does something different, andmodify which does what is currently proposed.

update is the Map equivalent ofSet.S.update : elt -> elt -> t -> t, which is used to replace an element for another element that is equivalent for the given order (but may differ in irrelevant fields).Map.S.update is the lifted version of this, with the typekey -> key -> 'a -> 'a t -> 'a t, and is specified by the fact thatupdate k k' v m is only valid whenk andk' have the same ordering, and is then equivalent toadd k' v (remove k m).

modify comes in three variants:

valmodify:key -> ('a ->'a) ->'at ->'at(** [modify k f m] replaces the previous binding for [k] with [f] applied to      that value. If [k] is unbound in [m] or [Not_found] is raised during the      search, [Not_found] is raised.*)valmodify_def:'a ->key -> ('a ->'a) ->'at ->'at(** [modify_def v0 k f m] replaces the previous binding for [k]    with [f] applied to that value. If [k] is unbound in [m] or    [Not_found] is raised during the search, [f v0] is    inserted (as if the value found were [v0]).*)valmodify_opt:key -> ('aoption ->'aoption) ->'at ->'at(** [modify_opt k f m] allows to modify the binding for [k] in [m]      or absence thereof.*)

The Base library has two functions for value modification, the'a option -> 'a version and the'a option -> 'a option version.

val change  :  ('k, 'v, 'cmp) t  -> 'k  -> f:('v option -> 'v option)  -> ('k, 'v, 'cmp) tval update  :  ('k, 'v, 'cmp) t  -> 'k  -> f:('v option -> 'v)-> ('k, 'v, 'cmp) t

Finally, Containers only has the'a option -> 'a option version

  val update : key -> ('a option -> 'a option) -> 'a t -> 'a t

I think that if we are to add only one, the'a option -> 'a option version probably makes the most sense. It is a bit less efficient as it requires allocation ofSome blocks.

Should the name beupdate,change,modify or something else?

Finally, I think that changes to the standard library should come with unit tests. If that can help, here are themodify_opt tests in Batteries:

  let test_modify_opt () =    let sum t = M.fold (+) t 0 in    let t = il [(1, 2); (3, 4)] in    (* usage to modify values *)    let test1 k t =      "sum (modify_opt k (+1 or 2) t) = sum t + (mem k t ? 1 : 2)" @?        (sum (M.modify_opt k (function None -> Some 2 | Some x -> Some (x+1)) t) =         sum t + if M.mem k t then 1 else 2) in    test1 1 t;    test1 57 t;    (* usage to delete values *)    "modify_opt k (fun _ -> None) t -> remove k" @?      (M.modify_opt 3 (function Some _ -> None | None -> None) t |> M.mem 3 |> not);    "modify_opt k (fun _ -> None) (singleton k) -> empty" @?      (M.singleton 1 0 |> M.modify_opt 1 (fun _ -> None) |> M.is_empty);    (* usage to add values *)    "modify_opt k (fun None -> Some x) t -> add k" @?      (M.modify_opt 2 (function None -> Some 1 | _ -> assert false) t |> M.mem 2);    ()

I think that if we are to add only one, the 'a option -> 'a option version probably makes the most sense.

I would be in favor of adding this one and this oneonly.

Regarding the tests, what is the policy? Add a new file, or extend the existing test for maps (ie basic/maps.ml) ?

I don't know of a policy, that would be for the stdlib expert (@alainfrisch ) to decide. I'm fine with whichever, I thinkbasic/maps.ml is just fine.

(The one part of the policy that I feel strongly about is that each new addition to the stdlib should come with tests. It is even easier for me to say as the implementor of a buggy new function in 4.04.0 thatshould have been better tested.)

The stdlib expert is fine with whichever as well, but this is more a question for the testsuite expert.

@Sbrais Tests definitely OK in the existing file. The only reason to branch off to a separate file would be if the test is particularly long (or, obviously, requires an extra harness of its own).

Everything looks good to me. Will merge if nobody objects to it shortly.

@sbriais : can you add a reference to#7309 to the Change file?

Done. Should I fill also the "..." for the reviewers with all the names of the participants in this thread?

Should I fill also the "..." for the reviewers with all the names of the participants in this thread?

Only if you feel they have meaningfully contributed. For example, don't add me.

Rebased and merged on trunk (25105b2). Thanks@sbriais!