Hi!

The following patch tries to improve diagnostics of toplevel asm qualifiers
in C++ by actually parsing them and complaining if they appear at toplevel,
instead of just emitting a parse error that ( is expected, e.g. some
versions of Qt do use toplevel asm volatile and apparently the Qt code is
copied into lots of various projects.

In addition to that, it mentions in the documentation that qualifiers are
not allowed at toplevel asm statements; apparently our documentation at
least from r220506 for GCC 5 says that at toplevel Basic Asm needs to be
used and for Basic Asm lists volatile qualifier as optional and its behavior
(that it is ignored for Basic Asm).  Makes me wonder if we don't want to
keep accepting/ignoring volatile at toplevel for both C and C++ instead of
rejecting it (and rejecting just the other qualifiers).  Thoughts on this?

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Attached is an untested backport of this patch to 8.4, which does allow
asm volatile at toplevel, so that we don't break in 8.4 what has been
accepted in 8.2.  Ok if it passes bootstrap/regtest there?

2019-03-07  Jakub Jelinek  <ja...@redhat.com>

        PR c++/89585
        * doc/extend.texi (Basic Asm): Document qualifiers are not allowed
        at toplevel.

        * parser.c (cp_parser_asm_definition): Parse asm qualifiers even
        at toplevel, but diagnose them.

        * g++.dg/asm-qual-3.C: Adjust expected diagnostics.

--- gcc/doc/extend.texi.jj      2019-02-26 21:35:26.782115737 +0100
+++ gcc/doc/extend.texi 2019-03-06 10:23:14.894032621 +0100
@@ -9064,6 +9064,8 @@ outside of C functions, you must use bas
 You can use this technique to emit assembler directives,
 define assembly language macros that can be invoked elsewhere in the file,
 or write entire functions in assembly language.
+Basic @code{asm} statements outside of functions may not use any
+qualifiers.
 
 @item
 Functions declared
--- gcc/cp/parser.c.jj  2019-03-05 09:39:18.850146559 +0100
+++ gcc/cp/parser.c     2019-03-06 19:41:27.861895296 +0100
@@ -19766,8 +19766,9 @@ cp_parser_asm_definition (cp_parser* par
   location_t volatile_loc = UNKNOWN_LOCATION;
   location_t inline_loc = UNKNOWN_LOCATION;
   location_t goto_loc = UNKNOWN_LOCATION;
+  location_t first_loc = UNKNOWN_LOCATION;
 
-  if (cp_parser_allow_gnu_extensions_p (parser) && parser->in_function_body)
+  if (cp_parser_allow_gnu_extensions_p (parser))
     for (;;)
       {
        cp_token *token = cp_lexer_peek_token (parser->lexer);
@@ -19782,6 +19783,8 @@ cp_parser_asm_definition (cp_parser* par
              }
            else
              volatile_loc = loc;
+           if (!first_loc)
+             first_loc = loc;
            cp_lexer_consume_token (parser->lexer);
            continue;
 
@@ -19793,6 +19796,8 @@ cp_parser_asm_definition (cp_parser* par
              }
            else
              inline_loc = loc;
+           if (!first_loc)
+             first_loc = loc;
            cp_lexer_consume_token (parser->lexer);
            continue;
 
@@ -19804,6 +19809,8 @@ cp_parser_asm_definition (cp_parser* par
              }
            else
              goto_loc = loc;
+           if (!first_loc)
+             first_loc = loc;
            cp_lexer_consume_token (parser->lexer);
            continue;
 
@@ -19823,6 +19830,12 @@ cp_parser_asm_definition (cp_parser* par
   bool inline_p = (inline_loc != UNKNOWN_LOCATION);
   bool goto_p = (goto_loc != UNKNOWN_LOCATION);
 
+  if (!parser->in_function_body && (volatile_p || inline_p || goto_p))
+    {
+      error_at (first_loc, "asm qualifier outside of function body");
+      volatile_p = inline_p = goto_p = false;
+    }
+
   /* Look for the opening `('.  */
   if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
     return;
--- gcc/testsuite/g++.dg/asm-qual-3.C.jj        2018-12-20 08:50:27.234484465 
+0100
+++ gcc/testsuite/g++.dg/asm-qual-3.C   2019-03-06 19:40:20.932993605 +0100
@@ -2,11 +2,11 @@
 // { dg-do compile }
 // { dg-options "-std=gnu++98" }
 
-asm const ("");    // { dg-error {expected '\(' before 'const'} }
-asm volatile (""); // { dg-error {expected '\(' before 'volatile'} }
+asm const ("");    // { dg-error {'const' is not an asm qualifier} }
+asm volatile (""); // { dg-error {asm qualifier outside of function body} }
 asm restrict (""); // { dg-error {expected '\(' before 'restrict'} }
-asm inline ("");   // { dg-error {expected '\(' before 'inline'} }
-asm goto ("");     // { dg-error {expected '\(' before 'goto'} }
+asm inline ("");   // { dg-error {asm qualifier outside of function body} }
+asm goto ("");     // { dg-error {asm qualifier outside of function body} }
 
 // There are many other things wrong with this code, so:
 // { dg-excess-errors "" }

        Jakub
2019-03-07  Jakub Jelinek  <ja...@redhat.com>

        PR c++/89585
        * parser.c (cp_parser_asm_definition): Parse asm qualifiers even
        at toplevel, but diagnose them.

        * g++.dg/asm-qual-3.C: Adjust expected diagnostics.

--- gcc/cp/parser.c.jj  2019-03-05 09:39:18.850146559 +0100
+++ gcc/cp/parser.c     2019-03-06 19:41:27.861895296 +0100
@@ -19766,8 +19766,9 @@ cp_parser_asm_definition (cp_parser* par
   location_t volatile_loc = UNKNOWN_LOCATION;
   location_t inline_loc = UNKNOWN_LOCATION;
   location_t goto_loc = UNKNOWN_LOCATION;
+  location_t first_loc = UNKNOWN_LOCATION;
 
-  if (cp_parser_allow_gnu_extensions_p (parser) && parser->in_function_body)
+  if (cp_parser_allow_gnu_extensions_p (parser))
     for (;;)
       {
        cp_token *token = cp_lexer_peek_token (parser->lexer);
@@ -19793,6 +19796,8 @@ cp_parser_asm_definition (cp_parser* par
              }
            else
              inline_loc = loc;
+           if (!first_loc)
+             first_loc = loc;
            cp_lexer_consume_token (parser->lexer);
            continue;
 
@@ -19804,6 +19809,8 @@ cp_parser_asm_definition (cp_parser* par
              }
            else
              goto_loc = loc;
+           if (!first_loc)
+             first_loc = loc;
            cp_lexer_consume_token (parser->lexer);
            continue;
 
@@ -19823,6 +19830,12 @@ cp_parser_asm_definition (cp_parser* par
   bool inline_p = (inline_loc != UNKNOWN_LOCATION);
   bool goto_p = (goto_loc != UNKNOWN_LOCATION);
 
+  if (!parser->in_function_body && (inline_p || goto_p))
+    {
+      error_at (first_loc, "asm qualifier outside of function body");
+      inline_p = goto_p = false;
+    }
+
   /* Look for the opening `('.  */
   if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
     return;
--- gcc/testsuite/g++.dg/asm-qual-3.C.jj        2018-12-20 08:50:27.234484465 
+0100
+++ gcc/testsuite/g++.dg/asm-qual-3.C   2019-03-06 19:40:20.932993605 +0100
@@ -2,11 +2,11 @@
 // { dg-do compile }
 // { dg-options "-std=gnu++98" }
 
-asm const ("");    // { dg-error {expected '\(' before 'const'} }
-asm volatile (""); // { dg-error {expected '\(' before 'volatile'} }
+asm const ("");    // { dg-error {'const' is not an asm qualifier} }
+asm volatile ("");
 asm restrict (""); // { dg-error {expected '\(' before 'restrict'} }
-asm inline ("");   // { dg-error {expected '\(' before 'inline'} }
-asm goto ("");     // { dg-error {expected '\(' before 'goto'} }
+asm inline ("");   // { dg-error {asm qualifier outside of function body} }
+asm goto ("");     // { dg-error {asm qualifier outside of function body} }
 
 // There are many other things wrong with this code, so:
 // { dg-excess-errors "" }

Reply via email to