| Message ID | 20190823041740.7186-1-alexhenrie24@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [resend,1/2] PR c/65403 - Ignore -Wno-error=<some-future-warning> | expand |
Hello. I have couple of notes about the patch: On 8/23/19 6:17 AM, Alex Henrie wrote: > From: Manuel López-Ibáñez <manu@gcc.gnu.org> > > * opts-common.c (ignored_wnoerror_options): New global variable. > * opts-global.c (print_ignored_options): Ignore > -Wno-error=<some-future-warning> except if there are other > diagnostics. > * opts.c (enable_warning_as_error): Record ignored -Wno-error > options. > * opts.h (ignored_wnoerror_options): Declare. > * gcc.dg/Werror-13.c: Don't expect hints for > -Wno-error=<some-future-warning>. > --- > gcc/opts-common.c | 2 ++ > gcc/opts-global.c | 10 +++++++--- > gcc/opts.c | 21 +++++++++++++-------- > gcc/opts.h | 2 ++ > gcc/testsuite/gcc.dg/Werror-13.c | 2 +- > 5 files changed, 25 insertions(+), 12 deletions(-) > > diff --git a/gcc/opts-common.c b/gcc/opts-common.c > index e2a315ba229..86e518bdd2b 100644 > --- a/gcc/opts-common.c > +++ b/gcc/opts-common.c > @@ -26,6 +26,8 @@ along with GCC; see the file COPYING3. If not see > #include "diagnostic.h" > #include "spellcheck.h" > > +vec<const char *> ignored_wnoerror_options; Here you'll need to add a comment. > + > static void prune_options (struct cl_decoded_option **, unsigned int *); > > /* An option that is undocumented, that takes a joined argument, and > diff --git a/gcc/opts-global.c b/gcc/opts-global.c > index 7c5bd16c7ea..b4b4576e450 100644 > --- a/gcc/opts-global.c > +++ b/gcc/opts-global.c > @@ -136,12 +136,16 @@ print_ignored_options (void) > { > while (!ignored_options.is_empty ()) > { > - const char *opt; > - > - opt = ignored_options.pop (); > + const char * opt = ignored_options.pop (); > warning_at (UNKNOWN_LOCATION, 0, > "unrecognized command-line option %qs", opt); > } > + while (!ignored_wnoerror_options.is_empty ()) > + { > + const char * opt = ignored_wnoerror_options.pop (); No space between '*' and 'opt' please. > + warning_at (UNKNOWN_LOCATION, 0, > + "%<-Wno-error=%s%>: no option %<-W%s%>", opt, opt); > + } > } > > /* Handle an unknown option DECODED, returning true if an error should > diff --git a/gcc/opts.c b/gcc/opts.c > index bb0d8b5e7db..a78e5cf1949 100644 > --- a/gcc/opts.c > +++ b/gcc/opts.c > @@ -3175,15 +3175,20 @@ enable_warning_as_error (const char *arg, int value, unsigned int lang_mask, > option_index = find_opt (new_option, lang_mask); > if (option_index == OPT_SPECIAL_unknown) > { > - option_proposer op; > - const char *hint = op.suggest_option (new_option); > - if (hint) > - error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>;" > - " did you mean %<-%s%>?", value ? "" : "no-", > - arg, new_option, hint); > + if (value) > + { > + option_proposer op; > + const char *hint = op.suggest_option (new_option); > + if (hint) > + error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>;" > + " did you mean %<-%s%>?", value ? "" : "no-", > + arg, new_option, hint); > + else > + error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>", > + value ? "" : "no-", arg, new_option); > + } > else > - error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>", > - value ? "" : "no-", arg, new_option); > + ignored_wnoerror_options.safe_push (arg); You don't want to append and option that is already in the ignored_wnoerror_options: $ ./xgcc -B. -Wunused-variable -Werror -Wno-error=some-future-warning -Wno-error=some-future-warning /tmp/main2.c /tmp/main2.c: In function ‘main’: /tmp/main2.c:3:7: error: unused variable ‘foo’ [-Werror=unused-variable] 3 | int foo; /* { dg-error "unused variable 'foo'" } */ | ^~~ /tmp/main2.c: At top level: cc1: error: ‘-Wno-error=some-future-warning’: no option ‘-Wsome-future-warning’ [-Werror] cc1: error: ‘-Wno-error=some-future-warning’: no option ‘-Wsome-future-warning’ [-Werror] cc1: all warnings being treated as errors As seen the error is there twice. > } > else if (!(cl_options[option_index].flags & CL_WARNING)) > error_at (loc, "%<-Werror=%s%>: %<-%s%> is not an option that " > diff --git a/gcc/opts.h b/gcc/opts.h > index 47223229388..ac281aef540 100644 > --- a/gcc/opts.h > +++ b/gcc/opts.h > @@ -461,4 +461,6 @@ extern bool parse_and_check_align_values (const char *flag, > bool report_error, > location_t loc); > > +extern vec<const char *> ignored_wnoerror_options; > + > #endif > diff --git a/gcc/testsuite/gcc.dg/Werror-13.c b/gcc/testsuite/gcc.dg/Werror-13.c > index 3a02b7ea2b5..7c2bf6836ed 100644 > --- a/gcc/testsuite/gcc.dg/Werror-13.c > +++ b/gcc/testsuite/gcc.dg/Werror-13.c > @@ -5,6 +5,6 @@ > /* { dg-error "'-Werror' is not an option that controls warnings" "" { target *-*-* } 0 } */ > /* { dg-error "'-Wfatal-errors' is not an option that controls warnings" "" { target *-*-* } 0 } */ > /* { dg-error "'-Werror=vla2': no option '-Wvla2'; did you mean '-Wvla." "" { target *-*-* } 0 } */ > -/* { dg-error "'-Wno-error=misleading-indentation2': no option '-Wmisleading-indentation2'; did you mean '-Wmisleading-indentation'" "" { target *-*-* } 0 } */ > +/* { dg-warning "'-Wno-error=misleading-indentation2': no option '-Wmisleading-indentation2'" "" { target *-*-* } 0 } */ > > int i; > One question about the behavior: Why do I need to have another warning to get the warning: ‘-Wno-error=some-future-warning’ printed? Example: $ ./xgcc -B. -Wno-error=some-future-warning /tmp/main2.c [no output] $ ./xgcc -B. -Wno-error=some-future-warning /tmp/main2.c -Wunused-variable /tmp/main2.c: In function ‘main’: /tmp/main2.c:3:7: warning: unused variable ‘foo’ [-Wunused-variable] 3 | int foo; /* { dg-error "unused variable 'foo'" } */ | ^~~ /tmp/main2.c: At top level: cc1: warning: ‘-Wno-error=some-future-warning’: no option ‘-Wsome-future-warning’ ? It seems illogical to me. Thanks, Martin
On Fri, Aug 23, 2019 at 3:13 AM Martin Liška <mliska@suse.cz> wrote: > On 8/23/19 6:17 AM, Alex Henrie wrote: > > +vec<const char *> ignored_wnoerror_options; > > Here you'll need to add a comment. The declaration of ignored_options in opts-global.c doesn't have a comment either. What would you like the comment to say? > > + const char * opt = ignored_wnoerror_options.pop (); > > No space between '*' and 'opt' please. Okay. > You don't want to append and option that is already in the ignored_wnoerror_options: > > $ > ./xgcc -B. -Wunused-variable -Werror -Wno-error=some-future-warning -Wno-error=some-future-warning /tmp/main2.c > /tmp/main2.c: In function ‘main’: > /tmp/main2.c:3:7: error: unused variable ‘foo’ [-Werror=unused-variable] > 3 | int foo; /* { dg-error "unused variable 'foo'" } */ > | ^~~ > /tmp/main2.c: At top level: > cc1: error: ‘-Wno-error=some-future-warning’: no option ‘-Wsome-future-warning’ [-Werror] > cc1: error: ‘-Wno-error=some-future-warning’: no option ‘-Wsome-future-warning’ [-Werror] > cc1: all warnings being treated as errors > > As seen the error is there twice. Joseph explicitly asked me to make -Wno-error=some-future-warning behave the same as -Wno-some-future-warning (see <https://gcc.gnu.org/ml/gcc-patches/2019-03/msg00065.html>), and -Wno-some-future-warning prints multiple warnings if the option is given multiple times. > One question about the behavior: > > Why do I need to have another warning to get the warning: ‘-Wno-error=some-future-warning’ printed? If we always give a warning about -Wno-error=some-future-warning then combining that option with -Werror would cause compilation to fail, which we don't want. -Alex
On 8/23/19 6:52 PM, Alex Henrie wrote: > On Fri, Aug 23, 2019 at 3:13 AM Martin Liška <mliska@suse.cz> wrote: >> On 8/23/19 6:17 AM, Alex Henrie wrote: >>> +vec<const char *> ignored_wnoerror_options; >> >> Here you'll need to add a comment. > > The declaration of ignored_options in opts-global.c doesn't have a > comment either. What would you like the comment to say? I would just write a comment for ignored_wnoerror_options > >>> + const char * opt = ignored_wnoerror_options.pop (); >> >> No space between '*' and 'opt' please. > > Okay. > >> You don't want to append and option that is already in the ignored_wnoerror_options: >> >> $ >> ./xgcc -B. -Wunused-variable -Werror -Wno-error=some-future-warning -Wno-error=some-future-warning /tmp/main2.c >> /tmp/main2.c: In function ‘main’: >> /tmp/main2.c:3:7: error: unused variable ‘foo’ [-Werror=unused-variable] >> 3 | int foo; /* { dg-error "unused variable 'foo'" } */ >> | ^~~ >> /tmp/main2.c: At top level: >> cc1: error: ‘-Wno-error=some-future-warning’: no option ‘-Wsome-future-warning’ [-Werror] >> cc1: error: ‘-Wno-error=some-future-warning’: no option ‘-Wsome-future-warning’ [-Werror] >> cc1: all warnings being treated as errors >> >> As seen the error is there twice. > > Joseph explicitly asked me to make -Wno-error=some-future-warning > behave the same as -Wno-some-future-warning (see > <https://gcc.gnu.org/ml/gcc-patches/2019-03/msg00065.html>), and > -Wno-some-future-warning prints multiple warnings if the option is > given multiple times. > >> One question about the behavior: >> >> Why do I need to have another warning to get the warning: ‘-Wno-error=some-future-warning’ printed? > > If we always give a warning about -Wno-error=some-future-warning then > combining that option with -Werror would cause compilation to fail, > which we don't want. Ah, I see. It's not easy to implement that in a reasonable way. Now I understand that. Thanks, Martin > > -Alex >
On 8/26/19 6:24 AM, Martin Liška wrote: > On 8/23/19 6:52 PM, Alex Henrie wrote: >> On Fri, Aug 23, 2019 at 3:13 AM Martin Liška <mliska@suse.cz> wrote: >>> On 8/23/19 6:17 AM, Alex Henrie wrote: >>>> +vec<const char *> ignored_wnoerror_options; >>> >>> Here you'll need to add a comment. >> >> The declaration of ignored_options in opts-global.c doesn't have a >> comment either. What would you like the comment to say? > > I would just write a comment for > ignored_wnoerror_options FWIW, since the vector stores pointers it doesn't own, it might be helpful to consider (and mention in the comment) who owns them. I assume the strings they point to stick around for the lifetime of the vector but that they do isn't obvious from its use, and neither is how they are allocated (does the vector need to be annotated with GTY?) Martin > >> >>>> + const char * opt = ignored_wnoerror_options.pop (); >>> >>> No space between '*' and 'opt' please. >> >> Okay. >> >>> You don't want to append and option that is already in the ignored_wnoerror_options: >>> >>> $ >>> ./xgcc -B. -Wunused-variable -Werror -Wno-error=some-future-warning -Wno-error=some-future-warning /tmp/main2.c >>> /tmp/main2.c: In function ‘main’: >>> /tmp/main2.c:3:7: error: unused variable ‘foo’ [-Werror=unused-variable] >>> 3 | int foo; /* { dg-error "unused variable 'foo'" } */ >>> | ^~~ >>> /tmp/main2.c: At top level: >>> cc1: error: ‘-Wno-error=some-future-warning’: no option ‘-Wsome-future-warning’ [-Werror] >>> cc1: error: ‘-Wno-error=some-future-warning’: no option ‘-Wsome-future-warning’ [-Werror] >>> cc1: all warnings being treated as errors >>> >>> As seen the error is there twice. >> >> Joseph explicitly asked me to make -Wno-error=some-future-warning >> behave the same as -Wno-some-future-warning (see >> <https://gcc.gnu.org/ml/gcc-patches/2019-03/msg00065.html>), and >> -Wno-some-future-warning prints multiple warnings if the option is >> given multiple times. >> >>> One question about the behavior: >>> >>> Why do I need to have another warning to get the warning: ‘-Wno-error=some-future-warning’ printed? >> >> If we always give a warning about -Wno-error=some-future-warning then >> combining that option with -Werror would cause compilation to fail, >> which we don't want. > > Ah, I see. It's not easy to implement that in a reasonable way. Now I understand that. > > Thanks, > Martin > >> >> -Alex >> >
diff --git a/gcc/opts-common.c b/gcc/opts-common.c index e2a315ba229..86e518bdd2b 100644 --- a/gcc/opts-common.c +++ b/gcc/opts-common.c @@ -26,6 +26,8 @@ along with GCC; see the file COPYING3. If not see #include "diagnostic.h" #include "spellcheck.h" +vec<const char *> ignored_wnoerror_options; + static void prune_options (struct cl_decoded_option **, unsigned int *); /* An option that is undocumented, that takes a joined argument, and diff --git a/gcc/opts-global.c b/gcc/opts-global.c index 7c5bd16c7ea..b4b4576e450 100644 --- a/gcc/opts-global.c +++ b/gcc/opts-global.c @@ -136,12 +136,16 @@ print_ignored_options (void) { while (!ignored_options.is_empty ()) { - const char *opt; - - opt = ignored_options.pop (); + const char * opt = ignored_options.pop (); warning_at (UNKNOWN_LOCATION, 0, "unrecognized command-line option %qs", opt); } + while (!ignored_wnoerror_options.is_empty ()) + { + const char * opt = ignored_wnoerror_options.pop (); + warning_at (UNKNOWN_LOCATION, 0, + "%<-Wno-error=%s%>: no option %<-W%s%>", opt, opt); + } } /* Handle an unknown option DECODED, returning true if an error should diff --git a/gcc/opts.c b/gcc/opts.c index bb0d8b5e7db..a78e5cf1949 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -3175,15 +3175,20 @@ enable_warning_as_error (const char *arg, int value, unsigned int lang_mask, option_index = find_opt (new_option, lang_mask); if (option_index == OPT_SPECIAL_unknown) { - option_proposer op; - const char *hint = op.suggest_option (new_option); - if (hint) - error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>;" - " did you mean %<-%s%>?", value ? "" : "no-", - arg, new_option, hint); + if (value) + { + option_proposer op; + const char *hint = op.suggest_option (new_option); + if (hint) + error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>;" + " did you mean %<-%s%>?", value ? "" : "no-", + arg, new_option, hint); + else + error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>", + value ? "" : "no-", arg, new_option); + } else - error_at (loc, "%<-W%serror=%s%>: no option %<-%s%>", - value ? "" : "no-", arg, new_option); + ignored_wnoerror_options.safe_push (arg); } else if (!(cl_options[option_index].flags & CL_WARNING)) error_at (loc, "%<-Werror=%s%>: %<-%s%> is not an option that " diff --git a/gcc/opts.h b/gcc/opts.h index 47223229388..ac281aef540 100644 --- a/gcc/opts.h +++ b/gcc/opts.h @@ -461,4 +461,6 @@ extern bool parse_and_check_align_values (const char *flag, bool report_error, location_t loc); +extern vec<const char *> ignored_wnoerror_options; + #endif diff --git a/gcc/testsuite/gcc.dg/Werror-13.c b/gcc/testsuite/gcc.dg/Werror-13.c index 3a02b7ea2b5..7c2bf6836ed 100644 --- a/gcc/testsuite/gcc.dg/Werror-13.c +++ b/gcc/testsuite/gcc.dg/Werror-13.c @@ -5,6 +5,6 @@ /* { dg-error "'-Werror' is not an option that controls warnings" "" { target *-*-* } 0 } */ /* { dg-error "'-Wfatal-errors' is not an option that controls warnings" "" { target *-*-* } 0 } */ /* { dg-error "'-Werror=vla2': no option '-Wvla2'; did you mean '-Wvla." "" { target *-*-* } 0 } */ -/* { dg-error "'-Wno-error=misleading-indentation2': no option '-Wmisleading-indentation2'; did you mean '-Wmisleading-indentation'" "" { target *-*-* } 0 } */ +/* { dg-warning "'-Wno-error=misleading-indentation2': no option '-Wmisleading-indentation2'" "" { target *-*-* } 0 } */ int i;
From: Manuel López-Ibáñez <manu@gcc.gnu.org> * opts-common.c (ignored_wnoerror_options): New global variable. * opts-global.c (print_ignored_options): Ignore -Wno-error=<some-future-warning> except if there are other diagnostics. * opts.c (enable_warning_as_error): Record ignored -Wno-error options. * opts.h (ignored_wnoerror_options): Declare. * gcc.dg/Werror-13.c: Don't expect hints for -Wno-error=<some-future-warning>. --- gcc/opts-common.c | 2 ++ gcc/opts-global.c | 10 +++++++--- gcc/opts.c | 21 +++++++++++++-------- gcc/opts.h | 2 ++ gcc/testsuite/gcc.dg/Werror-13.c | 2 +- 5 files changed, 25 insertions(+), 12 deletions(-)