This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Fix PR52822 (stable_partition move-assigns object to itself) in trunk, 4.7, and 4.6


On Wed, Apr 4, 2012 at 11:09 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
>> The attached patches fix http://gcc.gnu.org/PR52822, and have been
>> tested with `make check-c++` on linux-x86_64. The trunk patch applies
>> and tests cleanly on gcc-4_7-branch. The gcc-4_6-branch patch is
>> significantly simpler, as Paolo suggested on the bug.
>
> A few small issues.
>
> For the 4.6 version of the patch, you want to use std::__addressof, instead
> of simply &, which may be overloaded.
> And by the way what's wrong with just comparing the iterators?

Nothing's wrong with comparing the iterators. Switched.

> For the mainline/4.7 version, you want to cast __pred to bool:
> !bool(__pred(*__first)). Also, isn't clear to me why you have __local_len in
> __find_if_not_n instead of just working on __len (and in any case please
> prefer just __local_len as condition instead of the redundant __local_len !=
> 0; likewise in many other similar situations).

I was micro-optimizing, since the compiler might think __len is
modified in __pred if it doesn't inline enough. I've removed
__local_len, casted pred results to bool, and avoided !=0.

> Also, very minor nit, you will be touching the libstdc++-v3 Changelog thus
> no libstdc++-v3 prefixes in the ChangeLog entry.

Yep, done.

Attachment: pr52822_gcc46.patch
Description: Binary data

Attachment: pr52822_trunk.patch
Description: Binary data


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]