public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Jason Merrill <jason@redhat.com>
Cc: gcc-patches@gcc.gnu.org, Marek Polacek <polacek@redhat.com>
Subject: [PATCH] c++: Implement C++ DR 2262 - Attributes for asm-definition [PR110734]
Date: Tue, 5 Dec 2023 08:40:32 +0100	[thread overview]
Message-ID: <ZW7T8HZqnnI2FXJQ@tucnak> (raw)

Hi!

Seems in 2017 attribute-specifier-seq[opt] was added to asm-declaration
and the change was voted in as a DR.

The following patch implements it by parsing the attributes and warning
about them.

I found one attribute parsing bug I'll send a fix for momentarily.

And there is another thing I wonder about: with -Wno-attributes= we are
supposed to ignore the attributes altogether, but we are actually still
warning about them when we emit these generic warnings about ignoring
all attributes which appertain to this and that (perhaps with some
exceptions we first remove from the attribute chain), like:
void foo () { [[foo::bar]]; }
with -Wattributes -Wno-attributes=foo::bar
Shouldn't we call some helper function in cases like this and warn
not when std_attrs (or how the attribute chain var is called) is non-NULL,
but if it is non-NULL and contains at least one non-attribute_ignored_p
attribute?  cp_parser_declaration at least tries:
      if (std_attrs != NULL_TREE && !attribute_ignored_p (std_attrs))
        warning_at (make_location (attrs_loc, attrs_loc, parser->lexer),
                    OPT_Wattributes, "attribute ignored");
but attribute_ignored_p here checks the first attribute rather than the
whole chain.  So it will incorrectly not warn if there is an ignored
attribute followed by non-ignored.

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

2023-12-05  Jakub Jelinek  <jakub@redhat.com>

	PR c++/110734
	* parser.cc (cp_parser_block_declaration): Implement C++ DR 2262
	- Attributes for asm-definition.  Call cp_parser_asm_definition
	even if RID_ASM token is only seen after sequence of standard
	attributes.
	(cp_parser_asm_definition): Parse standard attributes before
	RID_ASM token and warn for them with -Wattributes.

	* g++.dg/DRs/dr2262.C: New test.
	* g++.dg/cpp0x/gen-attrs-76.C (foo, bar): Don't expect errors
	on attributes on asm definitions.
	* g++.dg/gomp/attrs-11.C: Remove 2 expected errors.

--- gcc/cp/parser.cc.jj	2023-12-04 08:59:06.871357329 +0100
+++ gcc/cp/parser.cc	2023-12-04 20:23:53.225009856 +0100
@@ -15398,7 +15398,6 @@ cp_parser_block_declaration (cp_parser *
   /* Peek at the next token to figure out which kind of declaration is
      present.  */
   cp_token *token1 = cp_lexer_peek_token (parser->lexer);
-  size_t attr_idx;
 
   /* If the next keyword is `asm', we have an asm-definition.  */
   if (token1->keyword == RID_ASM)
@@ -15452,22 +15451,36 @@ cp_parser_block_declaration (cp_parser *
   /* If the next token is `static_assert' we have a static assertion.  */
   else if (token1->keyword == RID_STATIC_ASSERT)
     cp_parser_static_assert (parser, /*member_p=*/false);
-  /* If the next tokens after attributes is `using namespace', then we have
-     a using-directive.  */
-  else if ((attr_idx = cp_parser_skip_std_attribute_spec_seq (parser, 1)) != 1
-	   && cp_lexer_nth_token_is_keyword (parser->lexer, attr_idx,
-					     RID_USING)
-	   && cp_lexer_nth_token_is_keyword (parser->lexer, attr_idx + 1,
-					     RID_NAMESPACE))
+  else
     {
-      if (statement_p)
-	cp_parser_commit_to_tentative_parse (parser);
-      cp_parser_using_directive (parser);
+      size_t attr_idx = cp_parser_skip_std_attribute_spec_seq (parser, 1);
+      cp_token *after_attr = NULL;
+      if (attr_idx != 1)
+	after_attr = cp_lexer_peek_nth_token (parser->lexer, attr_idx);
+      /* If the next tokens after attributes is `using namespace', then we have
+	 a using-directive.  */
+      if (after_attr
+	  && after_attr->keyword == RID_USING
+	  && cp_lexer_nth_token_is_keyword (parser->lexer, attr_idx + 1,
+					    RID_NAMESPACE))
+	{
+	  if (statement_p)
+	    cp_parser_commit_to_tentative_parse (parser);
+	  cp_parser_using_directive (parser);
+	}
+      /* If the next token after attributes is `asm', then we have
+	 an asm-definition.  */
+      else if (after_attr && after_attr->keyword == RID_ASM)
+	{
+	  if (statement_p)
+	    cp_parser_commit_to_tentative_parse (parser);
+	  cp_parser_asm_definition (parser);
+	}
+      /* Anything else must be a simple-declaration.  */
+      else
+	cp_parser_simple_declaration (parser, !statement_p,
+				      /*maybe_range_for_decl*/NULL);
     }
-  /* Anything else must be a simple-declaration.  */
-  else
-    cp_parser_simple_declaration (parser, !statement_p,
-				  /*maybe_range_for_decl*/NULL);
 }
 
 /* Parse a simple-declaration.
@@ -22424,6 +22437,7 @@ cp_parser_asm_definition (cp_parser* par
   bool invalid_inputs_p = false;
   bool invalid_outputs_p = false;
   required_token missing = RT_NONE;
+  tree std_attrs = cp_parser_std_attribute_spec_seq (parser);
   location_t asm_loc = cp_lexer_peek_token (parser->lexer)->location;
 
   /* Look for the `asm' keyword.  */
@@ -22657,6 +22671,10 @@ cp_parser_asm_definition (cp_parser* par
       else
 	symtab->finalize_toplevel_asm (string);
     }
+
+  if (std_attrs)
+    warning_at (asm_loc, OPT_Wattributes,
+		"attributes ignored on %<asm%> declaration");
 }
 
 /* Given the type TYPE of a declaration with declarator DECLARATOR, return the
--- gcc/testsuite/g++.dg/DRs/dr2262.C.jj	2023-12-04 19:58:06.433811915 +0100
+++ gcc/testsuite/g++.dg/DRs/dr2262.C	2023-12-04 20:23:01.655737020 +0100
@@ -0,0 +1,16 @@
+// DR 2262 - Attributes for asm-definition
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wattributes" }
+
+[[]] asm ("nop");
+[[foo::bar]] asm ("nop");	// { dg-warning "attributes ignored on 'asm' declaration" }
+
+void
+foo ()
+{
+  int i = 42;
+  [[]] asm ("nop");
+  [[foo::bar]] asm ("nop");	// { dg-warning "attributes ignored on 'asm' declaration" }
+  [[]] asm ("nop" : "+r" (i));
+  [[foo::bar]] [[bar::baz]] asm ("nop" : "+r" (i));	// { dg-warning "attributes ignored on 'asm' declaration" }
+}
--- gcc/testsuite/g++.dg/cpp0x/gen-attrs-76.C.jj	2021-08-12 09:34:16.094246634 +0200
+++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-76.C	2023-12-04 20:43:45.002188817 +0100
@@ -8,9 +8,9 @@ namespace P {}
 void
 foo ()
 {
-  [[]] asm ("");				// { dg-error "expected" }
+  [[]] asm ("");
   [[]] __extension__ asm ("");			// { dg-error "expected" }
-  __extension__ [[]] asm ("");			// { dg-error "expected" }
+  __extension__ [[]] asm ("");
   [[]] namespace M = ::N;			// { dg-error "expected" }
   [[]] using namespace N;			// { dg-bogus "expected" }
   using namespace P [[]];			// { dg-error "expected" }
@@ -22,9 +22,9 @@ foo ()
 void
 bar ()
 {
-  [[gnu::unused]] asm ("");			// { dg-error "expected" }
+  [[gnu::unused]] asm ("");
   [[gnu::unused]] __extension__ asm ("");	// { dg-error "expected" }
-  __extension__ [[gnu::unused]] asm ("");	// { dg-error "expected" }
+  __extension__ [[gnu::unused]] asm ("");
   [[gnu::unused]] namespace M = ::N;		// { dg-error "expected" }
   [[gnu::unused]] using namespace N;		// { dg-bogus "expected" }
   using namespace P [[gnu::unused]];		// { dg-error "expected" }
--- gcc/testsuite/g++.dg/gomp/attrs-11.C.jj	2021-08-12 09:34:16.720237822 +0200
+++ gcc/testsuite/g++.dg/gomp/attrs-11.C	2023-12-05 07:47:03.336641204 +0100
@@ -7,9 +7,9 @@ namespace O { typedef int T; };
 void
 foo ()
 {
-  [[omp::directive (parallel)]] asm ("");			// { dg-error "expected" }
+  [[omp::directive (parallel)]] asm ("");
   [[omp::directive (parallel)]] __extension__ asm ("");		// { dg-error "expected" }
-  __extension__ [[omp::directive (parallel)]] asm ("");		// { dg-error "expected" }
+  __extension__ [[omp::directive (parallel)]] asm ("");
   [[omp::directive (parallel)]] namespace M = ::N;		// { dg-error "expected" }
   [[omp::directive (parallel)]] using namespace N;		// { dg-error "not allowed to be specified in this context" }
   [[omp::directive (parallel)]] using O::T;			// { dg-error "expected" }

	Jakub


             reply	other threads:[~2023-12-05  7:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-05  7:40 Jakub Jelinek [this message]
2023-12-05 16:01 ` Jason Merrill
2023-12-06 14:10   ` [PATCH] c++: Don't diagnose ignoring of attributes if all ignored attributes are attribute_ignored_p Jakub Jelinek
2023-12-07  7:47     ` Jakub Jelinek
2023-12-08 17:06     ` Jason Merrill
2023-12-08 17:53       ` [PATCH] c++, v2: " Jakub Jelinek
2023-12-08 19:48         ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZW7T8HZqnnI2FXJQ@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    --cc=polacek@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).