Support for log function genericity

[dbuenzli]

It is currently a bit difficult to define functions that are generic on the log function:

# let f log x y = 
    log (fun m -> m "start"); 
    let v = x + y in 
    log (fun m -> m "result: %d" v); 
    v;;
Error: This expression has type 'a but an expression was expected of type int -> 'a
       The type variable 'a occurs inside int -> 'a

It seems the simplest would be to define:

type 'a Logs.func = { log : 'a. 'a Logs.log }

Functions that want to be generic on log function then need to take a 'a Logs.func argument:

# type 'a func = { log : 'a. 'a Logs.log };;
type 'a func = { log : 'a0. 'a0 Logs.log; }
# let f log x y = 
    log.log (fun m -> m "start"); 
    let v = x + y in 
    log.log (fun m -> m "result: %d" v); 
    v;;
val f : 'a func -> int -> int -> int = <fun>
# f { log = Logs.warn } 4 2;;
ocaml: [WARNING] start
ocaml: [WARNING] result: 6
- : int = 6
# f { log = Logs.err } 4 2;;
ocaml: [ERROR] start
ocaml: [ERROR] result: 6
- : int = 6

Is there maybe a simpler solution that I fail to see ? (/cc @yallop @Drup)

[yallop]

For this particular example I think the simplest solution is to parameterise by level:

# let f level x y = 
    msg level (fun m -> m "start"); 
    let v = x + y in 
    msg level (fun m -> m "result: %d" v); 
    v;;
val f : Logs.level -> int -> int -> int = <fun>
# f Logs.Warning 4 2;;
ocaml: [WARNING] start
ocaml: [WARNING] result: 6
- : int = 6
# f Logs.Error 4 2;;
ocaml: [ERROR] start
ocaml: [ERROR] result: 6
- : int = 6

[dbuenzli]

Indeed thanks...

However I was looking at this in a slightly different context (and other implementation) where the argument would be optional and default to a kind of Logs.nil log function that doesn't report anything.

Of course this could be made to work with your solution if there was a Logs.msg function that would take an optional level which should maybe be added to Logs. I just need a good name (Logs.opt_msg ?).

Looking at this with a bit of hindsight though I think that the tags stuff in the msgf type was kind of a design error or over-engineering; unless someone actually uses that. Without this the type for logging funs would have been purely structural which would have allowed looser coupling for libraries.

[samoht]

I am not sure anyone is using tags at large scale yet (I saw @talex5 or @hannesm using this in a few places but that's pretty much it)

[hannesm]

Logs.Tag: yes, I use them (for example here). I like them a lot, especially combined with (some sort of) dynamic binding (see Lwt thread storage). Once we have support for structured syslog, they will be even more useful since tags map nicely into SD.

[dbuenzli]

So basically @yallop is right here (parametrize by level). For context: I really sometimes want to define functions with that signature:

val some_fun : ?log:Log.level -> ...

with a good default level but also the ability to not perform any logging at all (because the messages might not make sense in a given execution context of the function). There could be two designs here.

Design 1

Keep things as they are and add:

Log.maybe : level option ‑> 'a log

This means that my functions are defined as:

val some_fun : ?log:Log.level option -> ...

with the default for log being usually (Some level) and I simply use Log.maybe in some_fun I would also need to introduce Log.kmaybe. The advantages is that backward compatibility is fully kept, but we add more churn at the API level. And default optional arguments with options are a bit more fiddly.

Design 2

In this design the level cases are augmented with a Quiet case, that comes before App:

type level = Quiet | App | Error | Warning | Info | Debug

whose semantic is to simply drop the log message. This would allow to write your functions as:

val some_fun : ?log:Log.level -> ...

with the default for log being some level and the existing Log.msg function can simply be used in some_fun.

In this design the options in the level reporting functions would become a bit redundant but could be kept to minimize API breakage disruption. With this API breakage would should be mainly limited to libraries that write their own reporters because of the case addition (however since Quiet has the semantics of a reporting None level, the reporters would actually never be called with Quiet, handling it should be an assert false).

I would tend to favour 2., any comments ?