Skip to content

Commit 04b733b

Browse files
authored
Merge pull request #242 from Lupus/more-compliant-options-parsing
More compliant options parsing
2 parents b7a1bd7 + 5f3beb0 commit 04b733b

26 files changed

+757
-210
lines changed

src/compilerlib/dune

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
pb_codegen_encode_yojson pb_codegen_formatting pb_codegen_ocaml_type_dump
1515
pb_codegen_ocaml_type pb_codegen_pp pb_codegen_plugin pb_codegen_types
1616
pb_codegen_services pb_codegen_util pb_exception pb_field_type pb_location
17-
pb_logger pb_option pb_parsing pb_parsing_lexer pb_parsing_parser
18-
pb_parsing_parse_tree pb_parsing_util pb_typing_graph pb_typing
19-
pb_typing_recursion pb_typing_resolution pb_typing_type_tree
17+
pb_logger pb_option pb_raw_option pb_parsing pb_parsing_lexer
18+
pb_parsing_parser pb_parsing_parse_tree pb_parsing_util pb_typing_graph
19+
pb_typing pb_typing_recursion pb_typing_resolution pb_typing_type_tree
2020
pb_typing_util pb_typing_validation pb_util pb_format_util)
2121
(libraries stdlib-shims))

src/compilerlib/pb_codegen_all.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ let new_ocaml_mod ~proto_file_options ~proto_file_name () : ocaml_mod =
4040
let self = { ml = F.empty_scope (); mli = F.empty_scope () } in
4141

4242
let print_ppx sc =
43-
match Pb_option.get proto_file_options "ocaml_file_ppx" with
43+
match Pb_raw_option.get_ext proto_file_options "ocaml_file_ppx" with
4444
| None -> ()
4545
| Some Pb_option.(Scalar_value (Constant_string s)) ->
4646
F.linep sc "[@@@%s]" s

src/compilerlib/pb_codegen_all.mli

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ type ocaml_mod = {
1212
val codegen :
1313
Ot.proto ->
1414
generate_make:bool ->
15-
proto_file_options:Pb_option.set ->
15+
proto_file_options:Pb_raw_option.set ->
1616
proto_file_name:string ->
1717
services:bool ->
1818
Plugin.t list ->

src/compilerlib/pb_codegen_backend.ml

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ let encoding_info_of_field_type ~all_types field_type : Ot.payload_kind =
195195
let encoding_of_field ~all_types (field : (Pb_field_type.resolved, 'a) Tt.field)
196196
=
197197
let packed =
198-
match Typing_util.field_option field "packed" with
198+
match Typing_util.field_option field (Pb_option.Simple_name "packed") with
199199
| Some Pb_option.(Scalar_value (Constant_bool x)) -> x
200200
| Some _ -> E.invalid_packed_option (Typing_util.field_name field)
201201
| None -> false
@@ -209,34 +209,34 @@ let encoding_of_field ~all_types (field : (Pb_field_type.resolved, 'a) Tt.field)
209209
let compile_field_type ~unsigned_tag ~(all_types : _ Tt.proto_type list)
210210
file_options field_options file_name field_type : Ot.field_type =
211211
let ocaml_type =
212-
match Pb_option.get field_options "ocaml_type" with
212+
match Pb_option.get_ext field_options "ocaml_type" with
213213
| Some Pb_option.(Scalar_value (Constant_literal "int_t")) -> `Int_t
214214
| _ -> `None
215215
in
216216

217217
let int32_type =
218-
match Pb_option.get file_options "int32_type" with
218+
match Pb_option.get_ext file_options "int32_type" with
219219
| Some Pb_option.(Scalar_value (Pb_option.Constant_literal "int_t")) ->
220220
Ot.(Ft_basic_type Bt_int)
221221
| _ -> Ot.(Ft_basic_type Bt_int32)
222222
in
223223

224224
let uint32_type =
225-
match Pb_option.get file_options "int32_type" with
225+
match Pb_option.get_ext file_options "int32_type" with
226226
| Some Pb_option.(Scalar_value (Constant_literal "int_t")) ->
227227
Ot.(Ft_basic_type Bt_int)
228228
| _ -> Ot.(Ft_basic_type Bt_uint32)
229229
in
230230

231231
let int64_type =
232-
match Pb_option.get file_options "int64_type" with
232+
match Pb_option.get_ext file_options "int64_type" with
233233
| Some Pb_option.(Scalar_value (Constant_literal "int_t")) ->
234234
Ot.(Ft_basic_type Bt_int)
235235
| _ -> Ot.(Ft_basic_type Bt_int64)
236236
in
237237

238238
let uint64_type =
239-
match Pb_option.get file_options "int64_type" with
239+
match Pb_option.get_ext file_options "int64_type" with
240240
| Some Pb_option.(Scalar_value (Constant_literal "int_t")) ->
241241
Ot.(Ft_basic_type Bt_int)
242242
| _ -> Ot.(Ft_basic_type Bt_uint64)
@@ -289,13 +289,13 @@ let compile_field_type ~unsigned_tag ~(all_types : _ Tt.proto_type list)
289289
| `User_defined id, _ -> user_defined_type_of_id ~all_types ~file_name id
290290

291291
let is_mutable ?field_name field_options =
292-
match Pb_option.get field_options "ocaml_mutable" with
292+
match Pb_option.get_ext field_options "ocaml_mutable" with
293293
| Some Pb_option.(Scalar_value (Constant_bool v)) -> v
294294
| Some _ -> Pb_exception.invalid_mutable_option ?field_name ()
295295
| None -> false
296296

297297
let ocaml_container field_options =
298-
match Pb_option.get field_options "ocaml_container" with
298+
match Pb_option.get_ext field_options "ocaml_container" with
299299
| None -> None
300300
| Some Pb_option.(Scalar_value (Constant_literal container_name)) ->
301301
Some container_name
@@ -371,7 +371,7 @@ let process_all_types_ppx_extension file_name file_options
371371
match type_level_ppx_extension with
372372
| Some x -> Some x
373373
| None ->
374-
Pb_option.get file_options "ocaml_all_types_ppx"
374+
Pb_option.get_ext file_options "ocaml_all_types_ppx"
375375
|> string_of_string_option file_name
376376

377377
let compile_message ~(unsigned_tag : bool) (file_options : Pb_option.set)
@@ -388,7 +388,8 @@ let compile_message ~(unsigned_tag : bool) (file_options : Pb_option.set)
388388
let { Tt.message_names; _ } = scope in
389389

390390
let type_level_ppx_extension =
391-
Typing_util.message_option message "ocaml_type_ppx"
391+
Typing_util.message_option message
392+
(Pb_option.Extension_name "ocaml_type_ppx")
392393
|> string_of_string_option message_name
393394
|> process_all_types_ppx_extension file_name file_options
394395
in
@@ -633,7 +634,7 @@ let compile_enum file_options file_name scope enum =
633634
in
634635

635636
let type_level_ppx_extension =
636-
Typing_util.enum_option enum "ocaml_enum_ppx"
637+
Typing_util.enum_option enum (Pb_option.Extension_name "ocaml_enum_ppx")
637638
|> string_of_string_option enum_name
638639
|> process_all_types_ppx_extension file_name file_options
639640
in

src/compilerlib/pb_codegen_ocaml_type_dump.ml

Lines changed: 8 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -74,29 +74,6 @@ module PP = struct
7474
| Constant_literal s ->
7575
Printf.sprintf "Constant_literal %S" (String.escaped s)
7676

77-
(* Helper function to convert value to string *)
78-
let rec string_of_value value =
79-
match value with
80-
| Pb_option.Scalar_value c -> string_of_constant c
81-
| Message_literal ml -> string_of_message_literal ml
82-
| List_literal ll -> string_of_list_literal ll
83-
84-
(* Helper function to convert message_literal to string *)
85-
and string_of_message_literal ml =
86-
"{"
87-
^ String.concat ", "
88-
(List.map
89-
(fun (k, v) -> Printf.sprintf "%S: %s" k (string_of_value v))
90-
ml)
91-
^ "}"
92-
93-
(* Helper function to convert list_literal to string *)
94-
and string_of_list_literal ll =
95-
"[" ^ String.concat ", " (List.map string_of_value ll) ^ "]"
96-
97-
(* Function to convert options (message_literal) to string *)
98-
let string_of_options options = string_of_message_literal options
99-
10077
(* Helper function to convert default_value to string *)
10178
let string_of_default_value dv =
10279
match dv with
@@ -171,7 +148,8 @@ module PP = struct
171148
(string_of_variant_constructor_type vc.vc_field_type);
172149
F.linep sc " Encoding Number: %d, Payload Kind: %s" vc.vc_encoding_number
173150
(string_of_payload_kind vc.vc_payload_kind);
174-
F.linep sc " Options: %s" (string_of_options vc.vc_options)
151+
F.linep sc " Options: %s"
152+
(Format.asprintf "%a" Pb_option.pp_set vc.vc_options)
175153

176154
(* Helper function to convert variant_constructor_type to string *)
177155
and string_of_variant_constructor_type vct =
@@ -189,7 +167,8 @@ module PP = struct
189167
and print_record_field sc record_field =
190168
F.linep sc "- Field: %s" record_field.rf_label;
191169
print_record_field_type sc record_field.rf_field_type;
192-
F.linep sc " Field options: %s" (string_of_options record_field.rf_options)
170+
F.linep sc " Field options: %s"
171+
(Format.asprintf "%a" Pb_option.pp_set record_field.rf_options)
193172

194173
(* Recursive function to print a const_variant *)
195174
let rec print_const_variant sc const_variant =
@@ -201,7 +180,8 @@ module PP = struct
201180
F.linep sc " Constructor: %s" cvc.cvc_name;
202181
F.linep sc " Binary Value: %d, String Value: %s" cvc.cvc_binary_value
203182
cvc.cvc_string_value;
204-
F.linep sc " Options: %s" (string_of_options cvc.cvc_options)
183+
F.linep sc " Options: %s"
184+
(Format.asprintf "%a" Pb_option.pp_set cvc.cvc_options)
205185

206186
(* Recursive function to print the type_spec *)
207187
let print_type_spec sc type_spec =
@@ -215,7 +195,8 @@ module PP = struct
215195
let print_type sc type_ =
216196
F.linep sc "Module Prefix: %s" type_.module_prefix;
217197
print_type_spec sc type_.spec;
218-
F.linep sc "Options: %s" (string_of_options type_.type_options);
198+
F.linep sc "Options: %s"
199+
(Format.asprintf "%a" Pb_option.pp_set type_.type_options);
219200
match type_.type_level_ppx_extension with
220201
| Some ext -> F.linep sc "PPX Extension: %s" ext
221202
| None -> ()

src/compilerlib/pb_option.ml

Lines changed: 110 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,39 @@
1+
(*
2+
The MIT License (MIT)
3+
4+
Copyright (c) 2016 Maxime Ransan <[email protected]>
5+
6+
Permission is hereby granted, free of charge, to any person obtaining a copy
7+
of this software and associated documentation files (the "Software"), to deal
8+
in the Software without restriction, including without limitation the rights
9+
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
10+
copies of the Software, and to permit persons to whom the Software is
11+
furnished to do so, subject to the following conditions:
12+
13+
The above copyright notice and this permission notice shall be included in all
14+
copies or substantial portions of the Software.
15+
16+
THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
17+
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
18+
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
19+
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
20+
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
21+
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
22+
SOFTWARE.
23+
24+
*)
25+
126
type constant =
227
| Constant_string of string
328
| Constant_bool of bool
429
| Constant_int of int
530
| Constant_float of float
631
| Constant_literal of string
732

33+
type option_name =
34+
| Simple_name of string
35+
| Extension_name of string
36+
837
type message_literal = (string * value) list
938
and list_literal = value list
1039

@@ -13,19 +42,92 @@ and value =
1342
| Message_literal of message_literal
1443
| List_literal of list_literal
1544

16-
type option_name = string
1745
type t = option_name * value
1846
type set = t list
1947

48+
let stringify_option_name = function
49+
| Simple_name s -> s
50+
| Extension_name s -> "(" ^ s ^ ")"
51+
52+
let option_name_equal a b =
53+
match a, b with
54+
| Simple_name a, Simple_name b -> String.equal a b
55+
| Extension_name a, Extension_name b -> String.equal a b
56+
| _ -> false
57+
2058
let empty = []
21-
let add t option_name value = (option_name, value) :: t
22-
let merge t1 t2 = t2 @ t1
59+
60+
let rec merge_value v1 v2 =
61+
match v1, v2 with
62+
| Message_literal ml1, Message_literal ml2 ->
63+
(* In this case, both the existing and new values are messages.
64+
Iterate through the fields of the new value.
65+
For each field, check if a field with the same name exists in the existing value.
66+
If it does and both field values are messages, merge them recursively.
67+
If it does not, add the new field to the existing message. *)
68+
let rec merge_lists list1 list2 =
69+
match list2 with
70+
| [] -> list1
71+
| (field, value) :: rest ->
72+
let updated_list, is_merged =
73+
List.fold_left
74+
(fun (acc, merged) (f, v) ->
75+
if String.equal f field then (
76+
match value, v with
77+
| Message_literal _, Message_literal _ ->
78+
( acc @ [ f, merge_value value v ],
79+
true (* recursively merges two message literals *) )
80+
| _ -> acc @ [ f, value ], merged
81+
) else
82+
acc @ [ f, v ], merged)
83+
([], false) list1
84+
in
85+
if is_merged then
86+
(* If the current field of list2 was found in list1 and the two
87+
values merged, continue with the rest of list2. The current field of
88+
list2 is not added to updated_list as its value has already been
89+
included during the merge. *)
90+
merge_lists updated_list rest
91+
else
92+
(* If the current field of list2 was not found in list1, add it to
93+
updated_list. *)
94+
merge_lists (updated_list @ [ field, value ]) rest
95+
in
96+
Message_literal (merge_lists ml1 ml2)
97+
| _ ->
98+
(* FIXME: This overrides the scalar value of an existing option with the
99+
scalar value of a new option, which is not allowed as per Protocol Buffer
100+
Language Specification. *)
101+
v2
102+
103+
let add option_set option_name value =
104+
match
105+
List.partition
106+
(fun ((name, _) : t) -> option_name_equal name option_name)
107+
option_set
108+
with
109+
| [], _ ->
110+
(* If the option does not currently exist in the set, add it *)
111+
(option_name, value) :: option_set
112+
| [ (_, existing_value) ], remainder ->
113+
(* If the option already exists in the set, merge it's value with the new value *)
114+
let merged_value = merge_value existing_value value in
115+
(option_name, merged_value) :: remainder
116+
| _ ->
117+
(* This is a sanity check. As we use an equality function, List.partition should
118+
* always partition the list into two lists where the first list has at most one element.
119+
* Hence, the condition that results in a call to failwith should never be satisfied. *)
120+
failwith
121+
"This should not happen, partition should result in at most single item \
122+
in left component"
23123

24124
let get t option_name =
25-
match List.assoc option_name t with
26-
| c -> Some c
125+
match List.find (fun (other, _) -> option_name_equal option_name other) t with
126+
| _, c -> Some c
27127
| exception Not_found -> None
28128

129+
let get_ext t option_name = get t (Extension_name option_name)
130+
29131
let pp_constant ppf = function
30132
| Constant_string s -> Format.fprintf ppf "%S" s
31133
| Constant_bool b -> Format.fprintf ppf "%B" b
@@ -56,7 +158,9 @@ and pp_message_field ppf (field, value) =
56158
Format.fprintf ppf "%S: %a" field pp_value value
57159

58160
let pp_t ppf (name, value) =
59-
Format.fprintf ppf "{@;<1 2>%S: %a@;<1 2>}" name pp_value value
161+
Format.fprintf ppf "{@;<1 2>%S: %a@;<1 2>}"
162+
(stringify_option_name name)
163+
pp_value value
60164

61165
let pp_set ppf set =
62166
Format.fprintf ppf "[@[<v>%a@]]"

0 commit comments

Comments
 (0)