| Message ID | b393e0e5-81e8-e3ec-25b2-84f239f95c97@acm.org |
|---|---|
| State | New |
| Headers | show |
On May 25, 2017 3:22:18 AM GMT+02:00, Nathan Sidwell <nathan@acm.org> wrote: >On 05/24/2017 09:13 PM, Nathan Sidwell wrote: >> On 05/24/2017 08:56 PM, Nathan Sidwell wrote: >>> On 05/24/2017 08:34 PM, Nathan Sidwell wrote: >>>> We now warn on casts to T const. Applied as obvious to fix >bootstrap. >>> >>> And this fixes c-common.c >> >> And fix auto-profile.c > >and lto-streamer-out.c, sigh What's the reason to warn here?! Richard.
On 05/25/2017 01:29 AM, Richard Biener wrote: > On May 25, 2017 3:22:18 AM GMT+02:00, Nathan Sidwell <nathan@acm.org> wrote: >> On 05/24/2017 09:13 PM, Nathan Sidwell wrote: >>> On 05/24/2017 08:56 PM, Nathan Sidwell wrote: >>>> On 05/24/2017 08:34 PM, Nathan Sidwell wrote: >>>>> We now warn on casts to T const. Applied as obvious to fix >> bootstrap. >>>> >>>> And this fixes c-common.c >>> >>> And fix auto-profile.c >> >> and lto-streamer-out.c, sigh > > What's the reason to warn here?! It's a new warning about trying to cast to a const T because the const is ignored. It might be better if the warning only triggered on trying to cast 'T' to 'const T' and not trigger casting 'U' to 'const T'? I dunno. nathan
On 25/05/17 06:54 -0400, Nathan Sidwell wrote: >On 05/25/2017 01:29 AM, Richard Biener wrote: >>On May 25, 2017 3:22:18 AM GMT+02:00, Nathan Sidwell <nathan@acm.org> wrote: >>>On 05/24/2017 09:13 PM, Nathan Sidwell wrote: >>>>On 05/24/2017 08:56 PM, Nathan Sidwell wrote: >>>>>On 05/24/2017 08:34 PM, Nathan Sidwell wrote: >>>>>>We now warn on casts to T const. Applied as obvious to fix >>>bootstrap. >>>>> >>>>>And this fixes c-common.c >>>> >>>>And fix auto-profile.c >>> >>>and lto-streamer-out.c, sigh >> >>What's the reason to warn here?! > >It's a new warning about trying to cast to a const T because the const >is ignored. > >It might be better if the warning only triggered on trying to cast 'T' >to 'const T' and not trigger casting 'U' to 'const T'? I dunno. Maybe, although the language is clear that casting to (const T) means exactly the same as casting to (T) when T is a scalar type. What benefit is there to saying (const T) if the compiler ignores the const? (which the standard says it should, and I implemented in r248432).
On 25/05/17 12:19 +0100, Jonathan Wakely wrote: >On 25/05/17 06:54 -0400, Nathan Sidwell wrote: >>On 05/25/2017 01:29 AM, Richard Biener wrote: >>>On May 25, 2017 3:22:18 AM GMT+02:00, Nathan Sidwell <nathan@acm.org> wrote: >>>>On 05/24/2017 09:13 PM, Nathan Sidwell wrote: >>>>>On 05/24/2017 08:56 PM, Nathan Sidwell wrote: >>>>>>On 05/24/2017 08:34 PM, Nathan Sidwell wrote: >>>>>>>We now warn on casts to T const. Applied as obvious to fix >>>>bootstrap. >>>>>> >>>>>>And this fixes c-common.c >>>>> >>>>>And fix auto-profile.c >>>> >>>>and lto-streamer-out.c, sigh >>> >>>What's the reason to warn here?! >> >>It's a new warning about trying to cast to a const T because the >>const is ignored. >> >>It might be better if the warning only triggered on trying to cast >>'T' to 'const T' and not trigger casting 'U' to 'const T'? I dunno. > >Maybe, although the language is clear that casting to (const T) means >exactly the same as casting to (T) when T is a scalar type. What >benefit is there to saying (const T) if the compiler ignores the >const? (which the standard says it should, and I implemented in >r248432). I don't mind removing the warning again if preferred. I thought it was useful (as we already warn for ignored const in return types). All I really care about is that the compiler ignores the const, if it does that without warning that's OK.
On 05/25/2017 07:21 AM, Jonathan Wakely wrote: > I don't mind removing the warning again if preferred. I thought it was > useful (as we already warn for ignored const in return types). Oh yeah, I recall noticing we did that (and noting we didn't warn elsewhere). This new warning seems consistent. I say leave it in unless the grumbling gets too much for you :) nathan
On May 25, 2017 1:38:36 PM GMT+02:00, Nathan Sidwell <nathan@acm.org> wrote: >On 05/25/2017 07:21 AM, Jonathan Wakely wrote: > >> I don't mind removing the warning again if preferred. I thought it >was >> useful (as we already warn for ignored const in return types). > >Oh yeah, I recall noticing we did that (and noting we didn't warn >elsewhere). This new warning seems consistent. > >I say leave it in unless the grumbling gets too much for you :) I wonder if we can somehow default to -Wno-error=xyz for such kind of 'style' warnings... Adding const can't possibly break anything or result in wrong expectations, can it? Richard. >nathan
On 25/05/17 14:35 +0200, Richard Biener wrote: >On May 25, 2017 1:38:36 PM GMT+02:00, Nathan Sidwell <nathan@acm.org> wrote: >>On 05/25/2017 07:21 AM, Jonathan Wakely wrote: >> >>> I don't mind removing the warning again if preferred. I thought it >>was >>> useful (as we already warn for ignored const in return types). >> >>Oh yeah, I recall noticing we did that (and noting we didn't warn >>elsewhere). This new warning seems consistent. >> >>I say leave it in unless the grumbling gets too much for you :) > >I wonder if we can somehow default to -Wno-error=xyz for such kind of 'style' warnings... Adding const can't possibly break anything or result in wrong expectations, can it? Now that G++ correctly ignores the const it can't change (or break) anything to add const in the cast. Before I fixed PR 80544, the presence/absence of the const affected the generated code and could result in e.g. different overloaded functions being called (in some fairly obscure cases). The original report I got was http://ideone.com/JSFEZ3 and GCC was giving different behaviour to all other C++ compilers (which ignored the const and so failed the static assertion). The warning is just saying "hey, you know what you wrote is going to be ignored, right?" That's a bit like "statement has no effect" warnings, although those warnings are usually because you mistyped something and so find real bugs.
2017-05-24 Nathan Sidwell <nathan@acm.org> * lto-streamer-in.c (lto_input_data_block): Adjust T const cast to avoid warning. Index: lto-streamer-in.c =================================================================== --- lto-streamer-in.c (revision 248441) +++ lto-streamer-in.c (working copy) @@ -86,7 +86,7 @@ void lto_input_data_block (struct lto_input_block *ib, void *addr, size_t length) { size_t i; - unsigned char *const buffer = (unsigned char *const) addr; + unsigned char *const buffer = (unsigned char *) addr; for (i = 0; i < length; i++) buffer[i] = streamer_read_uchar (ib);