Re: [C++ PATCH] empty-declarations

[David Malcolm]

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 
> > 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

Re: [C++ PATCH] empty-declarations

[Nathan Sidwell]

On 04/10/2018 12:43 PM, Jason Merrill wrote:

On Tue, Apr 10, 2018 at 12:32 PM, Nathan Sidwell  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

--
Nathan Sidwell

Re: [C++ PATCH] empty-declarations

[Jason Merrill]

On Tue, Apr 10, 2018 at 12:32 PM, Nathan Sidwell  wrote:
> On 04/10/2018 12:00 PM, Jason Merrill wrote:
>>
>> On Tue, Apr 10, 2018 at 11:06 AM, Nathan Sidwell  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

Re: [C++ PATCH] empty-declarations

[Nathan Sidwell]

On 04/10/2018 12:00 PM, Jason Merrill wrote:

On Tue, Apr 10, 2018 at 11:06 AM, Nathan Sidwell  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

--
Nathan Sidwell

Re: [C++ PATCH] empty-declarations

[Jason Merrill]

On Tue, Apr 10, 2018 at 11:06 AM, Nathan Sidwell  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

[C++ PATCH] empty-declarations

[Nathan Sidwell]

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
--
Nathan Sidwell
2018-04-10  Nathan Sidwell  

	* 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