[llvm] r245192 - [ADT] Teach FoldingSet to be movable.

[Chandler Carruth via llvm-commits]

Annoyingly we only have a unit test for the hashing part of FoldingSet. =/
I didn't really want to add one just for this but I can.

I do have a patch under review already that uses this (and fails
spectacularly if this patch isn't in place) so it won't be uncovered by
code in-tree, but I do see the value of unittests in general for this kind
of stuff.

On Sun, Aug 16, 2015 at 4:22 PM David Blaikie via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> On Sun, Aug 16, 2015 at 4:17 PM, Chandler Carruth via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: chandlerc
>> Date: Sun Aug 16 18:17:27 2015
>> New Revision: 245192
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=245192&view=rev
>> Log:
>> [ADT] Teach FoldingSet to be movable.
>>
>> This is a very minimal move support - it leaves the moved-from object in
>> a zombie state that is only valid for destruction and move assignment.
>> This seems fine to me, and leaving it in the default constructed state
>> would require adding more state to the object and potentially allocating
>> memory (!!!) and so seems like a Bad Idea.
>>
>
> Might be worth a unit test (possibly with a move-only value, or even a
> move-and-copyable value that checks that it only gets moved - actually, I
> assume neither happens, so perhaps it can be done with a non-copyable,
> non-movable type?)
>
> - Dave
>
>
>>
>> Modified:
>>     llvm/trunk/include/llvm/ADT/FoldingSet.h
>>     llvm/trunk/lib/Support/FoldingSet.cpp
>>
>> Modified: llvm/trunk/include/llvm/ADT/FoldingSet.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/FoldingSet.h?rev=245192&r1=245191&r2=245192&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/ADT/FoldingSet.h (original)
>> +++ llvm/trunk/include/llvm/ADT/FoldingSet.h Sun Aug 16 18:17:27 2015
>> @@ -122,9 +122,10 @@ protected:
>>    /// is greater than twice the number of buckets.
>>    unsigned NumNodes;
>>
>> -  ~FoldingSetImpl();
>> -
>>    explicit FoldingSetImpl(unsigned Log2InitSize = 6);
>> +  FoldingSetImpl(FoldingSetImpl &&Arg);
>> +  FoldingSetImpl &operator=(FoldingSetImpl &&RHS);
>> +  ~FoldingSetImpl();
>>
>>  public:
>>
>>  //===--------------------------------------------------------------------===//
>> @@ -391,6 +392,10 @@ DefaultContextualFoldingSetTrait<T, Ctx>
>>  /// implementation of the folding set to the node class T.  T must be a
>>  /// subclass of FoldingSetNode and implement a Profile function.
>>  ///
>> +/// Note that this set type is movable and move-assignable. However, its
>> +/// moved-from state is not a valid state for anything other than
>> +/// move-assigning and destroying. This is primarily to enable movable
>> APIs
>> +/// that incorporate these objects.
>>  template <class T> class FoldingSet final : public FoldingSetImpl {
>>  private:
>>    /// GetNodeProfile - Each instantiatation of the FoldingSet needs to
>> provide a
>> @@ -415,8 +420,13 @@ private:
>>
>>  public:
>>    explicit FoldingSet(unsigned Log2InitSize = 6)
>> -  : FoldingSetImpl(Log2InitSize)
>> -  {}
>> +      : FoldingSetImpl(Log2InitSize) {}
>> +
>> +  FoldingSet(FoldingSet &&Arg) : FoldingSetImpl(std::move(Arg)) {}
>> +  FoldingSet &operator=(FoldingSet &&RHS) {
>> +    (void)FoldingSetImpl::operator=(std::move(RHS));
>> +    return *this;
>> +  }
>>
>>    typedef FoldingSetIterator<T> iterator;
>>    iterator begin() { return iterator(Buckets); }
>>
>> Modified: llvm/trunk/lib/Support/FoldingSet.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/FoldingSet.cpp?rev=245192&r1=245191&r2=245192&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Support/FoldingSet.cpp (original)
>> +++ llvm/trunk/lib/Support/FoldingSet.cpp Sun Aug 16 18:17:27 2015
>> @@ -232,9 +232,29 @@ FoldingSetImpl::FoldingSetImpl(unsigned
>>    Buckets = AllocateBuckets(NumBuckets);
>>    NumNodes = 0;
>>  }
>> +
>> +FoldingSetImpl::FoldingSetImpl(FoldingSetImpl &&Arg)
>> +    : Buckets(Arg.Buckets), NumBuckets(Arg.NumBuckets),
>> NumNodes(Arg.NumNodes) {
>> +  Arg.Buckets = nullptr;
>> +  Arg.NumBuckets = 0;
>> +  Arg.NumNodes = 0;
>> +}
>> +
>> +FoldingSetImpl &FoldingSetImpl::operator=(FoldingSetImpl &&RHS) {
>> +  free(Buckets); // This may be null if the set is in a moved-from state.
>> +  Buckets = RHS.Buckets;
>> +  NumBuckets = RHS.NumBuckets;
>> +  NumNodes = RHS.NumNodes;
>> +  RHS.Buckets = nullptr;
>> +  RHS.NumBuckets = 0;
>> +  RHS.NumNodes = 0;
>> +  return *this;
>> +}
>> +
>>  FoldingSetImpl::~FoldingSetImpl() {
>>    free(Buckets);
>>  }
>> +
>>  void FoldingSetImpl::clear() {
>>    // Set all but the last bucket to null pointers.
>>    memset(Buckets, 0, NumBuckets*sizeof(void*));
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150816/f9d4d093/attachment.html>