diff --git a/lib/Conf.ml b/lib/Conf.ml index f1ff894729..39fe735e95 100644 --- a/lib/Conf.ml +++ b/lib/Conf.ml @@ -260,8 +260,7 @@ let default = ; ocaml_version= elt Ocaml_version.Releases.v4_04_0 ; quiet= elt false ; disable_conf_attrs= elt false - ; version_check= elt true - ; rewrite_old_style_jane_street_local_annotations= elt false } } + ; version_check= elt true } } module V = struct let v0_12 = Version.make ~major:0 ~minor:12 ~patch:None @@ -1454,23 +1453,6 @@ module Operational = struct (fun conf elt -> update conf ~f:(fun f -> {f with version_check= elt})) (fun conf -> conf.opr_opts.version_check) - let rewrite_old_style_jane_street_local_annotations = - let doc = - "Rewrite all Jane Street annotations for use with the local mode, \ - such as \"[%local]\" or \"[@ocaml.global]\", into their \ - pretty-printed syntactic form, such as \"local_\" or \"global_\". \ - THIS OPTION WILL CHANGE THE RESULTING AST." - in - Decl.flag ~default - ~names:["rewrite-old-style-jane-street-local-annotations"] - ~doc ~kind - (fun conf elt -> - update conf ~f:(fun f -> - {f with rewrite_old_style_jane_street_local_annotations= elt} ) - ) - (fun conf -> - conf.opr_opts.rewrite_old_style_jane_street_local_annotations ) - let options : Store.t = Store. [ elt comment_check @@ -1481,8 +1463,7 @@ module Operational = struct ; elt ocaml_version ; elt quiet ; elt disable_conf_attrs - ; elt version_check - ; elt rewrite_old_style_jane_street_local_annotations ] + ; elt version_check ] end let options = Operational.options @ Formatting.options @ options @@ -1590,12 +1571,12 @@ let parse_state_attr attr = | Ok ("disable", _) -> Some `Disable | _ -> None -let is_jane_street_local_annotation config name ~test = - String.equal test ("extension." ^ name) - || - if config.opr_opts.rewrite_old_style_jane_street_local_annotations.v then +let is_jane_street_local_annotation _config name ~test = + let is_legacy = String.equal test name || String.equal test ("ocaml." ^ name) - else false + in + if is_legacy then Erase_jane_syntax.set_local_rewrite_occurred true ; + is_legacy || String.equal test ("extension." ^ name) let print_config = Decl.print_config options diff --git a/lib/Conf.mli b/lib/Conf.mli index 385b4acd5c..d19faaf7d6 100644 --- a/lib/Conf.mli +++ b/lib/Conf.mli @@ -26,6 +26,10 @@ val update_state : t -> [`Enable | `Disable] -> t val parse_state_attr : Parsetree.attribute -> [`Enable | `Disable] option val is_jane_street_local_annotation : t -> string -> test:string -> bool +(** [is_jane_street_local_annotation c n ~test] checks if [test] refers to + a piece of jane street local annotation with the name [c]. This function + contains side effects: when a legacy piece of local annotation is encountered, + it sets [Erase_jane_syntax.local_rewrite_occurred ()] to [true]. *) val parse_line : t diff --git a/lib/Fmt_ast.ml b/lib/Fmt_ast.ml index 52fbc2d340..3a963e3b8c 100644 --- a/lib/Fmt_ast.ml +++ b/lib/Fmt_ast.ml @@ -751,6 +751,7 @@ and type_constr_and_body c xbody = and fmt_arrow_param c ctx ({pap_label= lI; pap_loc= locI; pap_type= tI}, localI) = + let localI = localI && not (Erase_jane_syntax.should_erase ()) in let arg_label lbl = match lbl with | Nolabel -> if localI then Some (str "local_ ") else None @@ -1478,7 +1479,7 @@ and fmt_body c ?ext ({ast= body; _} as xbody) = ~test:extension_local (* Don't wipe away comments before [local_]. *) && not (Cmts.has_before c.cmts pexp_loc) -> - ( fmt " local_" + ( fmt_if (not (Erase_jane_syntax.should_erase ())) " local_" , fmt_expression c ~eol:(fmt "@;<1000 0>") (sub_exp c.conf ~ctx sbody) ) | { pexp_desc= @@ -1492,7 +1493,7 @@ and fmt_body c ?ext ({ast= body; _} as xbody) = ~test:extension_exclave (* Don't wipe away comments before [exclave_]. *) && not (Cmts.has_before c.cmts pexp_loc) -> - ( fmt " exclave_" + ( fmt_if (not (Erase_jane_syntax.should_erase ())) " exclave_" , fmt_expression c ~eol:(fmt "@;<1000 0>") (sub_exp c.conf ~ctx sbody) ) | _ -> (noop, fmt_expression c ~eol:(fmt "@;<1000 0>") xbody) @@ -2121,7 +2122,8 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens ~test:extension_local -> pro $ Params.parens_if parens c.conf - (fmt "local_@ " $ fmt_expression c (sub_exp c.conf ~ctx sbody)) + ( fmt_if (not (Erase_jane_syntax.should_erase ())) "local_@ " + $ fmt_expression c (sub_exp c.conf ~ctx sbody) ) | Pexp_apply ( {pexp_desc= Pexp_extension ({txt= extension_exclave; _}, PStr []); _} , [(Nolabel, sbody)] ) @@ -2129,7 +2131,8 @@ and fmt_expression c ?(box = true) ?(pro = noop) ?eol ?parens ~test:extension_exclave -> pro $ Params.parens_if parens c.conf - (fmt "exclave_@ " $ fmt_expression c (sub_exp c.conf ~ctx sbody)) + ( fmt_if (not (Erase_jane_syntax.should_erase ())) "exclave_@ " + $ fmt_expression c (sub_exp c.conf ~ctx sbody) ) | Pexp_prefix (op, e) -> let has_cmts = Cmts.has_before c.cmts e.pexp_loc in pro @@ -3666,6 +3669,7 @@ and fmt_label_declaration c ctx ?(last = false) decl = (str ";") in let is_global, atrs = split_global_flags_from_attrs c.conf atrs in + let is_global = is_global && not (Erase_jane_syntax.should_erase ()) in hovbox 0 ( Cmts.fmt_before c pld_loc $ hvbox @@ -3724,6 +3728,7 @@ and fmt_constructor_declaration c ctx ~first ~last:_ cstr_decl = and fmt_core_type_gf c ctx typ = let {ptyp_attributes; _} = typ in let is_global, _ = split_global_flags_from_attrs c.conf ptyp_attributes in + let is_global = is_global && not (Erase_jane_syntax.should_erase ()) in fmt_if is_global "global_ " $ fmt_core_type c (sub_typ ~ctx typ) and fmt_constructor_arguments ?vars c ctx ~pre = function @@ -4802,7 +4807,12 @@ and fmt_value_binding c ~rec_flag ?ext ?in_ ?epi $ fmt_extension_suffix c ext $ fmt_attributes c at_attrs $ fmt_if rec_flag " rec" - $ fmt_if lb_local " local_" + $ fmt_if + ( lb_local + && not + (Erase_jane_syntax.should_erase ()) + ) + " local_" $ fmt_or pat_has_cmt "@ " " " $ fmt_pattern c lb_pat ) $ fmt_if_k diff --git a/lib/Normalize_std_ast.ml b/lib/Normalize_std_ast.ml index 11c3d27314..6cb3180bfa 100644 --- a/lib/Normalize_std_ast.ml +++ b/lib/Normalize_std_ast.ml @@ -12,6 +12,17 @@ open Parser_standard open Std_ast +let erase_legacy_jane_street_local_annotations c : attributes -> attributes = + List.filter ~f:(fun attr -> + match attr with + | {attr_name= {txt= old_name; _}; attr_payload= PStr []; _} -> + not + ( Conf.is_jane_street_local_annotation c "local" ~test:old_name + || Conf.is_jane_street_local_annotation c "global" ~test:old_name + || Conf.is_jane_street_local_annotation c "exclave" + ~test:old_name ) + | _ -> true ) + let is_doc = function | {attr_name= {Location.txt= "ocaml.doc" | "ocaml.text"; _}; _} -> true | _ -> false @@ -19,6 +30,9 @@ let is_doc = function let is_erasable_jane_syntax attr = String.is_prefix ~prefix:"jane.erasable." attr.attr_name.txt +let is_local_jane_syntax attr = + String.is_prefix ~prefix:"jane.erasable.local" attr.attr_name.txt + let dedup_cmts fragment ast comments = let of_ast ast = let docs = ref (Set.empty (module Cmt)) in @@ -69,7 +83,8 @@ let docstring (c : Conf.t) = let sort_attributes : attributes -> attributes = List.sort ~compare:Poly.compare -let make_mapper conf ~ignore_doc_comments ~erase_jane_syntax = +let make_mapper conf ~ignore_doc_comments ~erase_jane_syntax + ~local_rewrite_occurred = let open Ast_helper in (* remove locations *) let location _ _ = Location.none in @@ -108,6 +123,11 @@ let make_mapper conf ~ignore_doc_comments ~erase_jane_syntax = List.filter atrs ~f:(fun a -> not (is_erasable_jane_syntax a)) else atrs in + let atrs = + if local_rewrite_occurred then + List.filter atrs ~f:(fun a -> not (is_local_jane_syntax a)) + else atrs + in let atrs = if ignore_doc_comments then List.filter atrs ~f:(fun a -> not (is_doc a)) @@ -132,10 +152,27 @@ let make_mapper conf ~ignore_doc_comments ~erase_jane_syntax = (Exp.sequence ~loc:loc1 ~attrs:attrs1 (Exp.sequence ~loc:loc2 ~attrs:attrs2 exp1 exp2) exp3 ) + | Pexp_apply + ( {pexp_desc= Pexp_extension ({txt= extension_name; _}, PStr []); _} + , [(Nolabel, sbody)] ) + when (local_rewrite_occurred || erase_jane_syntax) + && ( Conf.is_jane_street_local_annotation conf "local" + ~test:extension_name + || Conf.is_jane_street_local_annotation conf "exclave" + ~test:extension_name ) -> + m.expr m sbody | _ -> Ast_mapper.default_mapper.expr m exp in let pat (m : Ast_mapper.mapper) pat = let pat = {pat with ppat_loc_stack= []} in + let pat = + if local_rewrite_occurred || erase_jane_syntax then + { pat with + ppat_attributes= + erase_legacy_jane_street_local_annotations conf + pat.ppat_attributes } + else pat + in let {ppat_desc; ppat_loc= loc1; ppat_attributes= attrs1; _} = pat in (* normalize nested or patterns *) match ppat_desc with @@ -158,6 +195,14 @@ let make_mapper conf ~ignore_doc_comments ~erase_jane_syntax = in let typ (m : Ast_mapper.mapper) typ = let typ = {typ with ptyp_loc_stack= []} in + let typ = + if local_rewrite_occurred || erase_jane_syntax then + { typ with + ptyp_attributes= + erase_legacy_jane_street_local_annotations conf + typ.ptyp_attributes } + else typ + in Ast_mapper.default_mapper.typ m typ in let structure = @@ -196,6 +241,17 @@ let make_mapper conf ~ignore_doc_comments ~erase_jane_syntax = Ast_mapper.default_mapper.class_signature m {x with pcsig_fields} else Ast_mapper.default_mapper.class_signature in + let label_declaration (m : Ast_mapper.mapper) ld = + let ld = + if local_rewrite_occurred || erase_jane_syntax then + { ld with + pld_attributes= + erase_legacy_jane_street_local_annotations conf ld.pld_attributes + } + else ld + in + Ast_mapper.default_mapper.label_declaration m ld + in { Ast_mapper.default_mapper with location ; attribute @@ -206,17 +262,21 @@ let make_mapper conf ~ignore_doc_comments ~erase_jane_syntax = ; class_structure ; expr ; pat - ; typ } + ; typ + ; label_declaration } -let ast fragment ~ignore_doc_comments ~erase_jane_syntax c = - map fragment (make_mapper c ~ignore_doc_comments ~erase_jane_syntax) +let ast fragment ~ignore_doc_comments ~erase_jane_syntax + ~local_rewrite_occurred c = + map fragment + (make_mapper c ~ignore_doc_comments ~erase_jane_syntax + ~local_rewrite_occurred ) -let equal fragment ~ignore_doc_comments ~erase_jane_syntax c ~old:ast1 - ~new_:ast2 = +let equal fragment ~ignore_doc_comments ~erase_jane_syntax + ~local_rewrite_occurred c ~old:ast1 ~new_:ast2 = let map = ast fragment c ~ignore_doc_comments in equal fragment - (map ~erase_jane_syntax ast1) - (map ~erase_jane_syntax:false ast2) + (map ~erase_jane_syntax ~local_rewrite_occurred ast1) + (map ~erase_jane_syntax:false ~local_rewrite_occurred ast2) let ast = ast ~ignore_doc_comments:false @@ -247,20 +307,22 @@ let docstrings (type a) (fragment : a t) s = let (_ : a) = map fragment (make_docstring_mapper docstrings) s in !docstrings -let docstring conf ~erase_jane_syntax = +let docstring conf ~erase_jane_syntax ~local_rewrite_occurred = let mapper = make_mapper conf ~ignore_doc_comments:false ~erase_jane_syntax + ~local_rewrite_occurred in let normalize_code = normalize_code conf mapper in docstring conf ~normalize_code -let moved_docstrings fragment ~erase_jane_syntax c ~old:s1 ~new_:s2 = +let moved_docstrings fragment ~erase_jane_syntax ~local_rewrite_occurred c + ~old:s1 ~new_:s2 = let d1 = docstrings fragment s1 in let d2 = docstrings fragment s2 in let equal ~old:(_, x) ~new_:(_, y) = String.equal - (docstring c x ~erase_jane_syntax) - (docstring c y ~erase_jane_syntax:false) + (docstring c x ~erase_jane_syntax ~local_rewrite_occurred) + (docstring c y ~erase_jane_syntax ~local_rewrite_occurred:false) in let cmt_kind = `Doc_comment in let cmt (loc, x) = Cmt.create_docstring x loc in diff --git a/lib/Normalize_std_ast.mli b/lib/Normalize_std_ast.mli index 5245daf2d6..aca9a075e4 100644 --- a/lib/Normalize_std_ast.mli +++ b/lib/Normalize_std_ast.mli @@ -9,7 +9,13 @@ (* *) (**************************************************************************) -val ast : 'a Std_ast.t -> erase_jane_syntax:bool -> Conf.t -> 'a -> 'a +val ast : + 'a Std_ast.t + -> erase_jane_syntax:bool + -> local_rewrite_occurred:bool + -> Conf.t + -> 'a + -> 'a (** Normalize an AST fragment. If [erase_jane_syntax] is true, remove all [Jane_syntax] attributes signaling erasable syntax. *) @@ -17,6 +23,7 @@ val equal : 'a Std_ast.t -> ignore_doc_comments:bool -> erase_jane_syntax:bool + -> local_rewrite_occurred:bool -> Conf.t -> old:'a -> new_:'a @@ -29,6 +36,7 @@ val equal : val moved_docstrings : 'a Std_ast.t -> erase_jane_syntax:bool + -> local_rewrite_occurred:bool -> Conf.t -> old:'a -> new_:'a diff --git a/lib/Translation_unit.ml b/lib/Translation_unit.ml index ca147b1299..bd338c2262 100644 --- a/lib/Translation_unit.ml +++ b/lib/Translation_unit.ml @@ -324,10 +324,18 @@ let format (type a b) (fg : a Extended_ast.t) (std_fg : b Std_ast.t) in (* Ast not preserved ? *) let erase_jane_syntax = Erase_jane_syntax.should_erase () in + let local_rewrite_occurred = + Erase_jane_syntax.local_rewrite_occurred () + in + if i = 1 && local_rewrite_occurred then + Format.eprintf + "Warning: Legacy local annotation rewrite occurred. AST \ + normalization will ignore local syntax differences.\n" ; ( if (not (Normalize_std_ast.equal std_fg conf ~old:std_t.ast ~new_:std_t_new.ast ~erase_jane_syntax + ~local_rewrite_occurred ~ignore_doc_comments:(not conf.opr_opts.comment_check.v) ) ) && not (Normalize_extended_ast.equal fg conf t.ast t_new.ast @@ -335,12 +343,13 @@ let format (type a b) (fg : a Extended_ast.t) (std_fg : b Std_ast.t) then let old_ast = dump_ast std_fg ~suffix:".old" - (Normalize_std_ast.ast std_fg ~erase_jane_syntax conf std_t.ast) + (Normalize_std_ast.ast std_fg ~erase_jane_syntax + ~local_rewrite_occurred conf std_t.ast ) in let new_ast = dump_ast std_fg ~suffix:".new" - (Normalize_std_ast.ast std_fg ~erase_jane_syntax:false conf - std_t_new.ast ) + (Normalize_std_ast.ast std_fg ~erase_jane_syntax:false + ~local_rewrite_occurred conf std_t_new.ast ) in let args ~suffix = [ ("output file", dump_formatted ~suffix fmted) @@ -351,11 +360,13 @@ let format (type a b) (fg : a Extended_ast.t) (std_fg : b Std_ast.t) in if Normalize_std_ast.equal std_fg ~ignore_doc_comments:true - ~erase_jane_syntax conf ~old:std_t.ast ~new_:std_t_new.ast + ~erase_jane_syntax ~local_rewrite_occurred conf ~old:std_t.ast + ~new_:std_t_new.ast then let docstrings = Normalize_std_ast.moved_docstrings std_fg ~erase_jane_syntax - conf ~old:std_t.ast ~new_:std_t_new.ast + ~local_rewrite_occurred conf ~old:std_t.ast + ~new_:std_t_new.ast in let args = args ~suffix:".unequal-docs" in internal_error @@ -366,8 +377,8 @@ let format (type a b) (fg : a Extended_ast.t) (std_fg : b Std_ast.t) internal_error [`Ast_changed] args else dump_ast std_fg ~suffix:"" - (Normalize_std_ast.ast std_fg ~erase_jane_syntax conf - std_t_new.ast ) + (Normalize_std_ast.ast std_fg ~erase_jane_syntax + ~local_rewrite_occurred conf std_t_new.ast ) |> function | Some file -> if i = 1 then Format.eprintf "[DEBUG] AST structure: %s\n" file diff --git a/lib/erase_jane_syntax/erase_jane_syntax.ml b/lib/erase_jane_syntax/erase_jane_syntax.ml index deb562ec79..ca538fb46f 100644 --- a/lib/erase_jane_syntax/erase_jane_syntax.ml +++ b/lib/erase_jane_syntax/erase_jane_syntax.ml @@ -3,3 +3,9 @@ let should_erase_ref = ref false let set_should_erase yn = should_erase_ref := yn let should_erase () = !should_erase_ref + +let local_rewrite_occurred_ref = ref false + +let set_local_rewrite_occurred yn = local_rewrite_occurred_ref := yn + +let local_rewrite_occurred () = !local_rewrite_occurred_ref diff --git a/lib/erase_jane_syntax/erase_jane_syntax.mli b/lib/erase_jane_syntax/erase_jane_syntax.mli index ae2f172ae0..ed3d7197f7 100644 --- a/lib/erase_jane_syntax/erase_jane_syntax.mli +++ b/lib/erase_jane_syntax/erase_jane_syntax.mli @@ -27,3 +27,11 @@ val should_erase : unit -> bool erased from parsing/printing: [true] if they should be erased (so that the parse tree will be modified by ocamlformat), [false] if they should not. *) + +val set_local_rewrite_occurred : bool -> unit +(** Set whether legacy local annotation rewrite has occurred. We need this information + when checking the ASTs for equality. If such a rewrite did take place, we will have to + ignore local annotation changes in the standard parsetree. *) + +val local_rewrite_occurred : unit -> bool +(** Returns whether or not a legacy local annotation rewrite has occurred. *) diff --git a/test/passing/tests/js_source.ml.err b/test/passing/tests/js_source.ml.err index 95f7fb8536..9f79d2dce0 100644 --- a/test/passing/tests/js_source.ml.err +++ b/test/passing/tests/js_source.ml.err @@ -1,3 +1,4 @@ +Warning: Legacy local annotation rewrite occurred. AST normalization will ignore local syntax differences. Warning: tests/js_source.ml:155 exceeds the margin Warning: tests/js_source.ml:9531 exceeds the margin Warning: tests/js_source.ml:9634 exceeds the margin diff --git a/test/passing/tests/js_source.ml.ocp b/test/passing/tests/js_source.ml.ocp index 7863961cd5..487aa638e7 100644 --- a/test/passing/tests/js_source.ml.ocp +++ b/test/passing/tests/js_source.ml.ocp @@ -10215,8 +10215,8 @@ let _ = module type For_let_syntax_local = For_let_syntax_gen - with type ('a, 'b) fn := ('a[@local]) -> 'b - and type ('a, 'b) f_labeled_fn := f:('a[@local]) -> 'b + with type ('a, 'b) fn := local_ 'a -> 'b + and type ('a, 'b) f_labeled_fn := f:local_ 'a -> 'b type fooooooooooooooooooooooooooooooo = ( fooooooooooooooooooooooooooooooo diff --git a/test/passing/tests/js_source.ml.ref b/test/passing/tests/js_source.ml.ref index dd4cb8b407..2cc840ced6 100644 --- a/test/passing/tests/js_source.ml.ref +++ b/test/passing/tests/js_source.ml.ref @@ -10215,8 +10215,8 @@ let _ = module type For_let_syntax_local = For_let_syntax_gen - with type ('a, 'b) fn := ('a[@local]) -> 'b - and type ('a, 'b) f_labeled_fn := f:('a[@local]) -> 'b + with type ('a, 'b) fn := local_ 'a -> 'b + and type ('a, 'b) f_labeled_fn := f:local_ 'a -> 'b type fooooooooooooooooooooooooooooooo = ( fooooooooooooooooooooooooooooooo diff --git a/test/passing/tests/local-erased.ml.err b/test/passing/tests/local-erased.ml.err new file mode 100644 index 0000000000..f5447cb377 --- /dev/null +++ b/test/passing/tests/local-erased.ml.err @@ -0,0 +1 @@ +Warning: Legacy local annotation rewrite occurred. AST normalization will ignore local syntax differences. diff --git a/test/passing/tests/local-erased.ml.ref b/test/passing/tests/local-erased.ml.ref index fc552815b5..cb3919323a 100644 --- a/test/passing/tests/local-erased.ml.ref +++ b/test/passing/tests/local-erased.ml.ref @@ -66,29 +66,23 @@ let foo () = if true then () ; () -type loc_long_attrs = (string[@ocaml.local]) -> (string[@ocaml.local]) +type loc_long_attrs = string -> string -type loc_short_attrs = (string[@local]) -> (string[@local]) +type loc_short_attrs = string -> string -type global_long_attrs = - | Foo of {s: string [@ocaml.global]} - | Bar of (string[@ocaml.global]) +type global_long_attrs = Foo of {s: string} | Bar of string -type global_short_attrs = - | Foo of {s: string [@global]} - | Bar of (string[@global]) +type global_short_attrs = Foo of {s: string} | Bar of string -type global_short_attrs = - | Foo of {s: string [@global]} - | Bar of (string[@global]) +type global_short_attrs = Foo of {s: string} | Bar of string -let local_long_ext = [%ocaml.local] () +let local_long_ext = () -let local_short_ext = [%local] () +let local_short_ext = () -let exclave_long_ext = [%ocaml.exclave] () +let exclave_long_ext = () -let exclave_short_ext = [%exclave] () +let exclave_short_ext = () let[@ocaml.local] upstream_local_attr_long x = x diff --git a/test/passing/tests/local.ml.err b/test/passing/tests/local.ml.err new file mode 100644 index 0000000000..f5447cb377 --- /dev/null +++ b/test/passing/tests/local.ml.err @@ -0,0 +1 @@ +Warning: Legacy local annotation rewrite occurred. AST normalization will ignore local syntax differences.