On Wed, May 24, 2017 at 8:07 PM, Andrew Pinski <pins...@gmail.com> wrote: > 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.
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 > > >> >>