diff mbox

Improve diagnostic messages of "#pragma omp cancel", "#pragma omp cancellation point" parsing (was: Clarify PRAGMA_OACC_* and PRAGMA_OMP_*)

Message ID 87wpl9o69q.fsf@hertz.schwinge.homeip.net
State New
Headers show

Commit Message

Thomas Schwinge June 28, 2016, 12:19 p.m. UTC 
Hi!

On Tue, 28 Jun 2016 14:13:24 +0200, I wrote:
> PRAGMA_OACC_* and PRAGMA_OMP_* don't describe what *eventually* is to be
> parsed [...], but instead they describe what *so far* has been parsed.

> --- gcc/c/c-parser.c
> +++ gcc/c/c-parser.c
> @@ -10186,7 +10186,7 @@ c_parser_pragma (c_parser *parser, enum pragma_context context, bool *if_p)

> -    case PRAGMA_OMP_CANCELLATION_POINT:
> +    case PRAGMA_OMP_CANCELLATION:
>        if (context != pragma_compound)
>  	{
>  	  if (context == pragma_stmt)

> --- gcc/cp/parser.c
> +++ gcc/cp/parser.c
> @@ -37290,7 +37290,7 @@ cp_parser_pragma (cp_parser *parser, enum pragma_context context, bool *if_p)

> -    case PRAGMA_OMP_CANCELLATION_POINT:
> +    case PRAGMA_OMP_CANCELLATION:
>        switch (context)
>  	{
>  	case pragma_compound:

This also makes it obvious that stuff like:

    void foo()
    {
      if (0)
    #pragma omp cancellation FOO
        ;
    }

... currently produces "wrong" ;-) diagnostics:

    ../omp.c: In function 'foo':
    ../omp.c:4:9: error: '#pragma omp cancellation point' may only be used in compound statements before '#pragma'
     #pragma omp cancellation FOO
             ^~~

We have in fact not yet parsed "#pragma omp cancellation point".  I
corrected that.  And the "before '#pragma'" also doesn't make sense in
that diagnostic message, so I switched that to use error_at instead of
c_parser_error/cp_parser_error (this remains to be fixed for other cases
in c_parser_pragma).  And, for one diagnostic, C++ used
cp_parser_require_pragma_eol instead of cp_parser_skip_to_pragma_eol,
resulting in spurious "error: expected end of line before [...]"
diagnostics, also fixed.  And, a few missing "%>" added.  At that point,
I stopped looking for additional things to fix.  ;-)

OK for trunk?

commit 2e17681659f8f8ec1ce3419c6587e1ee569ee8bc
Author: Thomas Schwinge <thomas@codesourcery.com>
Date:   Tue Jun 28 14:16:20 2016 +0200

    Improve diagnostic messages of "#pragma omp cancel", "#pragma omp cancellation point" parsing
    
    	gcc/c/
    	* c-parser.c (c_parser_pragma) <PRAGMA_OMP_CANCELLATION>: Move
    	pragma context checking into...
    	(c_parser_omp_cancellation_point): ... here, and improve
    	diagnostic messages.
    	* c-typeck.c (c_finish_omp_cancel)
    	(c_finish_omp_cancellation_point): Improve diagnostic messages.
    	gcc/cp/
    	* parser.c (cp_parser_pragma) <PRAGMA_OMP_CANCELLATION>: Move
    	pragma context checking into...
    	(cp_parser_omp_cancellation_point): ... here, and improve
    	diagnostic messages.
    	* semantics.c (finish_omp_cancel, finish_omp_cancellation_point):
    	Improve diagnostic messages.
    	gcc/testsuite/
    	* c-c++-common/gomp/cancel-1.c: Extend.
---
 gcc/c/c-parser.c                           | 24 +++++++++++++---------
 gcc/c/c-typeck.c                           |  4 ++--
 gcc/cp/parser.c                            | 33 +++++++++++++++---------------
 gcc/cp/semantics.c                         |  4 ++--
 gcc/testsuite/c-c++-common/gomp/cancel-1.c | 15 ++++++++++++++
 5 files changed, 50 insertions(+), 30 deletions(-)



Grüße
 Thomas

Comments

Jakub Jelinek June 28, 2016, 12:25 p.m. UTC | #1 
On Tue, Jun 28, 2016 at 02:19:13PM +0200, Thomas Schwinge wrote:
> This also makes it obvious that stuff like:
> 
>     void foo()
>     {
>       if (0)
>     #pragma omp cancellation FOO
>         ;
>     }
> 
> ... currently produces "wrong" ;-) diagnostics:
> 
>     ../omp.c: In function 'foo':
>     ../omp.c:4:9: error: '#pragma omp cancellation point' may only be used in compound statements before '#pragma'
>      #pragma omp cancellation FOO
>              ^~~
> 
> We have in fact not yet parsed "#pragma omp cancellation point".  I
> corrected that.  And the "before '#pragma'" also doesn't make sense in
> that diagnostic message, so I switched that to use error_at instead of
> c_parser_error/cp_parser_error (this remains to be fixed for other cases
> in c_parser_pragma).  And, for one diagnostic, C++ used
> cp_parser_require_pragma_eol instead of cp_parser_skip_to_pragma_eol,
> resulting in spurious "error: expected end of line before [...]"
> diagnostics, also fixed.  And, a few missing "%>" added.  At that point,
> I stopped looking for additional things to fix.  ;-)
> 
> OK for trunk?

This LGTM if you tweak it to apply without the previous patch or with just
a patch for PRAGMA_OMP_DECLARE.

Thanks.

> commit 2e17681659f8f8ec1ce3419c6587e1ee569ee8bc
> Author: Thomas Schwinge <thomas@codesourcery.com>
> Date:   Tue Jun 28 14:16:20 2016 +0200
> 
>     Improve diagnostic messages of "#pragma omp cancel", "#pragma omp cancellation point" parsing
>     
>     	gcc/c/
>     	* c-parser.c (c_parser_pragma) <PRAGMA_OMP_CANCELLATION>: Move
>     	pragma context checking into...
>     	(c_parser_omp_cancellation_point): ... here, and improve
>     	diagnostic messages.
>     	* c-typeck.c (c_finish_omp_cancel)
>     	(c_finish_omp_cancellation_point): Improve diagnostic messages.
>     	gcc/cp/
>     	* parser.c (cp_parser_pragma) <PRAGMA_OMP_CANCELLATION>: Move
>     	pragma context checking into...
>     	(cp_parser_omp_cancellation_point): ... here, and improve
>     	diagnostic messages.
>     	* semantics.c (finish_omp_cancel, finish_omp_cancellation_point):
>     	Improve diagnostic messages.
>     	gcc/testsuite/
>     	* c-c++-common/gomp/cancel-1.c: Extend.

	Jakub
diff mbox

Patch

diff --git gcc/c/c-parser.c gcc/c/c-parser.c
index ccdd043..581fb01 100644
--- gcc/c/c-parser.c
+++ gcc/c/c-parser.c
@@ -1358,11 +1358,11 @@  static tree c_parser_omp_for_loop (location_t, c_parser *, enum tree_code,
 static void c_parser_omp_taskwait (c_parser *);
 static void c_parser_omp_taskyield (c_parser *);
 static void c_parser_omp_cancel (c_parser *);
-static void c_parser_omp_cancellation_point (c_parser *);
 
 enum pragma_context { pragma_external, pragma_struct, pragma_param,
 		      pragma_stmt, pragma_compound };
 static bool c_parser_pragma (c_parser *, enum pragma_context, bool *);
+static void c_parser_omp_cancellation_point (c_parser *, enum pragma_context);
 static bool c_parser_omp_target (c_parser *, enum pragma_context, bool *);
 static void c_parser_omp_end_declare_target (c_parser *);
 static void c_parser_omp_declare (c_parser *, enum pragma_context);
@@ -10187,14 +10187,7 @@  c_parser_pragma (c_parser *parser, enum pragma_context context, bool *if_p)
       return false;
 
     case PRAGMA_OMP_CANCELLATION:
-      if (context != pragma_compound)
-	{
-	  if (context == pragma_stmt)
-	    c_parser_error (parser, "%<#pragma omp cancellation point%> may "
-				    "only be used in compound statements");
-	  goto bad_stmt;
-	}
-      c_parser_omp_cancellation_point (parser);
+      c_parser_omp_cancellation_point (parser, context);
       return false;
 
     case PRAGMA_OMP_THREADPRIVATE:
@@ -15668,7 +15661,7 @@  c_parser_omp_cancel (c_parser *parser)
 	| (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_TASKGROUP))
 
 static void
-c_parser_omp_cancellation_point (c_parser *parser)
+c_parser_omp_cancellation_point (c_parser *parser, enum pragma_context context)
 {
   location_t loc = c_parser_peek_token (parser)->location;
   tree clauses;
@@ -15691,6 +15684,17 @@  c_parser_omp_cancellation_point (c_parser *parser)
       return;
     }
 
+  if (context != pragma_compound)
+    {
+      if (context == pragma_stmt)
+	error_at (loc, "%<#pragma omp cancellation point%> may only be used in"
+		  " compound statements");
+      else
+	c_parser_error (parser, "expected declaration specifiers");
+      c_parser_skip_to_pragma_eol (parser, false);
+      return;
+    }
+
   clauses
     = c_parser_omp_all_clauses (parser, OMP_CANCELLATION_POINT_CLAUSE_MASK,
 				"#pragma omp cancellation point");
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index 7c6241c..a9e286a 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -11926,7 +11926,7 @@  c_finish_omp_cancel (location_t loc, tree clauses)
     mask = 8;
   else
     {
-      error_at (loc, "%<#pragma omp cancel must specify one of "
+      error_at (loc, "%<#pragma omp cancel%> must specify one of "
 		     "%<parallel%>, %<for%>, %<sections%> or %<taskgroup%> "
 		     "clauses");
       return;
@@ -11965,7 +11965,7 @@  c_finish_omp_cancellation_point (location_t loc, tree clauses)
     mask = 8;
   else
     {
-      error_at (loc, "%<#pragma omp cancellation point must specify one of "
+      error_at (loc, "%<#pragma omp cancellation point%> must specify one of "
 		     "%<parallel%>, %<for%>, %<sections%> or %<taskgroup%> "
 		     "clauses");
       return;
diff --git gcc/cp/parser.c gcc/cp/parser.c
index 6a90706..cdba3de 100644
--- gcc/cp/parser.c
+++ gcc/cp/parser.c
@@ -34395,7 +34395,8 @@  cp_parser_omp_cancel (cp_parser *parser, cp_token *pragma_tok)
 	| (OMP_CLAUSE_MASK_1 << PRAGMA_OMP_CLAUSE_TASKGROUP))
 
 static void
-cp_parser_omp_cancellation_point (cp_parser *parser, cp_token *pragma_tok)
+cp_parser_omp_cancellation_point (cp_parser *parser, cp_token *pragma_tok,
+				  enum pragma_context context)
 {
   tree clauses;
   bool point_seen = false;
@@ -34414,7 +34415,19 @@  cp_parser_omp_cancellation_point (cp_parser *parser, cp_token *pragma_tok)
   if (!point_seen)
     {
       cp_parser_error (parser, "expected %<point%>");
-      cp_parser_require_pragma_eol (parser, pragma_tok);
+      cp_parser_skip_to_pragma_eol (parser, pragma_tok);
+      return;
+    }
+
+  if (context != pragma_compound)
+    {
+      if (context == pragma_stmt)
+	error_at (pragma_tok->location,
+		  "%<#pragma omp cancellation point%> may only be used in"
+		  " compound statements");
+      else
+	cp_parser_error (parser, "expected declaration specifiers");
+      cp_parser_skip_to_pragma_eol (parser, pragma_tok);
       return;
     }
 
@@ -37291,20 +37304,8 @@  cp_parser_pragma (cp_parser *parser, enum pragma_context context, bool *if_p)
       break;
 
     case PRAGMA_OMP_CANCELLATION:
-      switch (context)
-	{
-	case pragma_compound:
-	  cp_parser_omp_cancellation_point (parser, pragma_tok);
-	  return false;
-	case pragma_stmt:
-	  error_at (pragma_tok->location,
-		    "%<#pragma omp cancellation point%> may only be "
-		    "used in compound statements");
-	  break;
-	default:
-	  goto bad_stmt;
-	}
-      break;
+      cp_parser_omp_cancellation_point (parser, pragma_tok, context);
+      return false;
 
     case PRAGMA_OMP_THREADPRIVATE:
       cp_parser_omp_threadprivate (parser, pragma_tok);
diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index fa4698e..d1fb119 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -8571,7 +8571,7 @@  finish_omp_cancel (tree clauses)
     mask = 8;
   else
     {
-      error ("%<#pragma omp cancel must specify one of "
+      error ("%<#pragma omp cancel%> must specify one of "
 	     "%<parallel%>, %<for%>, %<sections%> or %<taskgroup%> clauses");
       return;
     }
@@ -8608,7 +8608,7 @@  finish_omp_cancellation_point (tree clauses)
     mask = 8;
   else
     {
-      error ("%<#pragma omp cancellation point must specify one of "
+      error ("%<#pragma omp cancellation point%> must specify one of "
 	     "%<parallel%>, %<for%>, %<sections%> or %<taskgroup%> clauses");
       return;
     }
diff --git gcc/testsuite/c-c++-common/gomp/cancel-1.c gcc/testsuite/c-c++-common/gomp/cancel-1.c
index 896a768..d26fcf1 100644
--- gcc/testsuite/c-c++-common/gomp/cancel-1.c
+++ gcc/testsuite/c-c++-common/gomp/cancel-1.c
@@ -455,3 +455,18 @@  f3 (void)
       }
     }
 }
+
+#pragma omp cancellation point /* { dg-error "expected declaration specifiers before end of line" } */
+
+void
+f4 (void)
+{
+  if (0)
+#pragma omp cancellation EKAHI /* { dg-error "expected .point. before .EKAHI." } */
+    ;
+#pragma omp cancellation HO OKAHI /* { dg-error "expected .point. before .HO." } */
+  if (0)
+#pragma omp cancellation point /* { dg-error ".pragma omp cancellation point. may only be used in compound statements" } */
+    ;
+#pragma omp cancellation point /* { dg-error ".pragma omp cancellation point. must specify one of" } */
+}