Re: r254423 - It appears that this horrible mutating copy constructor is unused. Kill it with fire.

[Aaron Ballman via cfe-commits]

On Tue, Dec 1, 2015 at 6:47 PM, Richard Smith  wrote:
> On Tue, Dec 1, 2015 at 3:42 PM, Duncan P. N. Exon Smith via cfe-commits
>  wrote:
>>
>>
>> > On 2015-Dec-01, at 09:15, Aaron Ballman via cfe-commits
>> >  wrote:
>> >
>> > Author: aaronballman
>> > Date: Tue Dec  1 11:15:13 2015
>> > New Revision: 254423
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=254423=rev
>> > Log:
>> > It appears that this horrible mutating copy constructor is unused. Kill
>> > it with fire.
>> >
>> > Modified:
>> >cfe/trunk/include/clang/Sema/AttributeList.h
>> >
>> > Modified: cfe/trunk/include/clang/Sema/AttributeList.h
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=254423=254422=254423=diff
>> >
>> > ==
>> > --- cfe/trunk/include/clang/Sema/AttributeList.h (original)
>> > +++ cfe/trunk/include/clang/Sema/AttributeList.h Tue Dec  1 11:15:13
>> > 2015
>> > @@ -557,11 +557,6 @@ public:
>> >   /// Create a new pool for a factory.
>> >   AttributePool(AttributeFactory ) : Factory(factory),
>> > Head(nullptr) {}
>> >
>> > -  /// Move the given pool's allocations to this pool.
>> > -  AttributePool(AttributePool ) : Factory(pool.Factory),
>> > Head(pool.Head) {
>> > -pool.Head = nullptr;
>> > -  }
>> > -
>>
>> Moving without R-value references is weird, but it seems to me like
>> at least this was preventing double-ownership of the Head.
>>
>> I'm pretty sure now that this function has been removed, the copy
>> constructor will be auto-generated.  Should you `= delete` it?
>
>
> Or, preferably, reinstate it but make it a move constructor.

I originally replaced it with a move constructor, but felt that since
it was unused, no constructor was better. However, Duncan's point
still stands that the constructor would be auto-generated anyway, and
someone may accidentally copy or move this, so it should do the right
thing. I've fixed in r254515, thank you for the review!

~Aaron
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254423 - It appears that this horrible mutating copy constructor is unused. Kill it with fire.

[Richard Smith via cfe-commits]

On Tue, Dec 1, 2015 at 3:42 PM, Duncan P. N. Exon Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> > On 2015-Dec-01, at 09:15, Aaron Ballman via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> >
> > Author: aaronballman
> > Date: Tue Dec  1 11:15:13 2015
> > New Revision: 254423
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=254423=rev
> > Log:
> > It appears that this horrible mutating copy constructor is unused. Kill
> it with fire.
> >
> > Modified:
> >cfe/trunk/include/clang/Sema/AttributeList.h
> >
> > Modified: cfe/trunk/include/clang/Sema/AttributeList.h
> > URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=254423=254422=254423=diff
> >
> ==
> > --- cfe/trunk/include/clang/Sema/AttributeList.h (original)
> > +++ cfe/trunk/include/clang/Sema/AttributeList.h Tue Dec  1 11:15:13 2015
> > @@ -557,11 +557,6 @@ public:
> >   /// Create a new pool for a factory.
> >   AttributePool(AttributeFactory ) : Factory(factory),
> Head(nullptr) {}
> >
> > -  /// Move the given pool's allocations to this pool.
> > -  AttributePool(AttributePool ) : Factory(pool.Factory),
> Head(pool.Head) {
> > -pool.Head = nullptr;
> > -  }
> > -
>
> Moving without R-value references is weird, but it seems to me like
> at least this was preventing double-ownership of the Head.
>
> I'm pretty sure now that this function has been removed, the copy
> constructor will be auto-generated.  Should you `= delete` it?


Or, preferably, reinstate it but make it a move constructor.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r254423 - It appears that this horrible mutating copy constructor is unused. Kill it with fire.

[Duncan P. N. Exon Smith via cfe-commits]

> On 2015-Dec-01, at 09:15, Aaron Ballman via cfe-commits 
>  wrote:
> 
> Author: aaronballman
> Date: Tue Dec  1 11:15:13 2015
> New Revision: 254423
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=254423=rev
> Log:
> It appears that this horrible mutating copy constructor is unused. Kill it 
> with fire.
> 
> Modified:
>cfe/trunk/include/clang/Sema/AttributeList.h
> 
> Modified: cfe/trunk/include/clang/Sema/AttributeList.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=254423=254422=254423=diff
> ==
> --- cfe/trunk/include/clang/Sema/AttributeList.h (original)
> +++ cfe/trunk/include/clang/Sema/AttributeList.h Tue Dec  1 11:15:13 2015
> @@ -557,11 +557,6 @@ public:
>   /// Create a new pool for a factory.
>   AttributePool(AttributeFactory ) : Factory(factory), Head(nullptr) 
> {}
> 
> -  /// Move the given pool's allocations to this pool.
> -  AttributePool(AttributePool ) : Factory(pool.Factory), 
> Head(pool.Head) {
> -pool.Head = nullptr;
> -  }
> -

Moving without R-value references is weird, but it seems to me like
at least this was preventing double-ownership of the Head.

I'm pretty sure now that this function has been removed, the copy
constructor will be auto-generated.  Should you `= delete` it?

>   AttributeFactory () const { return Factory; }
> 
>   void clear() {
> 
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r254423 - It appears that this horrible mutating copy constructor is unused. Kill it with fire.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Dec  1 11:15:13 2015
New Revision: 254423

URL: http://llvm.org/viewvc/llvm-project?rev=254423=rev
Log:
It appears that this horrible mutating copy constructor is unused. Kill it with 
fire.

Modified:
cfe/trunk/include/clang/Sema/AttributeList.h

Modified: cfe/trunk/include/clang/Sema/AttributeList.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=254423=254422=254423=diff
==
--- cfe/trunk/include/clang/Sema/AttributeList.h (original)
+++ cfe/trunk/include/clang/Sema/AttributeList.h Tue Dec  1 11:15:13 2015
@@ -557,11 +557,6 @@ public:
   /// Create a new pool for a factory.
   AttributePool(AttributeFactory ) : Factory(factory), Head(nullptr) {}
 
-  /// Move the given pool's allocations to this pool.
-  AttributePool(AttributePool ) : Factory(pool.Factory), Head(pool.Head) {
-pool.Head = nullptr;
-  }
-
   AttributeFactory () const { return Factory; }
 
   void clear() {


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits