Style question regarding try statement

Which of the following ways to write the (procedural) function foo would be considered most idiomatic?

Variant 1:

let foo limit =
  let sum = ref 1 in
  let exception Limit_reached in
  (try
     for i = 1 to 10 do
       let newsum = !sum * i in
       if newsum > limit then raise Limit_reached;
       sum := newsum
     done
   with Limit_reached -> ());
  !sum - 1

(Side note: I find the 3-space indentation after the try statement a bit irritating, which was made by dune fmt / ocamlformat.)

Variant 2:

let foo limit =
  let sum = ref 1 in
  let exception Limit_reached in
  let () =
    try
      for i = 1 to 10 do
        let newsum = !sum * i in
        if newsum > limit then raise Limit_reached;
        sum := newsum
      done
    with Limit_reached -> ()
  in
  !sum - 1

I like variant 2 a bit more, but I’m unsure.

It is a question of style, so there is no definitive answer. At work we reserve let () = for top-level bindings. For local side-effects, we use instead begin ... end (equivalent to parentheses, but with better indentation and harder to miss).

Incidentally, for local exceptions such as your Limit_reached, you can use raise_notrace which is a bit more efficient than the ordinary raise as it does not keep track of the backtrace.

Cheers,
Nicolas

4 Likes

It’s useful to know about raise_notrace, but I would advise against using it unless you’re very concerned about performance and/or very confident that your code works as intended.
You can get the performance of raise_notrace with regular raises just by not passing the -g flag to the compiler, and if you do pass -g (and set backtraces on) then any notrace exception that is unhandled will be reported with a wrong backtrace (of the last regular raise that was caught).
This leads to very painful debugging experiences.

Example (you can run it in a toplevel):

let setup_backtrace () =
  Printexc.record_backtrace true;
  try raise Not_found with Not_found -> ()

let (let+) def body =
  fun () -> body def

let return c = fun () -> c

let get x = x ()

let oops () =
  let exception Local_exn in
  let r =
    try
      let+ x = 0 in
      if true then raise_notrace Local_exn
      else x
    with Local_exn -> return 0
  in
  get r

let main () =
  setup_backtrace ();
  oops ()

Oh interesting. I missed begin and end in the core language introduction of the manual, but it’s inside the language section of the manual under “Parenthesized expressions”.

Unfortunately, “dune fmt” changes all occurrences of begin … end to ( … ). I think it’s this issue. (edit: also see update below)

Hm, I like to use autoformatting, so not sure what to do. Maybe there is some option I can enable to not touch begin/end blocks?


Update:

Finding this PR, I figured out I can achieve that with

exp-grouping = preserve

inside the .ocamlformat file. Then I can write:

let foo limit =
  let sum = ref 1 in
  let exception Limit_reached in
  begin
    try
      for i = 1 to 10 do
        let newsum = !sum * i in
        if newsum > limit then raise Limit_reached;
        sum := newsum
      done
    with Limit_reached -> ()
  end;
  !sum - 1

That, I like!

2 Likes

Fun fact-- begin and end mean exactly the same thing as ( and ) in OCaml. Even to the extent that begin end is a valid expression which is exactly equivalent to () (i.e. the unit value).

2 Likes

Maybe parentheses aren’t so bad. But I dislike how they are formatted by default.

There is also a non-performance motivation to use raise_notrace some times. It does not modify the hidden current backtrace state. This matters for code that runs in an exception handler before reraising an exception such as:

try
  do_something ()
with
| Error as exc ->
    analyze exc ;
    raise exc

Here, if analyze as part of its operation raises (with raise) and handles an exception, then the backtrace associated with raise exc will not lead back to the original point where it was raised in do_something. If the implementation of analyze uses raise_notrace instead of raise, then the backtrace is left undisturbed. So if the OP foo function is meant to be used in contexts like analyze here, then I think it is preferable to use raise_notrace irrespective of performance.

2 Likes