[cost] Do not report `EXPENSIVE_EXECUTION_TIME` when a cost of a node is top

Summary:
> We don't report when the cost is Top as it corresponds to subsequent 'don't know's. Instead, we
> report Top cost only at the top level per function

The previous code just ignored top costed nodes, so it was able to report a non-top cost that was
from another node.  For example,

```
void foo() {
     linear-cost();
     top-cost();
}
```

It reported inconsistent reports: `EXPENSIVE_EXECUTION_TIME` with a linear cost and
`INFINITE_EXECUTION_TIME` at the same time.

This diff fixes it not to report `EXPENSIVE_EXECUTION_TIME` when there is a node with the top cost.

Reviewed By: ezgicicek

Differential Revision: D20139408

fbshipit-source-id: 9fedd4aec

infer/src/cost/cost.ml

@@ -140,6 +140,7 @@ module ThresholdReports = struct
   type threshold_or_report =
     | Threshold of BasicCost.t
     | ReportOn of {location: Location.t; cost: BasicCost.t}
+    | NoReport
 
   type t = threshold_or_report CostIssues.CostKindMap.t
 
@@ -163,10 +164,6 @@ module WorstCaseCost = struct
 
   (** We don't report when the cost is Top as it corresponds to subsequent 'don't know's. Instead,
       we report Top cost only at the top level per function. *)
-  let should_report_cost cost ~threshold =
-    (not (BasicCost.is_top cost)) && not (BasicCost.leq ~lhs:cost ~rhs:threshold)
-
-
   let exec_node tenv {costs; reports} extras instr_node =
     let {get_node_nb_exec} = extras in
     let node_cost =
@@ -183,13 +180,16 @@ module WorstCaseCost = struct
       CostIssues.CostKindMap.merge
         (fun _kind threshold_or_report_opt cost_opt ->
           match (threshold_or_report_opt, cost_opt) with
-          | None, _ ->
+          | Some ThresholdReports.NoReport, _ ->
-              None
+              threshold_or_report_opt
+          | Some ThresholdReports.(Threshold _ | ReportOn _), Some cost when BasicCost.is_top cost
+            ->
+              Some ThresholdReports.NoReport
           | Some (ThresholdReports.Threshold threshold), Some cost
-            when should_report_cost cost ~threshold ->
+            when not (BasicCost.leq ~lhs:cost ~rhs:threshold) ->
               Some (ThresholdReports.ReportOn {location= InstrCFG.Node.loc instr_node; cost})
           | Some (ThresholdReports.ReportOn {cost= prev}), Some cost
-            when (not (BasicCost.is_top cost)) && BasicCost.compare_by_degree prev cost < 0 ->
+            when BasicCost.compare_by_degree prev cost < 0 ->
               Some (ThresholdReports.ReportOn {location= InstrCFG.Node.loc instr_node; cost})
           | _ ->
               threshold_or_report_opt )
@@ -267,7 +267,7 @@ module Check = struct
     if not (Procname.is_java_access_method pname) then (
       CostIssues.CostKindMap.iter2 CostIssues.enabled_cost_map reports
         ~f:(fun _kind (CostIssues.{name; threshold} as kind_spec) -> function
-        | ThresholdReports.Threshold _ ->
+        | ThresholdReports.Threshold _ | ThresholdReports.NoReport ->
             ()
         | ThresholdReports.ReportOn {location; cost} ->
             report_threshold pname summary ~name ~location ~cost kind_spec

infer/tests/codetoanalyze/java/performance/issues.exp

@@ -4,7 +4,6 @@ codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.
 codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.binary_search_log(java.lang.String[]):int, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 2 + log(arr.length), O(log(arr.length)), degree = 0 + 1⋅log,{arr.length},Modeled call to Arrays.binarySearch]
 codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.bsearch_log(int):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 5 + log(i), O(log(i)), degree = 0 + 1⋅log,{i},Modeled call to Arrays.binarySearch]
 codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.call_gen_and_iter_types(int):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 13 + 5 ⋅ x, O(x), degree = 1,{x},call to java.lang.String[] Array.gen_and_iter_types(int),Loop at line 76]
-codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.call_gen_and_iter_types_linear_FP(int,int):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 13 + 5 ⋅ x, O(x), degree = 1,{x},call to java.lang.String[] Array.gen_and_iter_types(int),Loop at line 76]
 codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.call_gen_and_iter_types_linear_FP(int,int):void, 0, INFINITE_EXECUTION_TIME, no_bucket, ERROR, [Unbounded loop,Loop at line 88]
 codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.call_gen_and_iter_types_linear_FP(int,int):void, 2, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [<LHS trace>,Assignment,Binary operation: ([0, +oo] + 1):signed32]
 codetoanalyze/java/performance/Array.java, codetoanalyze.java.performance.Array.copyOf_linear(java.lang.String[]):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 4 + arr.length, O(arr.length), degree = 1,{arr.length},Modeled call to Arrays.copyOf]
@@ -172,7 +171,6 @@ codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.
 codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.string_concat_linear(java.lang.String,java.lang.String):void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 8 + 5 ⋅ (p.length + s.length) + 3 ⋅ (p.length + s.length + 1), O((p.length + s.length)), degree = 1,{p.length + s.length + 1},Loop at line 103,{p.length + s.length},Loop at line 103]
 codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.string_length_linear(java.lang.String):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 2 + 5 ⋅ s.length + 3 ⋅ (s.length + 1), O(s.length), degree = 1,{s.length + 1},Loop at line 98,{s.length},Loop at line 98]
 codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.unboundedSymbol():void, 0, INFINITE_EXECUTION_TIME, no_bucket, ERROR, [Unbounded value x,call to void Loops.linear(int),Loop at line 86]
-codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.unboundedSymbol():void, 1, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 6996, O(1), degree = 0]
 codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.unboundedSymbol():void, 2, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [<LHS trace>,Assignment,<RHS trace>,Assignment,Binary operation: ([-oo, +oo] × [-oo, +oo]):signed32]
 codetoanalyze/java/performance/Loops.java, codetoanalyze.java.performance.Loops.unboundedSymbol():void, 4, INTEGER_OVERFLOW_L5, no_bucket, ERROR, [Assignment,Call,<LHS trace>,Assignment,Binary operation: ([0, +oo] + 1):signed32 by call to `void Loops.linear(int)` ]
 codetoanalyze/java/performance/MapTest.java, MapTest.entrySet_linear(java.util.Map):void, 0, EXPENSIVE_EXECUTION_TIME, no_bucket, ERROR, [with estimated cost 7 + 8 ⋅ map.length + 3 ⋅ (map.length + 1), O(map.length), degree = 1,{map.length + 1},Loop at line 17,{map.length},Loop at line 17]