[cfe-commits] r169563 - in /cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive: CallEvent.h CheckerContext.h ExplodedGraph.h

[David Blaikie]

On Thu, Dec 6, 2012 at 3:55 PM, Jordan Rose <jordan_rose at apple.com> 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

> -  CallEvent(const Decl *D, ProgramStateRef state, const LocationContext *lctx)
> +  CallEvent(const Decl *D, const ProgramStateRef &state,
> +            const LocationContext *lctx)
>      : State(state), LCtx(lctx), Origin(D), RefCount(0) {}
>
>    // DO NOT MAKE PUBLIC
> @@ -181,7 +183,7 @@
>    }
>
>    /// \brief The state in which the call is being evaluated.
> -  ProgramStateRef getState() const {
> +  const ProgramStateRef &getState() const {
>      return State;
>    }
>
>
> 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=169563&r1=169562&r2=169563&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h Thu Dec  6 17:55:34 2012
> @@ -119,7 +119,7 @@
>    /// the state of the program before the checker ran. Note, checkers should
>    /// not retain the node in their state since the nodes might get invalidated.
>    ExplodedNode *getPredecessor() { return Pred; }
> -  ProgramStateRef getState() const { return Pred->getState(); }
> +  const ProgramStateRef &getState() const { return Pred->getState(); }
>
>    /// \brief Check if the checker changed the state of the execution; ex: added
>    /// a new transition or a bug report.
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h?rev=169563&r1=169562&r2=169563&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h Thu Dec  6 17:55:34 2012
> @@ -122,7 +122,7 @@
>
>  public:
>
> -  explicit ExplodedNode(const ProgramPoint &loc, ProgramStateRef state,
> +  explicit ExplodedNode(const ProgramPoint &loc, const ProgramStateRef &state,
>                          bool IsSink)
>      : Location(loc), State(state), Succs(IsSink) {
>      assert(isSink() == IsSink);
> @@ -152,7 +152,7 @@
>      return *getLocationContext()->getAnalysis<T>();
>    }
>
> -  ProgramStateRef getState() const { return State; }
> +  const ProgramStateRef &getState() const { return State; }
>
>    template <typename T>
>    const T* getLocationAs() const LLVM_LVALUE_FUNCTION {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits