janestreet / base

Standard library for OCaml
MIT License
860 stars 125 forks source link

Name the "equal" argument of List.equal #115

Closed benjub closed 3 years ago

benjub commented 3 years ago

When a function uses an equality function as an argument, that argument is generally named "equal" in Base. This is currently not the case for the function List.equal, as its signature val equal : ('a -> 'a -> bool) -> 'a t -> 'a t -> bool shows. Changing the current definition "let equal..." in list.ml to

let equal ~equal t1 t2 =
  let rec loop t1 t2 =
    match t1, t2 with
    | [], [] -> true
    | x1 :: t1, x2 :: t2 -> equal x1 x2 && loop t1 t2
    | _ -> false
  in
  loop t1 t2

seems to do the job. Additionally, it removes the extra argument that was used for loop. Unless there is something fishy going on with a possible clash between the two things named "equal" ? It seems ok since we have "let equal" and not "let rec equal" so the equal within the function can only refer to the argument.

bcc32 commented 3 years ago

Hi @benjub,

The reason List.equal has this type is because it needs to conform to the type convention for [@@deriving equal]. There isn't a name clash issue in the implementation. IIRC, the reason for the extra argument in the loop function is to ensure that equal doesn't allocate a closure.

You can see the convention in the Base.Equal signatures:


type 'a t = 'a -> 'a -> bool
type 'a equal = 'a t

module type S = sig
  type t

  val equal : t equal
end

module type S1 = sig
  type 'a t

  val equal : 'a equal -> 'a t equal
end

and so on.