Remove unnecessary type variables

[mrmr1993]

In types that are used by snarky, we've historically used a type variable for each type appearing in a field. These type variables are never actually useful to us, make type errors hard to read, and make type discovery far more opaque.

To get the same level of field sharing, we should consider converting e.g.

(* src/lib/mina_state/protocol_state.ml *)
module Body = struct
  module Poly = struct
    [%%versioned
    module Stable = struct
      module V1 = struct
        type ('state_hash, 'blockchain_state, 'consensus_state, 'constants) t =
          { genesis_state_hash: 'state_hash
          ; blockchain_state: 'blockchain_state
          ; consensus_state: 'consensus_state
          ; constants: 'constants }
      end
    end]
  end

  module Value = struct
    [%%versioned
    module Stable = struct
      module V1 = struct
        type t =
          ( State_hash.Stable.V1.t
          , Blockchain_state.Value.Stable.V1.t
          , Consensus.Data.Consensus_state.Value.Stable.V1.t
          , Protocol_constants_checked.Value.Stable.V1.t )
          Poly.Stable.V1.t
        [@@deriving eq, ord, bin_io, hash, sexp, yojson, version]

        let to_latest = Fn.id
      end
    end]
  end

  type ('state_hash, 'blockchain_state, 'consensus_state, 'constants) t =
    ('state_hash, 'blockchain_state, 'consensus_state, 'constants) Poly.t

  type value = Value.t [@@deriving sexp, yojson]

  type var =
    ( State_hash.var
    , Blockchain_state.var
    , Consensus.Data.Consensus_state.var
    , Protocol_constants_checked.var )
    Poly.t

  ...
end

to use functors to give concrete types

module Body = struct
  module Poly (State_hash : TYPE) (Blockchain_state : TYPE) (Consensus_state : TYPE) (Constants : TYPE) = struct
      type t =
        { genesis_state_hash: State_hash.t
        ; blockchain_state: Blockchain_state.t
        ; consensus_state: Consensus_state.t
        ; constants: Constants.t }
      [@@deriving sexp, eq, compare, yojson, hash, version, hlist]
      end
    end]
  end

  module Value = struct
    [%%versioned
    module Stable = struct
      module V2 = struct
        type t =
          Poly(State_hash.Stable.V1)(Blockchain_state.Value.Stable.V1)
            (Consensus.Data.Consensus_state.Value.Stable.V1)(Protocol_constants_checked.Value.Stable.V1)
          .t =
          { genesis_state_hash: State_hash.Value.Stable.V1.t
          ; blockchain_state: Blockchain_state.Value.Stable.V1.t
          ; consensus_state: Consensus.Data.Consensus_state.Value.Stable.V1.t
          ; constants: Protocol_constants_checked.Value.Stable.V1.t }
        [@@deriving eq, ord, bin_io, hash, sexp, yojson, version]

        let to_latest = Fn.id
      end
    end]
  end

  type value = Value.t [@@deriving sexp, yojson]

  type var =
    Poly(State_hash.Var)(Blockchain_state.Var)(Consensus.Data.Consensus_state.Var)
      (Protocol_constants_checked.Var).t =
    { genesis_state_hash: State_hash.Var.t
    ; blockchain_state: Blockchain_state.Var.t
    ; consensus_state: Consensus.Data.Consensus_state.Var.t
    ; constants: Protocol_constants_checked.Var.t }
end

Of particular benefit are:

• MerlinTypeOf in your editor of choice will tell you about the record's fields and their types directly
• we don't need to add explicit annotations to ensure that the polymorphic type is instantiated with the right field types
• we don't need to explicitly refer to the path of the polymorphic record's fields (e.g. {Protocol_state.Poly.genesis_state_hash= ...; ...})
• we can still convert between types fairly easily using the Poly functor
• type declarations are far more informative without becoming hugely longer.

[psteckler]

8581 is a different approach to this issue. I'd say that using a functor is more flexible, and much less difficult to implement.

[psteckler]

The example functor returns a versioned type, which the ppx lint rules currently reject.

It's fine, in fact.

[psteckler]

Making these changes requires a hard fork, since the version byte in serializations introduced by the versioned Poly will disappear (which is a good thing).

We should probably "version" these functors (Poly_V1, etc.), since the functors may differ when creating versioned types.

[mrmr1993]

Making these changes requires a hard fork, since the version byte in serializations introduced by the versioned Poly will disappear (which is a good thing).

Completely agree, and looking forward to this 😁

We should probably "version" these functors (Poly_V1, etc.), since the functors may differ when creating versioned types.

I'm not sure how necessary that is if we expand the aliases, since the linter will be able to check the actual structure of the type when it's expanded. (If ocaml/ocaml#10777 is considered for merge, we might be able to achieve this with polys instead of functors too, but I don't expect that any time soon.)

[psteckler]

I'm not sure how necessary that is if we expand the aliases

Right, if we keep around older versions, we may need multiple Poly functors, and it would be good to have a systematic way to distinguish them.

[psteckler]

We should have a standard module type name for the functor arguments, like Poly_arg. Unfortunately, the actual type will differ from case to case, because of the deriving contents, so we can't declare such a type once and for all. For Account.t, a signature that works is

sig
  type t [@@deriving sexp, equal, compare, hash, yojson]
end

[psteckler]

We could use a ppx like:

[%%sig_type_derives sexp, equal, compare, hash, json]

to create such module types. We could also have the functor version number as an argument.