Skip to content

Commit

Permalink
Fix _this_ on non-let-bindings
Browse files Browse the repository at this point in the history
  • Loading branch information
lukstafi committed Aug 3, 2024
1 parent 560e0d5 commit 246e07c
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 3 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,11 @@

- Runtime builders take a `description` optional argument.

### Fixed

- `_this_` infix on non-let-binding expressions throws a syntax error instead of being a no-op.
- As a design decision aligned with [#51](https://github.com/lukstafi/ppx_minidebug/issues/51), we output the error instead of processing the expression, i.e. we don't ignore the `_this_` infix.

## [1.5.1] -- 2024-07-07

### Changed
Expand Down
9 changes: 6 additions & 3 deletions ppx_minidebug.ml
Original file line number Diff line number Diff line change
Expand Up @@ -646,8 +646,8 @@ let unpack_runtime toplevel_opt_arg exp =
[%e exp]]

let has_runtime_arg = function
| { toplevel_opt_arg = Nested | Toplevel_no_arg; _ } -> false
| _ -> true
| { toplevel_opt_arg = PrintBox | Generic; _ } -> true
| _ -> false

let loc_to_name loc =
let fname = Filename.basename loc.loc_start.pos_fname |> Filename.remove_extension in
Expand Down Expand Up @@ -1343,7 +1343,10 @@ let debug_this_expander context payload =
(* This is the [let%debug_this ... in] use-case: do not debug the whole body. *)
let bindings = List.map (debug_binding context callback) bindings in
{ payload with pexp_desc = Pexp_let (recflag, bindings, body) }
| expr -> expr
| expr ->
A.pexp_extension ~loc:expr.pexp_loc
@@ Location.error_extensionf ~loc:expr.pexp_loc
"ppx_minidebug: to avoid confusion, _this_ indicator is only allowed on let-bindings"

let debug_expander context payload = traverse_expression#expression context payload

Expand Down
15 changes: 15 additions & 0 deletions test/dune
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@
(action
(run ./%{pp} --impl %{input} -o %{targets})))

(rule
(targets test_debug_show_error_this.actual.ml)
(deps
(:pp pp.exe)
(:input test_debug_show_error_this.ml))
(action
(run ./%{pp} --impl %{input} -o %{targets})))

(rule
(alias runtest)
(action
Expand Down Expand Up @@ -99,6 +107,13 @@
test_debug_log_prefixed.expected.ml
test_debug_log_prefixed.actual.ml)))

(rule
(alias runtest)
(action
(diff
test_debug_show_error_this.expected.ml
test_debug_show_error_this.actual.ml)))

(executable
(name test_debug_sexp)
(modules test_debug_sexp)
Expand Down
4 changes: 4 additions & 0 deletions test/test_debug_show_error_this.expected.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
let () =
[%ocaml.error
"ppx_minidebug: to avoid confusion, _this_ indicator is only allowed on let-bindings"];
()
9 changes: 9 additions & 0 deletions test/test_debug_show_error_this.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
(* module Debug_runtime =
(val Minidebug_runtime.debug_flushing ~filename:"debugger_show_flushing" ()) *)

let () =
[%debug_this_show
[%log_entry
"The end";
[%log (42 : int)]]];
()

0 comments on commit 246e07c

Please sign in to comment.