On Wed, May 24, 2017 at 12:29 PM, Jonathan Wakely <jwak...@redhat.com> wrote:
> On 24/05/17 14:50 -0400, Jason Merrill wrote:
>>
>> On Wed, May 24, 2017 at 10:20 AM, Jonathan Wakely <jwak...@redhat.com>
>> wrote:
>>>
>>> On 23/05/17 16:26 -0400, Jason Merrill wrote:
>>>>
>>>>
>>>> On Tue, May 23, 2017 at 2:00 PM, Jonathan Wakely <jwak...@redhat.com>
>>>> wrote:
>>>>>
>>>>>
>>>>> On 19/05/17 15:14 -0400, Jason Merrill wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Thu, Apr 27, 2017 at 12:59 PM, Jonathan Wakely <jwak...@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.
Thanks,
Andrew
>
>
