diff --git a/src/compilerlib/pb_option.ml b/src/compilerlib/pb_option.ml index 2c90fb9b..554d4966 100644 --- a/src/compilerlib/pb_option.ml +++ b/src/compilerlib/pb_option.ml @@ -60,6 +60,11 @@ let empty = [] let rec merge_value v1 v2 = match v1, v2 with | Message_literal ml1, Message_literal ml2 -> + (* In this case, both the existing and new values are messages. + Iterate through the fields of the new value. + For each field, check if a field with the same name exists in the existing value. + If it does and both field values are messages, merge them recursively. + If it does not, add the new field to the existing message. *) let rec merge_lists list1 list2 = match list2 with | [] -> list1 @@ -67,22 +72,33 @@ let rec merge_value v1 v2 = let updated_list, is_merged = List.fold_left (fun (acc, merged) (f, v) -> - if f = field then ( + if String.equal f field then ( match value, v with | Message_literal _, Message_literal _ -> - acc @ [ f, merge_value value v ], true + ( acc @ [ f, merge_value value v ], + true (* recursively merges two message literals *) ) | _ -> acc @ [ f, value ], merged ) else acc @ [ f, v ], merged) ([], false) list1 in if is_merged then + (* If the current field of list2 was found in list1 and the two + values merged, continue with the rest of list2. The current field of + list2 is not added to updated_list as its value has already been + included during the merge. *) merge_lists updated_list rest else + (* If the current field of list2 was not found in list1, add it to + updated_list. *) merge_lists (updated_list @ [ field, value ]) rest in Message_literal (merge_lists ml1 ml2) - | _ -> v2 (* FIXME: This overrides an existing value, which is not allowed *) + | _ -> + (* FIXME: This overrides the scalar value of an existing option with the + scalar value of a new option, which is not allowed as per Protocol Buffer + Language Specification. *) + v2 let add option_set option_name value = match @@ -90,11 +106,17 @@ let add option_set option_name value = (fun ((name, _) : t) -> option_name_equal name option_name) option_set with - | [], _ -> (option_name, value) :: option_set + | [], _ -> + (* If the option does not currently exist in the set, add it *) + (option_name, value) :: option_set | [ (_, existing_value) ], remainder -> + (* If the option already exists in the set, merge it's value with the new value *) let merged_value = merge_value existing_value value in (option_name, merged_value) :: remainder | _ -> + (* This is a sanity check. As we use an equality function, List.partition should + * always partition the list into two lists where the first list has at most one element. + * Hence, the condition that results in a call to failwith should never be satisfied. *) failwith "This should not happen, partition should result in at most single item \ in left component"