[cost] Separate purity analysis and reporting

Summary: This diff separates purity analysis and its reporting, since sometimes we want to use the purity analysis results in other checkers, but don't want to report purity issues.

Reviewed By: ezgicicek, jvillard

Differential Revision: D23054913

fbshipit-source-id: 12cc1fc42

infer/src/backend/registerCheckers.ml

@@ -77,11 +77,31 @@ let all_checkers =
   (* The order of the list is important for those checkers that depend on other checkers having run
      before them. *)
   [ {checker= SelfInBlock; callbacks= [(intraprocedural SelfInBlock.checker, Clang)]}
-  ; { checker= Purity
+  ; { checker= BufferOverrunAnalysis
+    ; callbacks=
+        (let bo_analysis =
+           interprocedural Payloads.Fields.buffer_overrun_analysis
+             BufferOverrunAnalysis.analyze_procedure
+         in
+         [(bo_analysis, Clang); (bo_analysis, Java)] ) }
+  ; { checker= BufferOverrunChecker
+    ; callbacks=
+        (let bo_checker =
+           interprocedural2 Payloads.Fields.buffer_overrun_checker
+             Payloads.Fields.buffer_overrun_analysis BufferOverrunChecker.checker
+         in
+         [(bo_checker, Clang); (bo_checker, Java)] ) }
+  ; { checker= PurityAnalysis
     ; callbacks=
         (let purity =
            interprocedural2 Payloads.Fields.purity Payloads.Fields.buffer_overrun_analysis
-             Purity.checker
+             PurityAnalysis.checker
+         in
+         [(purity, Java); (purity, Clang)] ) }
+  ; { checker= PurityChecker
+    ; callbacks=
+        (let purity =
+           intraprocedural_with_field_dependency Payloads.Fields.purity PurityChecker.checker
          in
          [(purity, Java); (purity, Clang)] ) }
   ; { checker= Starvation
@@ -161,20 +181,6 @@ let all_checkers =
         [ (intraprocedural_with_payload Payloads.Fields.nullsafe Eradicate.analyze_procedure, Java)
         ; (file NullsafeFileIssues Payloads.Fields.nullsafe FileLevelAnalysis.analyze_file, Java) ]
     }
-  ; { checker= BufferOverrunChecker
-    ; callbacks=
-        (let bo_checker =
-           interprocedural2 Payloads.Fields.buffer_overrun_checker
-             Payloads.Fields.buffer_overrun_analysis BufferOverrunChecker.checker
-         in
-         [(bo_checker, Clang); (bo_checker, Java)] ) }
-  ; { checker= BufferOverrunAnalysis
-    ; callbacks=
-        (let bo_analysis =
-           interprocedural Payloads.Fields.buffer_overrun_analysis
-             BufferOverrunAnalysis.analyze_procedure
-         in
-         [(bo_analysis, Clang); (bo_analysis, Java)] ) }
   ; { checker= Biabduction
     ; callbacks=
         (let biabduction =

infer/src/base/Checker.ml

@@ -27,7 +27,8 @@ type t =
   | NullsafeDeprecated
   | PrintfArgs
   | Pulse
-  | Purity
+  | PurityAnalysis
+  | PurityChecker
   | Quandary
   | RacerD
   | ResourceLeakLabExercise
@@ -252,7 +253,7 @@ let config_unsafe checker =
            for efficiency."
       ; cli_flags= Some {deprecated= []; show_in_help= true}
       ; enabled_by_default= false
-      ; activates= [BufferOverrunAnalysis; Purity] }
+      ; activates= [BufferOverrunAnalysis; PurityAnalysis] }
   | NullsafeDeprecated ->
       { id= "nullsafe"
       ; kind= Internal
@@ -285,7 +286,15 @@ let config_unsafe checker =
       ; cli_flags= Some {deprecated= ["-ownership"]; show_in_help= true}
       ; enabled_by_default= false
       ; activates= [] }
-  | Purity ->
+  | PurityAnalysis ->
+      { id= "purity-analysis"
+      ; kind= Internal
+      ; support= supports_clang_and_java_experimental
+      ; short_documentation= "Internal part of the purity checker."
+      ; cli_flags= None
+      ; enabled_by_default= false
+      ; activates= [BufferOverrunAnalysis] }
+  | PurityChecker ->
       { id= "purity"
       ; kind=
           UserFacing
@@ -295,7 +304,7 @@ let config_unsafe checker =
           "Detects pure (side-effect-free) functions. A different implementation of \"impurity\"."
       ; cli_flags= Some {deprecated= []; show_in_help= true}
       ; enabled_by_default= false
-      ; activates= [BufferOverrunAnalysis] }
+      ; activates= [PurityAnalysis] }
   | Quandary ->
       { id= "quandary"
       ; kind=

infer/src/base/Checker.mli

@@ -26,7 +26,8 @@ type t =
   | NullsafeDeprecated
   | PrintfArgs
   | Pulse
-  | Purity
+  | PurityAnalysis
+  | PurityChecker
   | Quandary
   | RacerD
   | ResourceLeakLabExercise

infer/src/base/IssueType.ml

@@ -775,7 +775,7 @@ let pulse_memory_leak =
 
 
 let pure_function =
-  register ~id:"PURE_FUNCTION" Error Purity
+  register ~id:"PURE_FUNCTION" Error PurityChecker
     ~user_documentation:[%blob "../../documentation/issues/PURE_FUNCTION.md"]
 
 

infer/src/checkers/PurityAnalysis.ml

@@ -179,34 +179,6 @@ end
 
 module Analyzer = LowerHil.MakeAbstractInterpreter (TransferFunctions)
 
-let should_report proc_name =
-  not
-    ( Procname.is_constructor proc_name
-    ||
-    match proc_name with
-    | Procname.Java java_pname ->
-        Procname.Java.is_class_initializer java_pname || Procname.Java.is_access_method java_pname
-    | Procname.ObjC_Cpp name ->
-        Procname.ObjC_Cpp.is_destructor name
-        || Procname.ObjC_Cpp.is_objc_constructor name.method_name
-    | _ ->
-        false )
-
-
-let report_errors {InterproceduralAnalysis.proc_desc; err_log} astate_opt =
-  let proc_name = Procdesc.get_proc_name proc_desc in
-  match astate_opt with
-  | Some astate ->
-      if should_report proc_name && PurityDomain.is_pure astate then
-        let loc = Procdesc.get_loc proc_desc in
-        let exp_desc = F.asprintf "Side-effect free function %a" Procname.pp proc_name in
-        let ltr = [Errlog.make_trace_element 0 loc exp_desc []] in
-        Reporting.log_issue proc_desc err_log ~loc ~ltr Purity IssueType.pure_function exp_desc
-  | None ->
-      L.internal_error "Analyzer failed to compute purity information for %a@." Procname.pp
-        proc_name
-
-
 let compute_summary {InterproceduralAnalysis.proc_desc; tenv; analyze_dependency}
     inferbo_invariant_map =
   let proc_name = Procdesc.get_proc_name proc_desc in
@@ -227,6 +199,5 @@ let checker analysis_data =
       (InterproceduralAnalysis.bind_payload ~f:snd analysis_data)
   in
   let astate_opt = compute_summary analysis_data inferbo_invariant_map in
-  report_errors analysis_data astate_opt ;
   Option.iter astate_opt ~f:(fun astate -> debug "Purity summary :%a \n" PurityDomain.pp astate) ;
   astate_opt

infer/src/checkers/PurityAnalysis.mli

@@ -10,5 +10,3 @@ open! IStd
 val checker :
      (PurityDomain.summary option * BufferOverrunAnalysisSummary.t option) InterproceduralAnalysis.t
   -> PurityDomain.summary option
-
-val should_report : Procname.t -> bool

infer/src/checkers/PurityChecker.ml

@@ -0,0 +1,38 @@
+(*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * 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
+module L = Logging
+
+let should_report proc_name =
+  not
+    ( Procname.is_constructor proc_name
+    ||
+    match proc_name with
+    | Procname.Java java_pname ->
+        Procname.Java.is_class_initializer java_pname || Procname.Java.is_access_method java_pname
+    | Procname.ObjC_Cpp name ->
+        Procname.ObjC_Cpp.is_destructor name
+        || Procname.ObjC_Cpp.is_objc_constructor name.method_name
+    | _ ->
+        false )
+
+
+let checker {IntraproceduralAnalysis.proc_desc; err_log} astate_opt =
+  let proc_name = Procdesc.get_proc_name proc_desc in
+  match astate_opt with
+  | Some astate ->
+      if should_report proc_name && PurityDomain.is_pure astate then
+        let loc = Procdesc.get_loc proc_desc in
+        let exp_desc = F.asprintf "Side-effect free function %a" Procname.pp proc_name in
+        let ltr = [Errlog.make_trace_element 0 loc exp_desc []] in
+        Reporting.log_issue proc_desc err_log ~loc ~ltr PurityChecker IssueType.pure_function
+          exp_desc
+  | None ->
+      L.internal_error "Analyzer failed to compute purity information for %a@." Procname.pp
+        proc_name

infer/src/checkers/PurityChecker.mli

@@ -0,0 +1,12 @@
+(*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *)
+
+open! IStd
+
+val should_report : Procname.t -> bool
+
+val checker : IntraproceduralAnalysis.t -> PurityDomain.summary option -> unit

infer/src/checkers/impurity.ml

@@ -155,7 +155,8 @@ let extract_impurity tenv pdesc (exec_state : ExecutionDomain.t) : ImpurityDomai
   let modified_globals = get_modified_globals pre_heap post post_stack in
   let skipped_calls =
     SkippedCalls.filter
-      (fun proc_name _ -> Purity.should_report proc_name && not (is_modeled_pure tenv proc_name))
+      (fun proc_name _ ->
+        PurityChecker.should_report proc_name && not (is_modeled_pure tenv proc_name) )
       astate.AbductiveDomain.skipped_calls
   in
   {modified_globals; modified_params; skipped_calls; exited}
@@ -185,7 +186,7 @@ let checker {IntraproceduralAnalysis.proc_desc; tenv; err_log} pulse_summary_opt
             let modified = extract_impurity tenv proc_desc exec_state in
             ImpurityDomain.join acc modified )
       in
-      if Purity.should_report proc_name && not (ImpurityDomain.is_pure impurity_astate) then
+      if PurityChecker.should_report proc_name && not (ImpurityDomain.is_pure impurity_astate) then
         let modified_ltr param_source set acc =
           ImpurityDomain.ModifiedVarSet.fold
             (ImpurityDomain.add_to_errlog ~nesting:1 param_source)