[StaticAnalyzer] Move inline counter increaser to inlineCall function

[szepet]

Summary

Even though the shouldInlineCall function returns true, it can happen that the function is not going to be inlined (as it can be seen at line 913 and below).

Moved the bumpNumTimesInlined(D) (the counter increaser) call to the inlineCall function where it logically belongs.

[NoQ]

Yep, this sounds logical!

I wonder how hard would it be to come up with a test for this. Like, we could set -analyzer-config max-times-inline-large to 2, and then call some large function in a loop with a bug that only manifests in the context of the 4th iteration of the loop.

Because this is a heuristic that controls the analyzer's bail-outs, did you accidentally try to see if the fix actually improves the analyzer's efficiency (in terms of bugs-per-minute, for example)?

[zaks.anna]

Looks good to me as well. Thanks!

[zaks.anna]

I am fine to land this without a test. @szepet, Do you have commit access or should we commit on your behalf?

lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp

if (isNew)
       Engine.getWorkList()->enqueue(N);
   }
 
   // If we decided to inline the call, the successor has been manually
   // added onto the work list so remove it from the node builder.
   Bldr.takeNodes(Pred);
 
   NumInlinedCalls++;
+  Engine.FunctionSummaries->bumpNumTimesInlined(D);
 
   // Mark the decl as visited.
   if (VisitedCallees)
     VisitedCallees->insert(D);
 
   return true;
 }
 
if ((Engine.FunctionSummaries->getNumTimesInlined(D) >
     return false;
   }
 
   if (HowToInline == Inline_Minimal &&
       (CalleeCFG->getNumBlockIDs() > Opts.getAlwaysInlineSize()
       || IsRecursive))
     return false;
 
-  Engine.FunctionSummaries->bumpNumTimesInlined(D);
-
   return true;
 }
 
 static bool isTrivialObjectAssignment(const CallEvent &Call) {
   const CXXInstanceCall *ICall = dyn_cast<CXXInstanceCall>(&Call);
   if (!ICall)
     return false;