* [C++ PATCH] Toplevel asm volatile (PR c++/89585)
@ 2019-03-06 23:49 Jakub Jelinek
2019-03-07 0:54 ` Jason Merrill
2019-03-07 21:20 ` [C++ PATCH] Toplevel asm volatile " Matthias Klose
0 siblings, 2 replies; 6+ messages in thread
From: Jakub Jelinek @ 2019-03-06 23:49 UTC (permalink / raw)
To: Jason Merrill, Segher Boessenkool, Joseph S. Myers; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 4601 bytes --]
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 <jakub@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
[-- Attachment #2: T689c --]
[-- Type: text/plain, Size: 2593 bytes --]
2019-03-07 Jakub Jelinek <jakub@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 "" }
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Toplevel asm volatile (PR c++/89585)
2019-03-06 23:49 [C++ PATCH] Toplevel asm volatile (PR c++/89585) Jakub Jelinek
@ 2019-03-07 0:54 ` Jason Merrill
2019-03-07 19:18 ` [C++ PATCH] Toplevel asm volatile followup " Jakub Jelinek
2019-03-07 21:20 ` [C++ PATCH] Toplevel asm volatile " Matthias Klose
1 sibling, 1 reply; 6+ messages in thread
From: Jason Merrill @ 2019-03-07 0:54 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Segher Boessenkool, Joseph S. Myers, gcc-patches List
On Wed, Mar 6, 2019 at 6:39 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> 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?
That seems reasonable. Or using warning or permerror instead of error.
> 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?
Both OK.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
* [C++ PATCH] Toplevel asm volatile followup (PR c++/89585)
2019-03-07 0:54 ` Jason Merrill
@ 2019-03-07 19:18 ` Jakub Jelinek
2019-03-08 7:04 ` Jason Merrill
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2019-03-07 19:18 UTC (permalink / raw)
To: Jason Merrill; +Cc: Segher Boessenkool, Joseph S. Myers, gcc-patches List
On Wed, Mar 06, 2019 at 07:44:25PM -0500, Jason Merrill wrote:
> > 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?
>
> That seems reasonable. Or using warning or permerror instead of error.
This incremental patch uses warning. Bootstrapped/regtested on
x86_64-linux and i686-linux, ok for trunk?
2019-03-07 Jakub Jelinek <jakub@redhat.com>
PR c++/89585
* parser.c (cp_parser_asm_definition): Just warn instead of error
on volatile qualifier outside of function body.
* g++.dg/asm-qual-3.C: Adjust expected diagnostics for toplevel
asm volatile.
--- gcc/cp/parser.c.jj 2019-03-07 09:17:41.406631683 +0100
+++ gcc/cp/parser.c 2019-03-07 10:06:49.297602880 +0100
@@ -19782,9 +19782,12 @@ cp_parser_asm_definition (cp_parser* par
inform (volatile_loc, "first seen here");
}
else
- volatile_loc = loc;
- if (!first_loc)
- first_loc = loc;
+ {
+ if (!parser->in_function_body)
+ warning_at (loc, 0, "asm qualifier %qT ignored outside of "
+ "function body", token->u.value);
+ volatile_loc = loc;
+ }
cp_lexer_consume_token (parser->lexer);
continue;
@@ -19830,10 +19833,10 @@ 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))
+ if (!parser->in_function_body && (inline_p || goto_p))
{
error_at (first_loc, "asm qualifier outside of function body");
- volatile_p = inline_p = goto_p = false;
+ inline_p = goto_p = false;
}
/* Look for the opening `('. */
--- gcc/testsuite/g++.dg/asm-qual-3.C.jj 2019-03-07 09:17:41.417631503 +0100
+++ gcc/testsuite/g++.dg/asm-qual-3.C 2019-03-07 10:11:18.160228806 +0100
@@ -3,7 +3,7 @@
// { dg-options "-std=gnu++98" }
asm const (""); // { dg-error {'const' is not an asm qualifier} }
-asm volatile (""); // { dg-error {asm qualifier outside of function body} }
+asm volatile (""); // { dg-warning {asm qualifier 'volatile' ignored outside of function body} }
asm restrict (""); // { dg-error {expected '\(' before 'restrict'} }
asm inline (""); // { dg-error {asm qualifier outside of function body} }
asm goto (""); // { dg-error {asm qualifier outside of function body} }
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Toplevel asm volatile (PR c++/89585)
2019-03-06 23:49 [C++ PATCH] Toplevel asm volatile (PR c++/89585) Jakub Jelinek
2019-03-07 0:54 ` Jason Merrill
@ 2019-03-07 21:20 ` Matthias Klose
2019-03-07 21:34 ` Jakub Jelinek
1 sibling, 1 reply; 6+ messages in thread
From: Matthias Klose @ 2019-03-07 21:20 UTC (permalink / raw)
To: Jakub Jelinek, Jason Merrill, Segher Boessenkool, Joseph S. Myers
Cc: gcc-patches
On 07.03.19 00:39, Jakub Jelinek wrote:
> 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?
isn't that required for the gcc-7 branch as well? r267536 backported these
patches to the 7 branch as well.
Matthias
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Toplevel asm volatile (PR c++/89585)
2019-03-07 21:20 ` [C++ PATCH] Toplevel asm volatile " Matthias Klose
@ 2019-03-07 21:34 ` Jakub Jelinek
0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2019-03-07 21:34 UTC (permalink / raw)
To: Matthias Klose
Cc: Jason Merrill, Segher Boessenkool, Joseph S. Myers, gcc-patches
On Thu, Mar 07, 2019 at 10:11:56PM +0100, Matthias Klose wrote:
> On 07.03.19 00:39, Jakub Jelinek wrote:
> > 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?
>
> isn't that required for the gcc-7 branch as well?
Yes.
> r267536 backported these patches to the 7 branch as well.
If you've tested it, feel free to commit it to 7.x.
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [C++ PATCH] Toplevel asm volatile followup (PR c++/89585)
2019-03-07 19:18 ` [C++ PATCH] Toplevel asm volatile followup " Jakub Jelinek
@ 2019-03-08 7:04 ` Jason Merrill
0 siblings, 0 replies; 6+ messages in thread
From: Jason Merrill @ 2019-03-08 7:04 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Segher Boessenkool, Joseph S. Myers, gcc-patches List
On 3/7/19 2:13 PM, Jakub Jelinek wrote:
> On Wed, Mar 06, 2019 at 07:44:25PM -0500, Jason Merrill wrote:
>>> 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?
>>
>> That seems reasonable. Or using warning or permerror instead of error.
>
> This incremental patch uses warning. Bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?
OK.
Jason
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-08 4:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-06 23:49 [C++ PATCH] Toplevel asm volatile (PR c++/89585) Jakub Jelinek
2019-03-07 0:54 ` Jason Merrill
2019-03-07 19:18 ` [C++ PATCH] Toplevel asm volatile followup " Jakub Jelinek
2019-03-08 7:04 ` Jason Merrill
2019-03-07 21:20 ` [C++ PATCH] Toplevel asm volatile " Matthias Klose
2019-03-07 21:34 ` Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).