On Dec 6, 2012, at 16:07 , David Blaikie <[email protected]> wrote:
> On Thu, Dec 6, 2012 at 3:55 PM, Jordan Rose <[email protected]> wrote: >> Author: jrose >> Date: Thu Dec 6 17:55:34 2012 >> New Revision: 169563 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=169563&view=rev >> Log: >> [analyzer] Avoid ProgramStateRef copy constructors. >> >> Suggested by David Blaikie. ExplodedNode, CallEvent, and CheckerContext all >> hang onto their ProgramState, so the accessors can return a reference to the >> internal state rather than preemptively copying it. This helps avoid >> temporary ProgramStateRefs, though local variables will still (correctly) >> do an extra retain and release. >> >> Modified: >> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h >> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h >> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h >> >> Modified: >> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h?rev=169563&r1=169562&r2=169563&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h >> (original) >> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h >> Thu Dec 6 17:55:34 2012 >> @@ -142,10 +142,12 @@ >> protected: >> friend class CallEventManager; >> >> - CallEvent(const Expr *E, ProgramStateRef state, const LocationContext >> *lctx) >> + CallEvent(const Expr *E, const ProgramStateRef &state, >> + const LocationContext *lctx) >> : State(state), LCtx(lctx), Origin(E), RefCount(0) {} > > FWIW - when passing in to take ownership (which wasn't the case in the > particular call chain I was looking at in your last review - that one > just called getPtr()) you could prefer to pass by value and use swap > to transfer ownership. This will avoid a retain/release in the case of > passing in a temporary. > > (passing things by value and using swap (or move assignment in C++11) > to take ownership is a nice habit to get into as it becomes more > common/powerful/useful in C++11 with movable types being a common > situation (whereas these days optimizing for swappable types in this > way is a bit less common)) > > Just a thought, in case this is important to you/this code. > > - David IntrusiveRefCntPtr is already properly move-assignable. These methods don't acquire ownership in general, though...there really are cases where multiple CallEvents or ExplodedNodes may refer to the same ProgramState. There are a few cases, though, where the local variables that feed into these things aren't being used afterwards (or if they come from temporaries). We probably would see a win just compiling as C++11. Thanks! Jordan _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
