[cfe-commits] r167351 - in /cfe/trunk: include/clang/Analysis/AnalysisContext.h include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h lib/Analysis/AnalysisDeclContext.cpp lib/StaticAnalyzer/Checkers/RetainCountChecker.cpp

[Jordan Rose]

On Nov 2, 2012, at 22:18 , Anna Zaks <ganna at apple.com> 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 <ganna at apple.com> 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
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> 
>