On Nov 2, 2012, at 22:18 , Anna Zaks <[email protected]> wrote: > > On Nov 2, 2012, at 8:45 PM, Jordan Rose wrote: > >> This is silly. > What is silly?
Both LocationContext and CheckerContext now have two methods which are opposites (i.e. one returns !other), but they are implemented in different ways. > >> For CheckerContext, inTopFrame is !isInInlined. >> For LocationContext, this shouldn't be virtual; it's just >> getEnclosingStackFrame()->getParent() == 0. > > I don't see the advantage of this. When you ask if a StackFrameContext is > inTopFrame, we would have to call getEnclosingStackFrame instead of just > evaluating "Parent==0". Okay, fair enough. I'm not sure whether the virtual call is going to be faster or slower than the regular call plus an extra dyn_cast, and I suppose I was assuming "slower". Anyway, even if the LocationContext method stands, CheckerContext should be updated to call through to it for both accessors, or one of the accessors should be removed. > Anna. > >> >> >> On Nov 2, 2012, at 19:54 , Anna Zaks <[email protected]> wrote: >> >>> Author: zaks >>> Date: Fri Nov 2 21:54:16 2012 >>> New Revision: 167351 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=167351&view=rev >>> Log: >>> [analyzer] add LocationContext::inTopFrame() helper. >>> >>> Modified: >>> cfe/trunk/include/clang/Analysis/AnalysisContext.h >>> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h >>> cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp >>> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp >>> >>> Modified: cfe/trunk/include/clang/Analysis/AnalysisContext.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/AnalysisContext.h?rev=167351&r1=167350&r2=167351&view=diff >>> ============================================================================== >>> --- cfe/trunk/include/clang/Analysis/AnalysisContext.h (original) >>> +++ cfe/trunk/include/clang/Analysis/AnalysisContext.h Fri Nov 2 21:54:16 >>> 2012 >>> @@ -237,6 +237,9 @@ >>> >>> const StackFrameContext *getCurrentStackFrame() const; >>> >>> + /// Return true if the current LocationContext has no caller context. >>> + virtual bool inTopFrame() const; >>> + >>> virtual void Profile(llvm::FoldingSetNodeID &ID) = 0; >>> >>> public: >>> @@ -271,6 +274,9 @@ >>> >>> const CFGBlock *getCallSiteBlock() const { return Block; } >>> >>> + /// Return true if the current LocationContext has no caller context. >>> + virtual bool inTopFrame() const { return getParent() == 0; } >>> + >>> unsigned getIndex() const { return Index; } >>> >>> void Profile(llvm::FoldingSetNodeID &ID); >>> >>> Modified: >>> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=167351&r1=167350&r2=167351&view=diff >>> ============================================================================== >>> --- >>> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h >>> (original) >>> +++ >>> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h >>> Fri Nov 2 21:54:16 2012 >>> @@ -100,6 +100,9 @@ >>> return Pred->getStackFrame(); >>> } >>> >>> + /// Return true if the current LocationContext has no caller context. >>> + bool inTopFrame() const { return getLocationContext()->inTopFrame(); } >>> + >>> /// Returns true if the predecessor is within an inlined function/method. >>> bool isWithinInlined() { >>> return (getStackFrame()->getParent() != 0); >>> >>> Modified: cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp?rev=167351&r1=167350&r2=167351&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp (original) >>> +++ cfe/trunk/lib/Analysis/AnalysisDeclContext.cpp Fri Nov 2 21:54:16 2012 >>> @@ -355,6 +355,10 @@ >>> return NULL; >>> } >>> >>> +bool LocationContext::inTopFrame() const { >>> + return getCurrentStackFrame()->inTopFrame(); >>> +} >>> + >>> bool LocationContext::isParentOf(const LocationContext *LC) const { >>> do { >>> const LocationContext *Parent = LC->getParent(); >>> >>> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp?rev=167351&r1=167350&r2=167351&view=diff >>> ============================================================================== >>> --- cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp (original) >>> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp Fri Nov 2 >>> 21:54:16 2012 >>> @@ -3190,12 +3190,6 @@ >>> // Handle return statements. >>> //===----------------------------------------------------------------------===// >>> >>> -// Return true if the current LocationContext has no caller context. >>> -static bool inTopFrame(CheckerContext &C) { >>> - const LocationContext *LC = C.getLocationContext(); >>> - return LC->getParent() == 0; >>> -} >>> - >>> void RetainCountChecker::checkPreStmt(const ReturnStmt *S, >>> CheckerContext &C) const { >>> >>> @@ -3204,7 +3198,7 @@ >>> // better checking even for inlined calls, and see if they match >>> // with their expected semantics (e.g., the method should return a retained >>> // object, etc.). >>> - if (!inTopFrame(C)) >>> + if (!C.inTopFrame()) >>> return; >>> >>> const Expr *RetE = S->getRetValue(); >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
