| Message ID | 20160224163122.GR14947@redhat.com |
|---|---|
| State | New |
| Headers | show |
Ping. On Wed, Feb 24, 2016 at 05:31:22PM +0100, Marek Polacek wrote: > The following is another issue with macros from system headers. In this case > bool is defined in a system header to expand to _Bool and the "is promoted to" > warning didn't trigger because of that. The fix is to use the expanded location. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2016-02-24 Marek Polacek <polacek@redhat.com> > > PR c/67854 > * gimplify.c (gimplify_va_arg_expr): Use expanded location for the > "is promoted to" warning. > > * gcc.dg/pr67854.c: New test. > > diff --git gcc/gimplify.c gcc/gimplify.c > index 7be6bd7..e7ea974 100644 > --- gcc/gimplify.c > +++ gcc/gimplify.c > @@ -11573,24 +11573,28 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p, > { > static bool gave_help; > bool warned; > + /* Use the expansion point to handle cases such as passing bool (defined > + in a system header) through `...'. */ > + source_location xloc > + = expansion_point_location_if_in_system_header (loc); > > /* Unfortunately, this is merely undefined, rather than a constraint > violation, so we cannot make this an error. If this call is never > executed, the program is still strictly conforming. */ > - warned = warning_at (loc, 0, > - "%qT is promoted to %qT when passed through %<...%>", > + warned = warning_at (xloc, 0, > + "%qT is promoted to %qT when passed through %<...%>", > type, promoted_type); > if (!gave_help && warned) > { > gave_help = true; > - inform (loc, "(so you should pass %qT not %qT to %<va_arg%>)", > + inform (xloc, "(so you should pass %qT not %qT to %<va_arg%>)", > promoted_type, type); > } > > /* We can, however, treat "undefined" any way we please. > Call abort to encourage the user to fix the program. */ > if (warned) > - inform (loc, "if this code is reached, the program will abort"); > + inform (xloc, "if this code is reached, the program will abort"); > /* Before the abort, allow the evaluation of the va_list > expression to exit or longjmp. */ > gimplify_and_add (valist, pre_p); > diff --git gcc/testsuite/gcc.dg/pr67854.c gcc/testsuite/gcc.dg/pr67854.c > index e69de29..af994c6 100644 > --- gcc/testsuite/gcc.dg/pr67854.c > +++ gcc/testsuite/gcc.dg/pr67854.c > @@ -0,0 +1,11 @@ > +/* PR c/67854 */ > +/* { dg-do compile } */ > + > +#include <stdbool.h> > +#include <stdarg.h> > + > +void > +foo (va_list ap) > +{ > + va_arg (ap, bool); /* { dg-warning "is promoted to .int. when passed through" } */ > +} > > Marek Marek
On 02/24/2016 09:31 AM, Marek Polacek wrote: > The following is another issue with macros from system headers. In this case > bool is defined in a system header to expand to _Bool and the "is promoted to" > warning didn't trigger because of that. The fix is to use the expanded location. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2016-02-24 Marek Polacek <polacek@redhat.com> > > PR c/67854 > * gimplify.c (gimplify_va_arg_expr): Use expanded location for the > "is promoted to" warning. > > * gcc.dg/pr67854.c: New test. This isn't a regression, so I put it in my gcc-7 bucket. Is this something we really need to address in gcc-6? FWIW, I think the patch is technically fine. Jeff
On Tue, Mar 01, 2016 at 09:31:26AM -0700, Jeff Law wrote: > On 02/24/2016 09:31 AM, Marek Polacek wrote: > >The following is another issue with macros from system headers. In this case > >bool is defined in a system header to expand to _Bool and the "is promoted to" > >warning didn't trigger because of that. The fix is to use the expanded location. > > > >Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > >2016-02-24 Marek Polacek <polacek@redhat.com> > > > > PR c/67854 > > * gimplify.c (gimplify_va_arg_expr): Use expanded location for the > > "is promoted to" warning. > > > > * gcc.dg/pr67854.c: New test. > This isn't a regression, so I put it in my gcc-7 bucket. Is this something > we really need to address in gcc-6? Well, gcc4.7 warned, the warning disappeared in gcc4.8, so I think this is a regression. I'm fine with deferring to 7 though. > FWIW, I think the patch is technically fine. Thanks, Marek
On Tue, Mar 01, 2016 at 08:08:03PM +0100, Marek Polacek wrote: > On Tue, Mar 01, 2016 at 09:31:26AM -0700, Jeff Law wrote: > > On 02/24/2016 09:31 AM, Marek Polacek wrote: > > >The following is another issue with macros from system headers. In this case > > >bool is defined in a system header to expand to _Bool and the "is promoted to" > > >warning didn't trigger because of that. The fix is to use the expanded location. > > > > > >Bootstrapped/regtested on x86_64-linux, ok for trunk? > > > > > >2016-02-24 Marek Polacek <polacek@redhat.com> > > > > > > PR c/67854 > > > * gimplify.c (gimplify_va_arg_expr): Use expanded location for the > > > "is promoted to" warning. > > > > > > * gcc.dg/pr67854.c: New test. > > This isn't a regression, so I put it in my gcc-7 bucket. Is this something > > we really need to address in gcc-6? > > Well, gcc4.7 warned, the warning disappeared in gcc4.8, so I think this is > a regression. I'm fine with deferring to 7 though. > > > FWIW, I think the patch is technically fine. > > Thanks, This is ok for trunk. Jakub
diff --git gcc/gimplify.c gcc/gimplify.c index 7be6bd7..e7ea974 100644 --- gcc/gimplify.c +++ gcc/gimplify.c @@ -11573,24 +11573,28 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p, { static bool gave_help; bool warned; + /* Use the expansion point to handle cases such as passing bool (defined + in a system header) through `...'. */ + source_location xloc + = expansion_point_location_if_in_system_header (loc); /* Unfortunately, this is merely undefined, rather than a constraint violation, so we cannot make this an error. If this call is never executed, the program is still strictly conforming. */ - warned = warning_at (loc, 0, - "%qT is promoted to %qT when passed through %<...%>", + warned = warning_at (xloc, 0, + "%qT is promoted to %qT when passed through %<...%>", type, promoted_type); if (!gave_help && warned) { gave_help = true; - inform (loc, "(so you should pass %qT not %qT to %<va_arg%>)", + inform (xloc, "(so you should pass %qT not %qT to %<va_arg%>)", promoted_type, type); } /* We can, however, treat "undefined" any way we please. Call abort to encourage the user to fix the program. */ if (warned) - inform (loc, "if this code is reached, the program will abort"); + inform (xloc, "if this code is reached, the program will abort"); /* Before the abort, allow the evaluation of the va_list expression to exit or longjmp. */ gimplify_and_add (valist, pre_p); diff --git gcc/testsuite/gcc.dg/pr67854.c gcc/testsuite/gcc.dg/pr67854.c index e69de29..af994c6 100644 --- gcc/testsuite/gcc.dg/pr67854.c +++ gcc/testsuite/gcc.dg/pr67854.c @@ -0,0 +1,11 @@ +/* PR c/67854 */ +/* { dg-do compile } */ + +#include <stdbool.h> +#include <stdarg.h> + +void +foo (va_list ap) +{ + va_arg (ap, bool); /* { dg-warning "is promoted to .int. when passed through" } */ +}