From eab7e9aeb7eaf3fc0be7edee0c1a076aa9ee1e78 Mon Sep 17 00:00:00 2001 From: Jules Villard Date: Tue, 26 May 2020 03:46:32 -0700 Subject: [PATCH] minor readability improvement in IssueType.ml Summary: - avoid creating issues just to look up their `unique_id` in the set - avoid `let _ =` since it can hide partial applications - delete outdated comment Reviewed By: skcho Differential Revision: D21663959 fbshipit-source-id: e50d02447 --- infer/src/base/IssueType.ml | 53 +++++++++++++++++------------- infer/src/istd/PrettyPrintable.ml | 32 ++++++++++++++---- infer/src/istd/PrettyPrintable.mli | 21 ++++++++++-- infer/src/pulse/PulseAttribute.ml | 4 ++- 4 files changed, 76 insertions(+), 34 deletions(-) diff --git a/infer/src/base/IssueType.ml b/infer/src/base/IssueType.ml index 51fa9b3f8..33c6ab91b 100644 --- a/infer/src/base/IssueType.ml +++ b/infer/src/base/IssueType.ml @@ -4,7 +4,9 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. *) + open! IStd +module F = Format (* Make sure we cannot create new issue types other than by calling [register_from_string]. This is because we want to keep track of the list of all the issues ever declared. *) @@ -20,6 +22,8 @@ module Unsafe : sig val equal : t -> t -> bool + val pp : F.formatter -> t -> unit + val register_from_string : ?enabled:bool -> ?hum:string @@ -33,7 +37,7 @@ module Unsafe : sig ?enabled:bool -> ?is_on_ui_thread:bool -> kind:CostKind.t - -> (string -> string, Format.formatter, unit, string) format4 + -> (string -> string, F.formatter, unit, string) format4 -> t val all_issues : unit -> t list @@ -52,10 +56,16 @@ end = struct let compare {unique_id= id1} {unique_id= id2} = String.compare id1 id2 let equal = [%compare.equal: t] + + type rank = string + + let to_rank {unique_id} = unique_id + + let pp fmt t = F.pp_print_string fmt t.unique_id end include T - module IssueSet = Caml.Set.Make (T) + module IssueSet = PrettyPrintable.MakePPUniqRankSet (String) (T) (** keep track of the list of all declared issue types *) let all_issues = ref IssueSet.empty @@ -80,25 +90,25 @@ end = struct of the issue type, eg in AL. *) let register_from_string ?(enabled = true) ?hum:hum0 ?doc_url ?linters_def_file ~id:unique_id checker = - let hum = match hum0 with Some str -> str | _ -> prettify unique_id in - let issue = {unique_id; checker; enabled; hum; doc_url; linters_def_file} in - try - let old = IssueSet.find issue !all_issues in - (* update human-readable string in case it was supplied this time, but keep the previous - value of enabled (see doc comment) *) - if Option.is_some hum0 then old.hum <- hum ; - if Option.is_some doc_url then old.doc_url <- doc_url ; - if Option.is_some linters_def_file then old.linters_def_file <- linters_def_file ; - old - with Caml.Not_found -> - all_issues := IssueSet.add issue !all_issues ; - issue + match IssueSet.find_rank !all_issues unique_id with + | Some issue -> + (* update human-readable string in case it was supplied this time, but keep the previous + value of enabled (see doc comment) *) + Option.iter hum0 ~f:(fun hum -> issue.hum <- hum) ; + if Option.is_some doc_url then issue.doc_url <- doc_url ; + if Option.is_some linters_def_file then issue.linters_def_file <- linters_def_file ; + issue + | None -> + let hum = match hum0 with Some str -> str | _ -> prettify unique_id in + let issue = {unique_id; checker; enabled; hum; doc_url; linters_def_file} in + all_issues := IssueSet.add !all_issues issue ; + issue (** cost issues are already registered below.*) let register_from_cost_string ?(enabled = true) ?(is_on_ui_thread = false) ~(kind : CostKind.t) s = - let issue_type_base = Format.asprintf s (CostKind.to_issue_string kind) in + let issue_type_base = F.asprintf s (CostKind.to_issue_string kind) in let issue_type = if is_on_ui_thread then issue_type_base ^ "_UI_THREAD" else issue_type_base in register_from_string ~enabled ~id:issue_type Cost @@ -108,9 +118,6 @@ end include Unsafe -(** pretty print a localised string *) -let pp fmt t = Format.pp_print_string fmt t.unique_id - let checker_can_report reporting_checker {checker= allowed_checker} = Checker.equal reporting_checker allowed_checker @@ -627,8 +634,8 @@ let unreachable_cost_call ~kind = let () = List.iter CostKind.enabled_cost_kinds ~f:(fun CostKind.{kind} -> List.iter [true; false] ~f:(fun is_on_ui_thread -> - let _ = unreachable_cost_call ~kind in - let _ = expensive_cost_call ~kind ~is_on_ui_thread in - let _ = infinite_cost_call ~kind in - let _ = complexity_increase ~kind ~is_on_ui_thread in + ignore (unreachable_cost_call ~kind) ; + ignore (expensive_cost_call ~kind ~is_on_ui_thread) ; + ignore (infinite_cost_call ~kind) ; + ignore (complexity_increase ~kind ~is_on_ui_thread) ; () ) ) diff --git a/infer/src/istd/PrettyPrintable.ml b/infer/src/istd/PrettyPrintable.ml index 16ce28ff7..87bea2ded 100644 --- a/infer/src/istd/PrettyPrintable.ml +++ b/infer/src/istd/PrettyPrintable.ml @@ -28,6 +28,12 @@ module type PrintableOrderedType = sig include PrintableType with type t := t end +module type PrintableEquatableOrderedType = sig + include Caml.Set.OrderedType + + include PrintableEquatableType with type t := t +end + module type PPSet = sig include Caml.Set.S @@ -209,12 +215,16 @@ module type PrintableRankedType = sig val equal : t -> t -> bool - val to_rank : t -> int + type rank + + val to_rank : t -> rank end module type PPUniqRankSet = sig type t + type rank + type elt val add : t -> elt -> t @@ -223,7 +233,7 @@ module type PPUniqRankSet = sig val equal : t -> t -> bool - val find_rank : t -> int -> elt option + val find_rank : t -> rank -> elt option val fold : t -> init:'accum -> f:('accum -> elt -> 'accum) -> 'accum @@ -239,6 +249,8 @@ module type PPUniqRankSet = sig val singleton : elt -> t + val elements : t -> elt list + val remove : elt -> t -> t val union_prefer_left : t -> t -> t @@ -246,11 +258,16 @@ module type PPUniqRankSet = sig val pp : ?print_rank:bool -> F.formatter -> t -> unit end -module MakePPUniqRankSet (Val : PrintableRankedType) : PPUniqRankSet with type elt = Val.t = struct - module Map = MakePPMonoMap (Int) (Val) +module MakePPUniqRankSet + (Rank : PrintableEquatableOrderedType) + (Val : PrintableRankedType with type rank = Rank.t) : + PPUniqRankSet with type elt = Val.t and type rank = Rank.t = struct + module Map = MakePPMonoMap (Rank) (Val) type t = Map.t + type rank = Rank.t + type elt = Val.t let add map value = Map.add (Val.to_rank value) value map @@ -278,7 +295,7 @@ module MakePPUniqRankSet (Val : PrintableRankedType) : PPUniqRankSet with type e Map.mapi (fun rank value -> let value' = f value in - assert (Int.equal rank (Val.to_rank value')) ; + assert (Rank.equal rank (Val.to_rank value')) ; value' ) m @@ -294,9 +311,10 @@ module MakePPUniqRankSet (Val : PrintableRankedType) : PPUniqRankSet with type e (!accum, m') + let elements map = Map.bindings map |> List.map ~f:snd + let pp ?(print_rank = false) fmt map = - if print_rank then Map.pp fmt map - else pp_collection ~pp_item:Val.pp fmt (Map.bindings map |> List.map ~f:snd) + if print_rank then Map.pp fmt map else pp_collection ~pp_item:Val.pp fmt (elements map) let remove value map = Map.remove (Val.to_rank value) map diff --git a/infer/src/istd/PrettyPrintable.mli b/infer/src/istd/PrettyPrintable.mli index 759bf4426..259397b6c 100644 --- a/infer/src/istd/PrettyPrintable.mli +++ b/infer/src/istd/PrettyPrintable.mli @@ -30,6 +30,12 @@ module type PrintableOrderedType = sig include PrintableType with type t := t end +module type PrintableEquatableOrderedType = sig + include Caml.Set.OrderedType + + include PrintableEquatableType with type t := t +end + module type PPSet = sig include Caml.Set.S @@ -159,13 +165,17 @@ module type PrintableRankedType = sig val equal : t -> t -> bool - val to_rank : t -> int + type rank + + val to_rank : t -> rank end (** set where at most one element of a given rank can be present *) module type PPUniqRankSet = sig type t + type rank + type elt val add : t -> elt -> t @@ -174,7 +184,7 @@ module type PPUniqRankSet = sig val equal : t -> t -> bool - val find_rank : t -> int -> elt option + val find_rank : t -> rank -> elt option val fold : t -> init:'accum -> f:('accum -> elt -> 'accum) -> 'accum @@ -190,6 +200,8 @@ module type PPUniqRankSet = sig val singleton : elt -> t + val elements : t -> elt list + val remove : elt -> t -> t val union_prefer_left : t -> t -> t @@ -199,4 +211,7 @@ module type PPUniqRankSet = sig val pp : ?print_rank:bool -> F.formatter -> t -> unit end -module MakePPUniqRankSet (Val : PrintableRankedType) : PPUniqRankSet with type elt = Val.t +module MakePPUniqRankSet + (Rank : PrintableEquatableOrderedType) + (Val : PrintableRankedType with type rank = Rank.t) : + PPUniqRankSet with type elt = Val.t and type rank = Rank.t diff --git a/infer/src/pulse/PulseAttribute.ml b/infer/src/pulse/PulseAttribute.ml index 4057573ea..74706f401 100644 --- a/infer/src/pulse/PulseAttribute.ml +++ b/infer/src/pulse/PulseAttribute.ml @@ -37,6 +37,8 @@ module Attribute = struct let equal = [%compare.equal: t] + type rank = int + let to_rank = Variants.to_rank let dummy_trace = Trace.Immediate {location= Location.dummy; history= []} @@ -91,7 +93,7 @@ module Attribute = struct end module Attributes = struct - module Set = PrettyPrintable.MakePPUniqRankSet (Attribute) + module Set = PrettyPrintable.MakePPUniqRankSet (Int) (Attribute) let get_invalid attrs = Set.find_rank attrs Attribute.invalid_rank