YAY. I could not figure out how to do this, especially with the block part.
A few comments. On Feb 21, 2013, at 18:59 , Anna Zaks <[email protected]> wrote: > Author: zaks > Date: Thu Feb 21 20:59:24 2013 > New Revision: 175857 > > URL: http://llvm.org/viewvc/llvm-project?rev=175857&view=rev > Log: > [analyzer] Place all inlining policy checks into one palce > > Previously, we had the decisions about inlining spread out > over multiple functions. > > In addition to the refactor, this commit ensures > that we will always inline BodyFarm functions as long as the Decl > is available. This fixes false positives due to those functions > not being inlined when no or minimal inlining is enabled such (as > shallow mode). > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp > cfe/trunk/test/Analysis/NSString.m > > + const LocationContext *ParentOfCallee = CallerSFC; > + if (Call.getKind() == CE_Block) { > const BlockDataRegion *BR = cast<BlockCall>(Call).getBlockRegion(); This should probably just use dyn_cast now -- more idiomatic. > +static bool shouldInlineCallKind(const CallEvent &Call, > + const ExplodedNode *Pred, > + AnalyzerOptions &Opts) { Indentation is wrong. > +bool ExprEngine::shouldInlineCall(const CallEvent &Call, const Decl *D, > + const ExplodedNode *Pred) { > + if (!D ) > + return false; Extra space? > + AnalyzerOptions &Opts = getAnalysisManager().options; > + AnalysisDeclContext *CalleeADC = > + Call.getLocationContext()->getAnalysisDeclContext()-> > + getManager()->getContext(D); AnalysisManager &AMgr = getAnalysisManager(); AnalyzerOptions &Opts = AMgr.options; AnalysisDeclContextManager &ADCMgr = AMgr.getAnalysisDeclContextManager(); AnalysisDeclContext *CalleeADC = ADCMgr.getContext(D); > + // The auto-synthesized bodies are essential to inline as they are > + // usually small and commonly used. note, we should do this check early on > to > + // ensure we always inline these calls. Capitalization. (Also, conventionally a colon instead of a comma after "Note".) > + if (CalleeADC->isBodyAutosynthesized()) > + return true; > + > + if (HowToInline == Inline_None) > + return false; > + > + // Check if we should inline a call based on it's kind. s/it's/its/ > + if (!shouldInlineCallKind(Call, Pred, Opts)) > + return false; > + > + // It is possible that the CFG cannot be constructed. > + // Be safe, and check if the CalleeCFG is valid. > + const CFG *CalleeCFG = CalleeADC->getCFG(); > + if (!CalleeCFG) > + return false; > + > + // Do not inline if recursive or we've reached max stack frame count. > + bool IsRecursive = false; > + unsigned StackDepth = 0; > + examineStackFrames(D, Pred->getLocationContext(), IsRecursive, StackDepth); > + if ((StackDepth >= Opts.InlineMaxStackDepth) && > + ((CalleeCFG->getNumBlockIDs() > Opts.getAlwaysInlineSize()) > + || IsRecursive)) > + return false; > + > + // Do not inline if it took too long to inline previously. > + if (Engine.FunctionSummaries->hasReachedMaxBlockCount(D)) > + return false; > + > + // Or if the function is too big. > + if (CalleeCFG->getNumBlockIDs() > Opts.getMaxInlinableSize()) > + return false; > + > + // Do not inline variadic calls (for now). > + if (const BlockDecl *BD = dyn_cast<BlockDecl>(D)) { > + if (BD->isVariadic()) > + return false; > + } > + else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) { > + if (FD->isVariadic()) > + return false; > + } Now that there's a CallEvent here, we should make a CallEvent::isVariadic, since ObjC method calls can be variadic as well. > + // Check our template policy. > + if (getContext().getLangOpts().CPlusPlus) { > + if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) { > + // Conditionally allow the inlining of template functions. > + if (!Opts.mayInlineTemplateFunctions()) > + if (FD->getTemplatedKind() != FunctionDecl::TK_NonTemplate) > + return false; > + > + // Conditionally allow the inlining of C++ standard library functions. > + if (!Opts.mayInlineCXXStandardLibrary()) > + if > (getContext().getSourceManager().isInSystemHeader(FD->getLocation())) > + if (IsInStdNamespace(FD)) > + return false; > + } > + } > + > + // It is possible that the live variables analysis cannot be > + // run. If so, bail out. > + if (!CalleeADC->getAnalysis<RelaxedLiveVariables>()) > + return false; > + > + // Do not inline large functions too many times. > + if (Engine.FunctionSummaries->getNumTimesInlined(D) > > + Opts.getMaxTimesInlineLarge() && > + CalleeCFG->getNumBlockIDs() > 13) { The indentation here confused me at first; IMHO, "Opts.getMaxTimesInlineLarge()" should be extra-indented, or perhaps the comparison should be wrapped in parens. The magic number 13 is pretty bad, too... (I realize these aren't new problems, but they're here.) _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
