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

[David Blaikie]

Oops, was looking at the wrong ~CallGraph (one in Clang, not in LLVM).

This one's a bit more involved & possibly trickier to make movable:
http://llvm.org/docs/doxygen/html/CallGraph_8cpp_source.html#l00035

Hmm, maybe not - just unique_ptrifying the relevant members and moving them
over should probably suffice.

On Wed, Aug 5, 2015 at 10:47 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> 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/58fcf727/attachment.html>