Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix indexing operators precedence #1039

Merged
merged 2 commits into from
Oct 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
### (master)

+ Fix indexing operators precedence (#1039) (Jules Aguillon)
+ Fix dropped comment after infix op (#1030) (Guillaume Petiot)
+ Fix: no newline if the input is empty (#1031) (Guillaume Petiot)
+ Fix unstable comments around attributes (#1029) (Guillaume Petiot)
Expand Down
17 changes: 12 additions & 5 deletions src/Ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,10 @@ let index_op_get_sugar ({txt= ident; loc} : Longident.t Location.loc) args =
, [{pexp_desc= Pexp_array l; _}] )
when List.length l > 3 ->
Some ({Location.txt= index_op_bigarray; loc}, l)
| _ -> None )
| lid, args -> (
match index_op_get_lid lid with
| None -> None
| Some index_op -> Some ({Location.txt= index_op; loc}, args) ) )

let index_op_set_sugar ({txt= ident; loc} : Longident.t Location.loc) args =
match all_args_unlabeled args with
Expand All @@ -165,6 +168,10 @@ let index_op_set_sugar ({txt= ident; loc} : Longident.t Location.loc) args =
, [{pexp_desc= Pexp_array l; _}; e] )
when List.length l > 3 ->
Some ({Location.txt= index_op_bigarray; loc}, l, e)
| lid, [argi; arge] -> (
match index_op_set_lid lid with
| None -> None
| Some index_op -> Some ({Location.txt= index_op; loc}, [argi], arge) )
| _ -> None )

let is_index_op exp =
Expand Down Expand Up @@ -1636,13 +1643,13 @@ end = struct
| _ -> Some (Apply, Non) ) )
| Pexp_apply ({pexp_desc= Pexp_ident ident; _}, (Nolabel, a1) :: args)
when Option.is_some (index_op_get_sugar ident args) ->
if a1 == exp then Some (Dot, Left) else Some (Comma, Left)
if a1 == exp then Some (Dot, Left) else Some (LessMinus, Left)
| Pexp_apply ({pexp_desc= Pexp_ident ident; _}, (Nolabel, a1) :: args)
when Option.is_some (index_op_set_sugar ident args) ->
let _, _, e = Option.value_exn (index_op_set_sugar ident args) in
if a1 == exp then Some (Dot, Left)
else if e == exp then Some (Comma, Right)
else Some (Comma, Left)
else if e == exp then Some (LessMinus, Right)
else Some (LessMinus, Left)
| Pexp_apply
({pexp_desc= Pexp_ident {txt= Lident i; _}; _}, [(_, e1); _]) -> (
let child = if e1 == exp then Left else Right in
Expand Down Expand Up @@ -1725,7 +1732,7 @@ end = struct
Some Dot
| Pexp_apply ({pexp_desc= Pexp_ident ident; _}, (Nolabel, _) :: args)
when Option.is_some (index_op_set_sugar ident args) ->
Some Comma
Some LessMinus
| Pexp_apply ({pexp_desc= Pexp_ident {txt= Lident i; _}; _}, [_; _])
-> (
match (i.[0], i) with
Expand Down
20 changes: 0 additions & 20 deletions src/Fmt_ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1670,26 +1670,6 @@ and fmt_expression c ?(box = true) ?pro ?epi ?eol ?parens ?(indent_wrap = 0)
, (fmt_op, args) )
| None -> (false, noop, noop, (noop, args))))
$ fmt_atrs )
| Pexp_apply
( { pexp_desc= Pexp_ident {txt= id; loc}
; pexp_loc
; pexp_attributes= _
; _ }
, (Nolabel, s) :: (Nolabel, i) :: _ )
when Option.is_some (index_op_get_lid id) ->
let index_op = Option.value_exn (index_op_get_lid id) in
Cmts.relocate c.cmts ~src:pexp_loc ~before:loc ~after:loc ;
fmt_index_op c ctx ~parens {txt= index_op; loc} s [i]
| Pexp_apply
( { pexp_desc= Pexp_ident {txt= id; loc}
; pexp_loc
; pexp_attributes= _
; _ }
, (Nolabel, s) :: (Nolabel, i) :: (Nolabel, e) :: _ )
when Option.is_some (index_op_set_lid id) ->
let index_op = Option.value_exn (index_op_set_lid id) in
Cmts.relocate c.cmts ~src:pexp_loc ~before:loc ~after:loc ;
fmt_index_op c ctx ~parens {txt= index_op; loc} s [i] ~set:e
| Pexp_apply (e0, [(Nolabel, e1)]) when is_prefix e0 ->
hvbox 2
(wrap_exp c ~loc:pexp_loc ~parens
Expand Down
34 changes: 33 additions & 1 deletion test/passing/index_op.ml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ let h = Hashtbl.create 17
;;
h.@("One") <- 1 ;
assert (h.@{"One"} = 1) ;
print_int (h.@{"One"}) ;
print_int h.@{"One"} ;
assert (h.?["Two"] = None)

(* from GPR#1392 *)
Expand Down Expand Up @@ -81,3 +81,35 @@ let () =
m.Mat.${[[2]; [5]]} |> ignore ;
let open Mat in
m.${[[2]; [5]]} |> ignore

let _ = (x.*{(y, z)} <- w) @ []

let _ = (x.{y, z} <- w) @ []

let _ = (x.*(y) <- z) @ []

let _ = (x.*(y) <- z) := []

let _ = ((x.*(y) <- z), [])

let _ = x.*(y) <- z ; []

let _ = (x.(y) <- z) @ []

let _ = (x.(y) <- z) := []

let _ = ((x.(y) <- z), [])

let _ = x.(y) <- z ; []

let _ = (x.y <- z) @ []

let _ = (x.y <- z) := []

let _ = ((x.y <- z), [])

let _ =
x.y <- z ;
[]

let _ = x.(y) <- (z.(w) <- u)