[resend,1/2] PR c/65403 - Ignore -Wno-error=<some-future-warning>

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>
Related show

Commit Message

Alex Henrie Aug. 23, 2019, 4:17 a.m.
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(-)

-- 
2.23.0

Comments

Martin Liška Aug. 23, 2019, 9:13 a.m. | #1
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
Alex Henrie Aug. 23, 2019, 4:52 p.m. | #2
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
Martin Liška Aug. 26, 2019, 12:24 p.m. | #3
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

>
Martin Sebor Aug. 28, 2019, 1:56 p.m. | #4
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

>>

>

Patch

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;