normalize mode order in the extended parser

the current state of the PR leaves the extended parse tree unnormalized, but this is hidden by the wonderful bug in the round-trip check this commit sorts the modes while parsing the extended ast as we are wont; now we don't need to handle them specially from `Fmt_ast.ml` or do a separate normalization Signed-off-by: David Vulakh <[email protected]>
janestreet · Nov 25, 2024 · 422d57c · 422d57c
1 parent 687b458
commit 422d57c
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 22 deletions.
diff --git a/lib/Fmt_ast.ml b/lib/Fmt_ast.ml
@@ -772,13 +772,6 @@ and type_constr_and_body c xbody =
   | _ -> (None, xbody)
 
 and fmt_modalities ?(break = true) c modalities =
-  let modalities =
-    List.sort
-      ~compare:(fun
-          {Location.txt= Modality m1; _} {Location.txt= Modality m2; _} ->
-        String.compare m1 m2 )
-      modalities
-  in
   let fmt_modality {txt= Modality modality; loc} =
     Cmts.fmt c loc (str modality)
   in
@@ -789,12 +782,6 @@ and fmt_modalities ?(break = true) c modalities =
     $ hvbox 0 (list modalities "@ " fmt_modality)
 
 and fmt_modes ~ats c modes =
-  let modes =
-    List.sort
-      ~compare:(fun {txt= Mode m1; _} {txt= Mode m2; _} ->
-        String.compare m1 m2 )
-      modes
-  in
   let fmt_mode {txt= Mode mode; loc} = Cmts.fmt c loc (str mode) in
   if List.is_empty modes then noop
   else

diff --git a/vendor/parser-extended/parser.mly b/vendor/parser-extended/parser.mly
@@ -3659,13 +3659,8 @@ type_parameters:
 ;
 
 jkind:
-    mkrhs(jkind) MOD mkrhs(LIDENT)+
-      { (* LIDENTs here are for modes *) let modes =
-          List.map
-            (fun {txt; loc} -> {txt = Mode txt; loc})
-            $3
-        in
-        Mod ($1, modes) }
+    mkrhs(jkind) MOD mode_expr
+      { Mod ($1, $3) }
   | mkrhs(jkind) WITH core_type
       { With ($1, $3) }
   | mkrhs(ident)
@@ -4254,7 +4249,16 @@ strict_function_or_labeled_tuple_type:
 ;
 
 %inline mode_expr:
-  | mode+ { if Erase_jane_syntax.should_erase () then [] else $1 }
+  | mode+ {
+    if Erase_jane_syntax.should_erase ()
+    then []
+    else
+      List.sort
+        (fun
+          { Location.txt = Mode m1; _ } { Location.txt = Mode m2; _ } ->
+        String.compare m1 m2)
+        $1
+    }
 ;
 
 at_mode_expr:
@@ -4289,7 +4293,16 @@ atat_mode_expr:
   | LIDENT { mkloc (Modality $1) (make_loc $sloc) }
 
 %inline modalities:
-  | modality+ { if Erase_jane_syntax.should_erase () then [] else $1 }
+  | modality+ {
+    if Erase_jane_syntax.should_erase ()
+    then []
+    else
+      List.sort
+        (fun
+          { Location.txt = Modality m1; _ } { Location.txt = Modality m2; _ } ->
+        String.compare m1 m2)
+        $1
+    }
 
 optional_atat_modalities_expr:
   | %prec below_HASH