[C++] empty-declarations

Message ID 2d7e797e-7fb8-5ec0-b3b4-026d306817f8@acm.org
State New
Headers show
Series
  • [C++] empty-declarations
Related show

Commit Message

Nathan Sidwell April 10, 2018, 3:06 p.m.
Jason,
thanks for looking up about empty-decls.  This patch implements the 
C++11 change.

At namespace-scope & class-scope we're now silent about empty 
declarations, when not in C++-98 mode.

The class-scope change was a little more invasive, because we silently 
accepted a semicolon after an in-class function definition:

class X {
   void foo () {};  // semi-colon accepted
};

And we emitted a fixit for that case.

This isn't a regression.  I don't think anyone's complained.  Jason, 
WDYT about committing this now?

nathan

Comments

Jason Merrill April 10, 2018, 4 p.m. | #1
On Tue, Apr 10, 2018 at 11:06 AM, Nathan Sidwell <nathan@acm.org> wrote:
> Jason,
> thanks for looking up about empty-decls.  This patch implements the C++11
> change.
>
> At namespace-scope & class-scope we're now silent about empty declarations,
> when not in C++-98 mode.
>
> The class-scope change was a little more invasive, because we silently
> accepted a semicolon after an in-class function definition:
>
> class X {
>   void foo () {};  // semi-colon accepted
> };
>
> And we emitted a fixit for that case.

This is the -Wextra-semi warning that Volker added recently, and your
patch would break.

Do we want -Wextra-semi to now warn about all empty declarations at
class/namespace scope, not just after a function definition?

> This isn't a regression.  I don't think anyone's complained.  Jason, WDYT
> about committing this now?

I'd hold it for stage 1.

Jason
Nathan Sidwell April 10, 2018, 4:32 p.m. | #2
On 04/10/2018 12:00 PM, Jason Merrill wrote:
> On Tue, Apr 10, 2018 at 11:06 AM, Nathan Sidwell <nathan@acm.org> wrote:

> This is the -Wextra-semi warning that Volker added recently, and your
> patch would break.
> 
> Do we want -Wextra-semi to now warn about all empty declarations at
> class/namespace scope, not just after a function definition?

I don't see why we should warn at all.

>> This isn't a regression.  I don't think anyone's complained.  Jason, WDYT
>> about committing this now?
> 
> I'd hold it for stage 1.

works for me.

nathan
Jason Merrill April 10, 2018, 4:43 p.m. | #3
On Tue, Apr 10, 2018 at 12:32 PM, Nathan Sidwell <nathan@acm.org> wrote:
> On 04/10/2018 12:00 PM, Jason Merrill wrote:
>>
>> On Tue, Apr 10, 2018 at 11:06 AM, Nathan Sidwell <nathan@acm.org> wrote:
>
>
>> This is the -Wextra-semi warning that Volker added recently, and your
>> patch would break.
>>
>> Do we want -Wextra-semi to now warn about all empty declarations at
>> class/namespace scope, not just after a function definition?
>
> I don't see why we should warn at all.

It's not on by default, but some people want to get a warning about
redundant semicolons.  Clang also has this flag.

Jason
Nathan Sidwell April 10, 2018, 5:47 p.m. | #4
On 04/10/2018 12:43 PM, Jason Merrill wrote:
> On Tue, Apr 10, 2018 at 12:32 PM, Nathan Sidwell <nathan@acm.org> wrote:
>> On 04/10/2018 12:00 PM, Jason Merrill wrote:

>> I don't see why we should warn at all.
> 
> It's not on by default, but some people want to get a warning about
> redundant semicolons.  Clang also has this flag.

OK, Then I presume it should warn about all such semicolons.

nathan
David Malcolm April 10, 2018, 6:56 p.m. | #5
On Tue, 2018-04-10 at 13:47 -0400, Nathan Sidwell wrote:
> On 04/10/2018 12:43 PM, Jason Merrill wrote:
> > On Tue, Apr 10, 2018 at 12:32 PM, Nathan Sidwell <nathan@acm.org>
> > wrote:
> > > On 04/10/2018 12:00 PM, Jason Merrill wrote:
> > > I don't see why we should warn at all.
> > 
> > It's not on by default, but some people want to get a warning about
> > redundant semicolons.  Clang also has this flag.
> 
> OK, Then I presume it should warn about all such semicolons.

...and presumably each such warning should have a fix-it hint, removing
the redundant token?

Dave

Patch

2018-04-10  Nathan Sidwell  <nathan@acm.org>

	* parser.c (cp_parser_declaration_seq_opt): Permit
	empty-declaration in c++11.
	(cp_parser_member_declaration): Likewise.

	* g++.dg/semicolon-fixits.C: Delete.
	* g++.dg/cpp0x/semicolon.C: New.
	* g++.dg/warn/Wextra-semi.C: C++98 only.
	* g++.dg/warn/pedantic2.C: C++98 only.
	* g++.old-deja/g++.bugs/900404_04.C: C++98 only.
	* g++.old-deja/g++.jason/parse11.C: C++98 only.
	* g++.old-deja/g++.law/missed-error2.C: C++98 only.

Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 259268)
+++ cp/parser.c	(working copy)
@@ -12614,9 +12614,9 @@  cp_parser_declaration_seq_opt (cp_parser
       if (token->type == CPP_SEMICOLON)
 	{
 	  /* A declaration consisting of a single semicolon is
-	     invalid.  Allow it unless we're being pedantic.  */
+	     valid since C++11.  */
 	  cp_lexer_consume_token (parser->lexer);
-	  if (!in_system_header_at (input_location))
+	  if (cxx_dialect < cxx11 && !in_system_header_at (input_location))
 	    pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
 	  continue;
 	}
@@ -23445,6 +23445,7 @@  cp_parser_member_specification_opt (cp_p
    C++0x Extensions:
 
    member-declaration:
+     empty-declaration
      static_assert-declaration  */
 
 static void
@@ -23472,6 +23473,15 @@  cp_parser_member_declaration (cp_parser*
       return;
     }
 
+  if (cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON))
+    {
+      /* C++11 allow an empty-declaration.  */
+      cp_lexer_consume_token (parser->lexer);
+      if (cxx_dialect < cxx11 && !in_system_header_at (input_location))
+	pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
+      return;
+    }
+
   /* Check for a template-declaration.  */
   if (cp_lexer_next_token_is_keyword (parser->lexer, RID_TEMPLATE))
     {
@@ -23912,8 +23922,9 @@  cp_parser_member_declaration (cp_parser*
 		    finish_member_declaration (decl);
 		  /* Peek at the next token.  */
 		  token = cp_lexer_peek_token (parser->lexer);
-		  /* If the next token is a semicolon, consume it.  */
-		  if (token->type == CPP_SEMICOLON)
+		  /* If the next token is a semicolon, consume it in
+		     c++98.  c++11 made this always ok.  */
+		  if (cxx_dialect < cxx11 && token->type == CPP_SEMICOLON)
 		    {
 		      location_t semicolon_loc
 			= cp_lexer_consume_token (parser->lexer)->location;
Index: testsuite/g++.dg/cpp0x/semicolon.C
===================================================================
--- testsuite/g++.dg/cpp0x/semicolon.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/semicolon.C	(working copy)
@@ -0,0 +1,10 @@ 
+// { dg-do compile { target c++11 } }
+// C++11 allows empty decls
+
+; // OK
+
+
+struct X 
+{
+  ; // OK
+};
Index: testsuite/g++.dg/semicolon-fixits.C
===================================================================
--- testsuite/g++.dg/semicolon-fixits.C	(revision 259268)
+++ testsuite/g++.dg/semicolon-fixits.C	(working copy)
@@ -1,17 +0,0 @@ 
-/* { dg-options "-fdiagnostics-show-caret -Wpedantic" } */
-
-/* Struct with extra semicolon.  */
-struct s1 { int i;; }; /* { dg-warning "19: extra .;." } */
-/* { dg-begin-multiline-output "" }
- struct s1 { int i;; };
-                   ^
-                   -
-   { dg-end-multiline-output "" } */
-
-/* Union with extra semicolon.  */
-union u1 { int i;; }; /* { dg-warning "18: extra .;." } */
-/* { dg-begin-multiline-output "" }
- union u1 { int i;; };
-                  ^
-                  -
-   { dg-end-multiline-output "" } */
Index: testsuite/g++.dg/warn/Wextra-semi.C
===================================================================
--- testsuite/g++.dg/warn/Wextra-semi.C	(revision 259268)
+++ testsuite/g++.dg/warn/Wextra-semi.C	(working copy)
@@ -1,3 +1,5 @@ 
+// { dg-do compile { target c++98_only } }
+
 // { dg-options "-Wextra-semi -fdiagnostics-show-caret" }
 
 struct A
Index: testsuite/g++.dg/warn/pedantic2.C
===================================================================
--- testsuite/g++.dg/warn/pedantic2.C	(revision 259268)
+++ testsuite/g++.dg/warn/pedantic2.C	(working copy)
@@ -1,3 +1,4 @@ 
+// { dg-do compile { target c++98_only } }
 // { dg-options "-pedantic" }
 
 class foo
Index: testsuite/g++.old-deja/g++.bugs/900404_04.C
===================================================================
--- testsuite/g++.old-deja/g++.bugs/900404_04.C	(revision 259268)
+++ testsuite/g++.old-deja/g++.bugs/900404_04.C	(working copy)
@@ -1,4 +1,4 @@ 
-// { dg-do assemble  }
+// { dg-do assemble { target c++98_only } }
 // g++ 1.37.1 bug 900404_04
 
 // [dcl.dcl] explains that simple-declarations may omit the
@@ -13,6 +13,7 @@ 
 
 int i;
 
+// Ok in C++11 up
 ;			// { dg-error "extra ';'" } 
 
 int main () { return 0; }
Index: testsuite/g++.old-deja/g++.jason/parse11.C
===================================================================
--- testsuite/g++.old-deja/g++.jason/parse11.C	(revision 259268)
+++ testsuite/g++.old-deja/g++.jason/parse11.C	(working copy)
@@ -1,4 +1,4 @@ 
-// { dg-do assemble  }
+// { dg-do assemble  { target c++98_only } }
 // PRMS Id: 6825
 
 class aClass 
Index: testsuite/g++.old-deja/g++.law/missed-error2.C
===================================================================
--- testsuite/g++.old-deja/g++.law/missed-error2.C	(revision 259268)
+++ testsuite/g++.old-deja/g++.law/missed-error2.C	(working copy)
@@ -1,10 +1,11 @@ 
-// { dg-do assemble  }
+// { dg-do assemble  { target c++98_only } }
 // GROUPS passed missed-error
 // missed-error file
 // From: ndc!don@csvax.cs.caltech.edu (Don Erway)
 // Date:     Thu, 21 May 92 15:40:45 PDT
 // Subject:  More on [g++ 2.1 : overloaded function selection incorrect]
 // Message-ID: <9205212240.AA17934@ndc.com>
+// naked semi-colons ok in C++11
 
 #include <iostream>