diff mbox

[c/c++] use explicit locations for some warnings in c-pragma.c

Message ID CAESRpQAss9wY48rxjsVC_VHn5c2bC+ZKf4m6fvL-ZNVFLkmKEA@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez May 25, 2015, 4:06 p.m. UTC
This changes from:

pragma-diag-3.c:2:9: warning: missing [error|warning|ignored] after
‘#pragma GCC diagnostic’ [-Wpragmas]
 #pragma GCC diagnostic /* { dg-warning "24:missing" "missing" { xfail
*-*-* } } */
         ^
pragma-diag-3.c:4:9: warning: expected
[error|warning|ignored|push|pop] after ‘#pragma GCC diagnostic’
[-Wpragmas]
 #pragma GCC diagnostic warn /* { dg-warning "24:expected" } */
         ^
pragma-diag-3.c:6:9: warning: unknown option after ‘#pragma GCC
diagnostic’ kind [-Wpragmas]
 #pragma GCC diagnostic ignored "-Wfoo" /* { dg-warning "32:unknown" } */
         ^

to:


pragma-diag-3.c:2:83: warning: missing
[error|warning|ignored|push|pop] after ‘#pragma GCC diagnostic’
[-Wpragmas]
pragma-diag-3.c:4:24: warning: expected
[error|warning|ignored|push|pop] after ‘#pragma GCC diagnostic’
[-Wpragmas]
 #pragma GCC diagnostic warn /* { dg-warning "24:expected" } */
                        ^
pragma-diag-3.c:6:32: warning: unknown option after ‘#pragma GCC
diagnostic’ kind [-Wpragmas]
 #pragma GCC diagnostic ignored "-Wfoo" /* { dg-warning "32:unknown" } */
                                ^

which is a clear improvement except for the fact that CPP_PRAGMA_EOL
tokens do not have a valid location. I think this is a bug and will
open a PR.

Of course, this opens the door to giving better locations for all
diagnostics in c-pragma.c, but I'll leave that for the future.

Bootstrapped and regression tested on x86_64-linux-gnu.

OK?

gcc/cp/ChangeLog:

2015-05-25  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    * parser.c (pragma_lex): Add loc argument. Rearrange the code to
    make it more similar to the C version.

gcc/c-family/ChangeLog:

2015-05-25  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    * c-pragma.c (handle_pragma_diagnostic): Use explicit location
    when warning.
    * c-pragma.h (pragma_lex): Add optional loc argument.

gcc/c/ChangeLog:

2015-05-25  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    * c-parser.c (pragma_lex): Add loc argument.

gcc/testsuite/ChangeLog:

2015-05-25  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    * gcc.dg/pragma-diag-3.c: New test.

Comments

Jason Merrill May 25, 2015, 7:39 p.m. UTC | #1
OK.

Jason
Marek Polacek May 25, 2015, 7:56 p.m. UTC | #2
On Mon, May 25, 2015 at 06:06:01PM +0200, Manuel López-Ibáñez wrote:
>    if (token != CPP_NAME)
> -    GCC_BAD ("missing [error|warning|ignored] after %<#pragma GCC diagnostic%>");
> +    {
> +      warning_at (loc, OPT_Wpragmas,
> +		  "missing [error|warning|ignored|push|pop] after %<#pragma GCC diagnostic%>");

Line too long.

> -    GCC_BAD ("expected [error|warning|ignored|push|pop] after %<#pragma GCC diagnostic%>");
> +    {
> +      warning_at (loc, OPT_Wpragmas,
> +		  "expected [error|warning|ignored|push|pop] after %<#pragma GCC diagnostic%>");

Likewise.

Perhaps we should introduce GCC_BAD_LOC with a location_t argument and use it
here.

	Marek
Manuel López-Ibáñez May 25, 2015, 8:16 p.m. UTC | #3
On 25 May 2015 at 21:56, Marek Polacek <polacek@redhat.com> wrote:
> Perhaps we should introduce GCC_BAD_LOC with a location_t argument and use it
> here.

Why would we want to obfuscate code like that? I would propose to
actually remove GCC_BAD completely.

Manuel.
Christophe Lyon Sept. 20, 2015, 8:32 p.m. UTC | #4
On 25 May 2015 at 22:16, Manuel López-Ibáñez <lopezibanez@gmail.com> wrote:
> On 25 May 2015 at 21:56, Marek Polacek <polacek@redhat.com> wrote:
>> Perhaps we should introduce GCC_BAD_LOC with a location_t argument and use it
>> here.
>
> Why would we want to obfuscate code like that? I would propose to
> actually remove GCC_BAD completely.
>
Hi
It looks like this patch has finally been committed on 2015-09-18
(r227923), right?

I can see the newly introduced test (gcc.dg/pragma-diag-5.c) failing:
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/pragma-diag-5.c:2:83:
warning: missing [error|warning|ignored|push|pop] after '#pragma GCC
diagnostic' [-Wpragmas]
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/pragma-diag-5.c:4:24:
warning: expected [error|warning|ignored|push|pop] after '#pragma GCC
diagnostic' [-Wpragmas]
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/pragma-diag-5.c:6:32:
warning: unknown option after '#pragma GCC diagnostic' kind
[-Wpragmas]

XFAIL: gcc.dg/pragma-diag-5.c missing (test for warnings, line 2)
PASS: gcc.dg/pragma-diag-5.c  (test for warnings, line 4)
PASS: gcc.dg/pragma-diag-5.c  (test for warnings, line 6)
FAIL: gcc.dg/pragma-diag-5.c (test for excess errors)
Excess errors:
/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gcc.dg/pragma-diag-5.c:2:83:
warning: missing [error|warning|ignored|push|pop] after '#pragma GCC
diagnostic' [-Wpragmas]

I'm not sure why, since the 1st warning is xfail.

Christophe.

> Manuel.
diff mbox

Patch

Index: gcc/c-family/c-pragma.c
===================================================================
--- gcc/c-family/c-pragma.c	(revision 223596)
+++ gcc/c-family/c-pragma.c	(working copy)
@@ -725,14 +725,20 @@  handle_pragma_diagnostic(cpp_reader *ARG
   unsigned int option_index;
   enum cpp_ttype token;
   diagnostic_t kind;
   tree x;
   struct cl_option_handlers handlers;
+  location_t loc;
 
-  token = pragma_lex (&x);
+  token = pragma_lex (&x, &loc);
   if (token != CPP_NAME)
-    GCC_BAD ("missing [error|warning|ignored] after %<#pragma GCC diagnostic%>");
+    {
+      warning_at (loc, OPT_Wpragmas,
+		  "missing [error|warning|ignored|push|pop] after %<#pragma GCC diagnostic%>");
+      return;
+    }
+
   kind_string = IDENTIFIER_POINTER (x);
   if (strcmp (kind_string, "error") == 0)
     kind = DK_ERROR;
   else if (strcmp (kind_string, "warning") == 0)
     kind = DK_WARNING;
@@ -747,15 +753,23 @@  handle_pragma_diagnostic(cpp_reader *ARG
     {
       diagnostic_pop_diagnostics (global_dc, input_location);
       return;
     }
   else
-    GCC_BAD ("expected [error|warning|ignored|push|pop] after %<#pragma GCC diagnostic%>");
+    {
+      warning_at (loc, OPT_Wpragmas,
+		  "expected [error|warning|ignored|push|pop] after %<#pragma GCC diagnostic%>");
+      return;
+    }
 
-  token = pragma_lex (&x);
+  token = pragma_lex (&x, &loc);
   if (token != CPP_STRING)
-    GCC_BAD ("missing option after %<#pragma GCC diagnostic%> kind");
+    {
+      warning_at (loc, OPT_Wpragmas,
+		  "missing option after %<#pragma GCC diagnostic%> kind");
+      return;
+    }
   option_string = TREE_STRING_POINTER (x);
   set_default_handlers (&handlers);
   for (option_index = 0; option_index < cl_options_count; option_index++)
     if (strcmp (cl_options[option_index].opt_text, option_string) == 0)
       {
@@ -763,11 +777,12 @@  handle_pragma_diagnostic(cpp_reader *ARG
 				input_location, c_family_lang_mask, &handlers,
 				&global_options, &global_options_set,
 				global_dc);
 	return;
       }
-  GCC_BAD ("unknown option after %<#pragma GCC diagnostic%> kind");
+  warning_at (loc, OPT_Wpragmas,
+	      "unknown option after %<#pragma GCC diagnostic%> kind");
 }
 
 /*  Parse #pragma GCC target (xxx) to set target specific options.  */
 static void
 handle_pragma_target(cpp_reader *ARG_UNUSED(dummy))
Index: gcc/c-family/c-pragma.h
===================================================================
--- gcc/c-family/c-pragma.h	(revision 223596)
+++ gcc/c-family/c-pragma.h	(working copy)
@@ -211,11 +211,11 @@  extern void c_invoke_pragma_handler (uns
 extern void maybe_apply_pragma_weak (tree);
 extern void maybe_apply_pending_pragma_weaks (void);
 extern tree maybe_apply_renaming_pragma (tree, tree);
 extern void add_to_renaming_pragma_list (tree, tree);
 
-extern enum cpp_ttype pragma_lex (tree *);
+extern enum cpp_ttype pragma_lex (tree *, location_t *loc = NULL);
 
 /* Flags for use with c_lex_with_flags.  The values here were picked
    so that 0 means to translate and join strings.  */
 #define C_LEX_STRING_NO_TRANSLATE 1 /* Do not lex strings into
 				       execution character set.  */
Index: gcc/c/c-parser.c
===================================================================
--- gcc/c/c-parser.c	(revision 223596)
+++ gcc/c/c-parser.c	(working copy)
@@ -9835,16 +9835,19 @@  c_parser_pragma (c_parser *parser, enum 
 }
 
 /* The interface the pragma parsers have to the lexer.  */
 
 enum cpp_ttype
-pragma_lex (tree *value)
+pragma_lex (tree *value, location_t *loc)
 {
   c_token *tok = c_parser_peek_token (the_parser);
   enum cpp_ttype ret = tok->type;
 
   *value = tok->value;
+  if (loc)
+    *loc = tok->location;
+
   if (ret == CPP_PRAGMA_EOL || ret == CPP_EOF)
     ret = CPP_EOF;
   else
     {
       if (ret == CPP_KEYWORD)
Index: gcc/testsuite/gcc.dg/pragma-diag-3.c
===================================================================
--- gcc/testsuite/gcc.dg/pragma-diag-3.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pragma-diag-3.c	(revision 0)
@@ -0,0 +1,6 @@ 
+/* { dg-do compile } */
+#pragma GCC diagnostic /* { dg-warning "24:missing" "missing" { xfail *-*-* } } */
+
+#pragma GCC diagnostic warn /* { dg-warning "24:expected" } */
+
+#pragma GCC diagnostic ignored "-Wfoo" /* { dg-warning "32:unknown" } */
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 223596)
+++ gcc/cp/parser.c	(working copy)
@@ -33175,29 +33175,28 @@  cp_parser_pragma (cp_parser *parser, enu
 }
 
 /* The interface the pragma parsers have to the lexer.  */
 
 enum cpp_ttype
-pragma_lex (tree *value)
+pragma_lex (tree *value, location_t *loc)
 {
-  cp_token *tok;
-  enum cpp_ttype ret;
+  cp_token *tok = cp_lexer_peek_token (the_parser->lexer);
+  enum cpp_ttype ret = tok->type;
 
-  tok = cp_lexer_peek_token (the_parser->lexer);
-
-  ret = tok->type;
   *value = tok->u.value;
+  if (loc)
+    *loc = tok->location;
 
   if (ret == CPP_PRAGMA_EOL || ret == CPP_EOF)
     ret = CPP_EOF;
   else if (ret == CPP_STRING)
     *value = cp_parser_string_literal (the_parser, false, false);
   else
     {
-      cp_lexer_consume_token (the_parser->lexer);
       if (ret == CPP_KEYWORD)
 	ret = CPP_NAME;
+      cp_lexer_consume_token (the_parser->lexer);
     }
 
   return ret;
 }