[llvm] r196095 - [PM] [cleanup] Rearrange the public and private sections of this class

[David Blaikie]

On Mon, Dec 2, 2013 at 4:35 AM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> Author: chandlerc
> Date: Mon Dec  2 06:35:56 2013
> New Revision: 196095
>
> URL: http://llvm.org/viewvc/llvm-project?rev=196095&view=rev
> Log:
> [PM] [cleanup] Rearrange the public and private sections of this class
> to be a bit more sensible. The public interface now is first followed by
> the implementation details.
>
> This also resolves a FIXME to make something private -- it was already
> possible as the one special caller was already a friend.
>
> No functionality changed.
>
> Modified:
>     llvm/trunk/include/llvm/Analysis/CallGraph.h
>
> Modified: llvm/trunk/include/llvm/Analysis/CallGraph.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/CallGraph.h?rev=196095&r1=196094&r2=196095&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/CallGraph.h (original)
> +++ llvm/trunk/include/llvm/Analysis/CallGraph.h Mon Dec  2 06:35:56 2013
> @@ -165,28 +165,11 @@ public:
>  /// Typically represents a function in the call graph. There are also
> special
>  /// "null" nodes used to represent theoretical entries in the call graph.
>  class CallGraphNode {
> -  friend class CallGraph;
> -
> -  AssertingVH<Function> F;
> -
>  public:
>    /// \brief A pair of the calling instruction (a call or invoke)
>    /// and the call graph node being called.
>    typedef std::pair<WeakVH, CallGraphNode *> CallRecord;
>
> -private:
> -  std::vector<CallRecord> CalledFunctions;
> -
> -  /// \brief The number of times that this CallGraphNode occurs in the
> -  /// CalledFunctions array of this or other CallGraphNodes.
> -  unsigned NumReferences;
> -
> -  CallGraphNode(const CallGraphNode &) LLVM_DELETED_FUNCTION;
> -  void operator=(const CallGraphNode &) LLVM_DELETED_FUNCTION;
> -
> -  void DropRef() { --NumReferences; }
> -  void AddRef() { ++NumReferences; }
> -
>  public:
>    typedef std::vector<CallRecord> CalledFunctionsVector;
>
> @@ -281,12 +264,48 @@ public:
>    /// Note that this method takes linear time, so it should be used
> sparingly.
>    void replaceCallEdge(CallSite CS, CallSite NewCS, CallGraphNode
> *NewNode);
>
> +private:
> +  friend class CallGraph;
> +
> +  AssertingVH<Function> F;
> +
> +  std::vector<CallRecord> CalledFunctions;
> +
> +  /// \brief The number of times that this CallGraphNode occurs in the
> +  /// CalledFunctions array of this or other CallGraphNodes.
> +  unsigned NumReferences;
> +
> +  CallGraphNode(const CallGraphNode &) LLVM_DELETED_FUNCTION;
> +  void operator=(const CallGraphNode &) LLVM_DELETED_FUNCTION;
> +
> +  void DropRef() { --NumReferences; }
> +  void AddRef() { ++NumReferences; }
> +
>    /// \brief A special function that should only be used by the CallGraph
> class.
> -  //
> -  // FIXME: Make this private?
>    void allReferencesDropped() { NumReferences = 0; }
>  };
>
> +/// \brief An analysis pass to compute the \c CallGraph for a \c Module.
> +///
> +/// This class implements the concept of an analysis pass used by the \c
> +/// ModuleAnalysisManager to run an analysis over a module and cache the
> +/// resulting data.
> +class CallGraphAnalysis {
> +public:
> +  /// \brief A formulaic typedef to inform clients of the result type.
> +  typedef CallGraph Result;
> +
> +  static void *ID() { return (void *)&PassID; }
> +
> +  /// \brief Compute the \c CallGraph for the module \c M.
> +  ///
> +  /// The real work here is done in the \c CallGraph constructor.
> +  CallGraph run(Module *M) { return CallGraph(*M); }
>

This introduces a call to the implicit copy ctor of CallGraph which is
probably incorrect (since ~CallGraph involves a call to "
llvm::DeleteContainerSeconds(FunctionMap);").

The type can probably be made implicitly movable by just updating
FunctionMap's values to be unique_ptrs, but this code also seems untested
(changing the definition of 'run' to be "= delete" doesn't break the
build). Should it be removed? updated? tested in some way?

(found this while cleaning up -Wdeprecated warnings in LLVM)

- David


> +
> +private:
> +  static char PassID;
> +};
> +
>  /// \brief The \c ModulePass which wraps up a \c CallGraph and the logic
> to
>  /// build it.
>  ///
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150805/2dcb995f/attachment.html>