Warning for moving to an abstract type

Hello,

I have an API with a module of the form:

module M : sig 
   type t = string * int 
   val v : string -> int -> t
end

which I would like to eventually evolve to:

module M : sig 
  type t
  val v : string -> int -> t
end

Is there a mecanism (or perhaps a TRICK !) to warn on non-abstract uses of M.t ? I don’t think alerts can solve this.

4 Likes

Private types can help a bit:

module M : sig
  type t = private string * int

  val v : string -> int -> t
end = struct
  type t = string * int

  let v a b = a, b
end

let f (_ : string * int) = ()

let () = f (M.v "a" 1)
Error: This expression has type M.t but an expression was expected of type
         string * int

However, that’s a as-breaking change as just making the type abstract.

Edit: Alright, that’s not a warning but an error.

Not an answer to your question, but as a workaround maybe you could make the type abstract, provide an explicit coercion function from abstract to explicit, and then mark that function as deprecated?

But this is of course already a breaking change, so I assume it is not the answer you were looking for.

Another workaround (quite equivalent to the one that @smolkaj proposes, maybe a bit more direct, but still a breaking change) is to make the type t a variant with only one constructor, marked as deprecated.

type t = C of string * int [@alert deprecated "M.t should be considered abstract"]

Thanks all for your input but all breaking changes are not helping :–)

I mean if breaking is an option I can simply make the thing abstract and provide conversion helpers to ease the transition.

If I understand it correctly you want to be warned when you pattern match against M.t. Something like that may work:

#use "topfind";;
#require "compiler-libs.common";;

#directory "+ocamldoc";;
#require "str";;
#require "unix";;
#load "odoc_info.cma";;

open Tast_iterator;;
let iterator =
  let case : 'k. iterator -> 'k Typedtree.case -> unit =
    fun super case_expr ->
      let { Typedtree.c_lhs; _ } = case_expr in
      let { Typedtree.pat_type; pat_loc; _ } = c_lhs in
      let type_repr = Odoc_info.string_of_type_expr pat_type in
        if type_repr = "M.t" then
          Format.printf "Non abstract use of M.t at %a\n" Location.print_loc pat_loc;

        super.case super case_expr
  in
    { default_iterator with case }
;;

let process_file cmt_file = 
  let { Cmt_format.cmt_annots; _ } = Cmt_format.read_cmt cmt_file in
    match cmt_annots with
      | Cmt_format.Implementation structure ->
          iterator.structure iterator structure
      | _ -> print_endline "Ooops!"
;;

Not tested. Also, this may work with 4.12, 4.13 versions of OCaml, but you’ll need to modify for 4.11 and bellow.

You may even go extra-fancy and create a ppx extension so that you can integrate it in your build.

1 Like

Interesting, this suggests it may not be that hard to integrate such mecanism upstream. Could be worth opening an issue but these things tend to be eventually closed by the bot so I won’t bother.

Yes ppx is always extra-fancy, even too. In any case I do not have control over the build system of people who use Cmdliner :–)

I see. You want something like an [@internal] annotation on the type and warning from the compiler if it is used in a non-abstract way outside of the module where it is defined.

Or, maybe [@alert external "Message"]. I guess, there is no such thing for now.

I think what you’re describing is an attribute on type aliases that causes a warning to be emitted every time the type checker expands the alias.
I believe this hasn’t been implemented yet, and I’m not familiar enough with the type checker to know whether it would be easy or not (or even possible at all).
But implementing this feature looks like a good way for someone to start understanding to type checker’s code, so I think some of the maintainers would appreciate the feature request for this reason at least.

In principle emitting an alert when expanding a type alias sounds doable. However, I am not sure how reliable the alert might end up : it might only distinguish between zero non-abstract use of M.t and many uses.

Another strange possibility might to add an intermediary alias step:

module M: sig
  type t = Deprecated.t
  val v: string -> int -> t
end

where the compilation unit Deprecated define the alias:

type t =  string * int

then it is theoretically possible to export a version of the library where M.t is abstract by hiding the Deprecated cmi while still keeping the compatibility with the public version.

1 Like

I’m not sure I understood your solution. I tried to implement the indirection but as long as I don’t mention the Deprecated.t name nothing warns.

But I’m not fully sure I understood that step:

I can’t make M.t abstract now it will break code.

Sorry, I wasn’t clear enough about my “proposition”. Currently, there is no mechanism in the typechecker to make a type equation deprecated, so there is no way to trigger the kind of warning that you are seeking.

The idea behind of the Deprecated module was to take a side-step and have two variants of the same library, compat and next, with two different views on the same M.t. In compat with deprecated.cmi available, M.t is a type abbreviation. Contrarily, by removing deprecated.cmi from next, M.t will behave as an abstract type. However, both libraries will remain completely compatible with each other since they are using exactly the same interfaces and implementations under the hood.
That does sound quite bothersome, so that was probably more idle musing on my part.

1 Like