[PATCH] PR c++/80544 strip cv-quals from cast results

[Andrew Pinski]

On Wed, May 24, 2017 at 8:07 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> On Wed, May 24, 2017 at 12:29 PM, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On 24/05/17 14:50 -0400, Jason Merrill wrote:
>>>
>>> On Wed, May 24, 2017 at 10:20 AM, Jonathan Wakely <jwakely@redhat.com>
>>> wrote:
>>>>
>>>> On 23/05/17 16:26 -0400, Jason Merrill wrote:
>>>>>
>>>>>
>>>>> On Tue, May 23, 2017 at 2:00 PM, Jonathan Wakely <jwakely@redhat.com>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On 19/05/17 15:14 -0400, Jason Merrill wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely <jwakely@redhat.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> I also tried to add a warning like EDG's (see the PR) but it gave a
>>>>>>>> false positive for direct-list-init of scoped enums (P0138R2,
>>>>>>>> r240449)
>>>>>>>> because that code goes through build_c_cast to perform the
>>>>>>>> conversion,
>>>>>>>> so looks like a cast to my new warning.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The enum init code could strip the cv-quals when calling build_c_cast
>>>>>>> to avoid the warning.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks, that works. I don't think this warning fits under any existing
>>>>>> option, so I'll add a new one. -Wuseless-cast-qual maybe? Or is that
>>>>>> too close to -Wuseless-cast and -Wcast-qual and would cause confusion?
>>>>>
>>>>>
>>>>>
>>>>> -Wcast-rvalue-qual ?
>>>>
>>>>
>>>>
>>>> I realised that -Wignored-qualifiers is a good fit. That warns about
>>>> ignored cv-qualifiers on return types, which is a very similar case.
>>>>
>>>> This patch adds a new function to conditionally warn and calls that
>>>> from the build_*_cast functions after they produce a valid result. I
>>>> originally put the warnings next to the cv_unqualified calls that
>>>> strip the quals, but was getting duplicate warnings when build_cp_cast
>>>> calls more than one of the build_*_cast_1 functions.
>>>>
>>>> This also removes the unnecessary check for reference types that
>>>> Nathan pointed out.
>>>>
>>>> Tested x86_64-linux and powerpc64-linux. OK for trunk?
>>>
>>>
>>>> +/*
>>>> +  Warns if the cast ignores cv-qualifiers on TYPE.
>>>> + */
>>>
>>>
>>> The GCC sources don't put /* and */ on their own line.  OK with that
>>> change, thanks!
>>
>>
>> OK, I'll also fix it on the maybe_warn_about_useless_cast function
>> just above, which I copied :-)
>
> This change caused a bootstrap failure on aarch64-linux-gnu and
> x86_64-linux-gnu:
> In file included from ../../gcc/gcc/system.h:691:0,
>                  from ../../gcc/gcc/read-rtl.c:31:
> ../../gcc/gcc/read-rtl.c: In member function ‘const char*
> md_reader::apply_iterator_to_string(const char*)’:
> ../../gcc/gcc/../include/libiberty.h:722:38: error: type qualifiers
> ignored on cast result type [-Werror=ignored-qualifiers]
>  # define alloca(x) __builtin_alloca(x)
>                                       ^
> ../../gcc/gcc/../include/libiberty.h:727:47: note: in expansion of
> macro ‘alloca’
>     char *const libiberty_nptr = (char *const) alloca (libiberty_len); \
>                                                ^~~~~~
> ../../gcc/gcc/read-rtl.c:380:21: note: in expansion of macro ‘ASTRDUP’
>    base = p = copy = ASTRDUP (string);
>                      ^~~~~~~
>
> I know you did not touch libiberty.h but that is emitting an error.
> Did you test your patch with a full bootstrap?  I thought that was
> recorded as being required now for C++ patches; I know a few years
> back when the GCC was not compiling as C++, it was not required.

Oh it looks like it was already fixed by revision 248442.  Just my
build automated build was not done for that timeframe.

Thanks,
Andrew

>
> Thanks,
> Andrew
>
>
>>
>>