diff mbox series

[03/12] OpenMP/C++: Remove case PRAGMA_OMP_ALLOCATE from cp_parser_omp_construct

Message ID 20260505151030.1749548-4-waffl3x@baylibre.com
State New
Headers
Series OpenMP/C++: 'allocate' directive |

Commit Message

Waffl3x May 5, 2026, 3:01 p.m. UTC 
  The OpenMP allocate directive is not a construct, and thus is not supposed
to be handled by cp_parser_omp_construct, as far as I can tell it never was.
With this change PRAGMA_OMP_ALLOCATE will correctly trap if handled here,
as such this patch should have no functional impact, merely clarifying
intent and preventing future incorrect usage.

This also adds a few clarifying comments, based on my interpretation of the
code. The significance of the return value of cp_parser_pragma is still not
totally clear to me so no general comments are included for it.

gcc/cp/ChangeLog:

	* parser.cc (cp_parser_omp_construct)
	<case PRAGMA_OMP_ALLOCATE>: Remove.
	(cp_parser_pragma): Add comments.

Signed-off-by: Waffl3x <waffl3x@baylibre.com>
---
 gcc/cp/parser.cc | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Sandra Loosemore June 3, 2026, 7:32 p.m. UTC | #1 
On 5/5/26 09:01, Waffl3x wrote:
> The OpenMP allocate directive is not a construct, and thus is not supposed
> to be handled by cp_parser_omp_construct, as far as I can tell it never was.
> With this change PRAGMA_OMP_ALLOCATE will correctly trap if handled here,
> as such this patch should have no functional impact, merely clarifying
> intent and preventing future incorrect usage.
> 
> This also adds a few clarifying comments, based on my interpretation of the
> code. The significance of the return value of cp_parser_pragma is still not
> totally clear to me so no general comments are included for it.
> 
> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_omp_construct)
> 	<case PRAGMA_OMP_ALLOCATE>: Remove.
> 	(cp_parser_pragma): Add comments.
> 
> Signed-off-by: Waffl3x <waffl3x@baylibre.com>

I think this one also qualifies as "obvious".  It's consistent with how 
the C parser handles this and fixes what appears to be a think-o in the 
C++ parser support (which was added at the same time).

I assume this is all adequately tested by later patches in the series, 
right?

-Sandra
Tobias Burnus June 4, 2026, 9:49 a.m. UTC | #2 
Waffl3x wrote:
> The OpenMP allocate directive is not a construct, and thus is not supposed
> to be handled by cp_parser_omp_construct,
...

> gcc/cp/ChangeLog:
> 
> 	* parser.cc (cp_parser_omp_construct)
> 	<case PRAGMA_OMP_ALLOCATE>: Remove.
> 	(cp_parser_pragma): Add comments.


> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -56537,9 +56537,6 @@ cp_parser_omp_construct (cp_parser *parser, cp_token *pragma_tok, bool *if_p)
>       case PRAGMA_OACC_WAIT:
>         stmt = cp_parser_oacc_wait (parser, pragma_tok);
>         break;
> -    case PRAGMA_OMP_ALLOCATE:
> -      cp_parser_omp_allocate (parser, pragma_tok);
> -      return;
>       case PRAGMA_OMP_ATOMIC:
>         cp_parser_omp_atomic (parser, pragma_tok, false);
>         return;

Well spotted. Thanks!


> @@ -57249,7 +57246,9 @@ cp_parser_pragma (cp_parser *parser, enum pragma_context context, bool *if_p)
>         cp_parser_omp_construct (parser, pragma_tok, if_p);
>         return true;
>       case PRAGMA_OMP_ALLOCATE:
> +      /* The allocate directive is not a construct.  */
>         cp_parser_omp_allocate (parser, pragma_tok);
> +      /* EOL is handled in cp_parser_omp_allocate, don't break.  */
>         return false;

The first one is IMHO more confusing than helpful - it is true for
about a third of the directives listed - i.e. both construct and
non-constructs are handled here.

I think the second comment is not needed (true for about half of the
directives in this function + both its the only thing done after the
switch statement) - however, it also does not really harm.

Otherwise, LGTM.

Tobias
diff mbox series

Patch

diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index dc67cfd9f7c..ed24621452a 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -56537,9 +56537,6 @@  cp_parser_omp_construct (cp_parser *parser, cp_token *pragma_tok, bool *if_p)
     case PRAGMA_OACC_WAIT:
       stmt = cp_parser_oacc_wait (parser, pragma_tok);
       break;
-    case PRAGMA_OMP_ALLOCATE:
-      cp_parser_omp_allocate (parser, pragma_tok);
-      return;
     case PRAGMA_OMP_ATOMIC:
       cp_parser_omp_atomic (parser, pragma_tok, false);
       return;
@@ -57249,7 +57246,9 @@  cp_parser_pragma (cp_parser *parser, enum pragma_context context, bool *if_p)
       cp_parser_omp_construct (parser, pragma_tok, if_p);
       return true;
     case PRAGMA_OMP_ALLOCATE:
+      /* The allocate directive is not a construct.  */
       cp_parser_omp_allocate (parser, pragma_tok);
+      /* EOL is handled in cp_parser_omp_allocate, don't break.  */
       return false;
     case PRAGMA_OACC_ATOMIC:
     case PRAGMA_OACC_CACHE: