On 21 March 2017 at 08:48, Jakub Jelinek <ja...@redhat.com> wrote:
> Formatting etc. nits:
>
>> 2017-03-21  Ville Voutilainen  <ville.voutilai...@gmail.com>
>>
>>     gcc/
>>
>>     PR c++/35878
>
> This should go into gcc/cp/ ChangeLog
>
>>     * cp/init.c (std_placement_new_fn_p): New.
>
> without cp/ here.

Yeah, so modified before the commit.

>>    nothrow = TYPE_NOTHROW_P (TREE_TYPE (alloc_fn));
>> -  check_new = (flag_check_new || nothrow);
>> +  check_new = flag_check_new
>> +    || (nothrow && !std_placement_new_fn_p (alloc_fn));
>
> This should be either:
>   check_new
>     = flag_check_new || (nothrow && !std_placement_new_fn_p (alloc_fn));
> or
>   check_new = (flag_check_new
>                || (nothrow && !std_placement_new_fn_p (alloc_fn)));
> i.e. || should not be when on next line at different column than
> flag_check_new (and the ()s to make emacs happy).

Bummer - I simply indented that with emacs and thought it does the right thing.

>
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/init/pr35878_1.C
>> @@ -0,0 +1,21 @@
>> +// { dg-options "-O2 --std=gnu++11" }
>
> -O2 -std=gnu++11 is enough, no need for double dash --std=gnu++11.
>
>> +// { dg-do compile }
>> +// { dg-final { scan-assembler "test.*%rdi, %rdi" { target i?86-*-* 
>> x86_64-*-* } } }
>
> This will surely fail on 32-bit or with -mx32.  So either you need to use it
> on { target { { i?86-*-* x86_64-*-* } && lp64 } } only, or perhaps instead
> of scanning assembler add -fdump-tree-optimized to dg-options and
> scan-tree-dump for the NULL? pointer comparison there.

I weakly vote for the former, since that's less intrusive. I managed
to commit the patch before
I saw these remarks, so we need to patch that fix in as a separate tweak.

Reply via email to