On 5/9/24 12:16, Marek Polacek wrote:
In GCC 14, I submitted a minimal fix for the "extra ; warning", pushed
in r14-8967. That patch mentions a more complete patch for GCC 15.
This is the patch.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
Prompted by c++/113760, I started looking into a bogus "extra ;"
warning in C++11. It quickly turned out that if I want to fix
this for good, the fix will not be so small.
This patch touches on DR 569, an extra ; at namespace scope should
be allowed since C++11:
struct S {
};
; // pedwarn in C++98
It also touches on DR 1693, which allows superfluous semicolons in
class definitions since C++11:
struct S {
int a;
; // pedwarn in C++98
};
Note that a single semicolon is valid after a member function definition:
struct S {
void foo () {}; // only warns with -Wextra-semi
};
There's a new function maybe_warn_extra_semi to handle all of the above
in a single place. So now they all get a fix-it hint.
-Wextra-semi turns on all "extra ;" diagnostics. Currently, options
like -Wc++11-compat or -Wc++11-extensions are not considered.
DR 1693
PR c++/113760
DR 569
gcc/c-family/ChangeLog:
* c.opt (Wextra-semi): Initialize to -1.
gcc/cp/ChangeLog:
* parser.cc (extra_semi_kind): New.
(maybe_warn_extra_semi): New.
(cp_parser_declaration): Call maybe_warn_extra_semi.
(cp_parser_member_declaration): Likewise.
gcc/ChangeLog:
* doc/invoke.texi: Update -Wextra-semi documentation.
gcc/testsuite/ChangeLog:
* g++.dg/diagnostic/semicolon1.C: New test.
* g++.dg/diagnostic/semicolon10.C: New test.
* g++.dg/diagnostic/semicolon11.C: New test.
* g++.dg/diagnostic/semicolon12.C: New test.
* g++.dg/diagnostic/semicolon13.C: New test.
* g++.dg/diagnostic/semicolon14.C: New test.
* g++.dg/diagnostic/semicolon15.C: New test.
* g++.dg/diagnostic/semicolon16.C: New test.
* g++.dg/diagnostic/semicolon17.C: New test.
* g++.dg/diagnostic/semicolon2.C: New test.
* g++.dg/diagnostic/semicolon3.C: New test.
* g++.dg/diagnostic/semicolon4.C: New test.
* g++.dg/diagnostic/semicolon5.C: New test.
* g++.dg/diagnostic/semicolon6.C: New test.
* g++.dg/diagnostic/semicolon7.C: New test.
* g++.dg/diagnostic/semicolon8.C: New test.
* g++.dg/diagnostic/semicolon9.C: New test.
---
gcc/c-family/c.opt | 2 +-
gcc/cp/parser.cc | 94 +++++++++++++++----
gcc/doc/invoke.texi | 29 +++++-
gcc/testsuite/g++.dg/diagnostic/semicolon1.C | 18 ++++
gcc/testsuite/g++.dg/diagnostic/semicolon10.C | 11 +++
gcc/testsuite/g++.dg/diagnostic/semicolon11.C | 29 ++++++
gcc/testsuite/g++.dg/diagnostic/semicolon12.C | 29 ++++++
gcc/testsuite/g++.dg/diagnostic/semicolon13.C | 29 ++++++
gcc/testsuite/g++.dg/diagnostic/semicolon14.C | 29 ++++++
gcc/testsuite/g++.dg/diagnostic/semicolon15.C | 29 ++++++
gcc/testsuite/g++.dg/diagnostic/semicolon16.C | 29 ++++++
gcc/testsuite/g++.dg/diagnostic/semicolon17.C | 29 ++++++
gcc/testsuite/g++.dg/diagnostic/semicolon2.C | 18 ++++
gcc/testsuite/g++.dg/diagnostic/semicolon3.C | 18 ++++
gcc/testsuite/g++.dg/diagnostic/semicolon4.C | 18 ++++
gcc/testsuite/g++.dg/diagnostic/semicolon5.C | 18 ++++
gcc/testsuite/g++.dg/diagnostic/semicolon6.C | 18 ++++
gcc/testsuite/g++.dg/diagnostic/semicolon7.C | 18 ++++
gcc/testsuite/g++.dg/diagnostic/semicolon8.C | 11 +++
gcc/testsuite/g++.dg/diagnostic/semicolon9.C | 11 +++
20 files changed, 468 insertions(+), 19 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon1.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon10.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon11.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon12.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon13.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon14.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon15.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon16.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon17.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon2.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon3.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon4.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon5.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon6.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon7.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon8.C
create mode 100644 gcc/testsuite/g++.dg/diagnostic/semicolon9.C
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 403abc1f26e..fb34c3b7031 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -727,7 +727,7 @@ C ObjC C++ ObjC++ Warning
; in common.opt
Wextra-semi
-C++ ObjC++ Var(warn_extra_semi) Warning
+C++ ObjC++ Var(warn_extra_semi) Init(-1) Warning
Warn about semicolon after in-class function definition.
Wflex-array-member-not-at-end
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 7306ce9a8a8..d896e025198 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -15331,6 +15331,61 @@ cp_parser_module_export (cp_parser *parser)
module_kind = mk;
}
+/* Used for maybe_warn_extra_semi. */
+
+enum class extra_semi_kind { decl, member, in_class_fn_def };
+
+/* Warn about an extra semicolon. KIND says in which context the extra
+ semicolon occurs. */
+
+static void
+maybe_warn_extra_semi (location_t loc, extra_semi_kind kind)
+{
+ /* -Wno-extra-semi suppresses all. */
+ if (warn_extra_semi == 0)
+ return;
+
+ gcc_rich_location richloc (loc);
+ richloc.add_fixit_remove ();
+
+ switch (kind)
+ {
+ case extra_semi_kind::decl:
+ if (warn_extra_semi > 0)
+ warning_at (&richloc, OPT_Wextra_semi,
+ "extra %<;%> outside of a function");
+ /* If -Wextra-semi wasn't specified, warn only when -pedantic is in
+ effect in C++98. DR 569 says that spurious semicolons at namespace
+ scope should be allowed. */
+ else if (pedantic && cxx_dialect < cxx11)
+ pedwarn (&richloc, OPT_Wextra_semi,
+ "extra %<;%> outside of a function only allowed in C++11");
Shouldn't the pedwarn case come first?
+ break;
+
+ case extra_semi_kind::member:
+ if (warn_extra_semi > 0)
+ warning_at (&richloc, OPT_Wextra_semi, "extra %<;%> inside a struct");
+ /* If -Wextra-semi wasn't specified, warn only when -pedantic is in
+ effect in C++98. DR 1693 added "empty-declaration" to the syntax for
+ "member-declaration". */
+ else if (pedantic && cxx_dialect < cxx11)
+ pedwarn (&richloc, OPT_Wextra_semi,
+ "extra %<;%> inside a struct only allowed in C++11");
And here.
+ break;
+
+ case extra_semi_kind::in_class_fn_def:
+ /* A single semicolon is valid after a member function definition
+ so this is just a warning. */
+ if (warn_extra_semi > 0)
+ warning_at (&richloc, OPT_Wextra_semi,
+ "extra %<;%> after in-class function definition");
+ break;
+
+ default:
+ gcc_unreachable ();
+ }
+}
+
/* Declarations [gram.dcl.dcl] */
/* Parse an optional declaration-sequence. TOP_LEVEL is true, if this
@@ -15430,11 +15485,11 @@ cp_parser_declaration (cp_parser* parser, tree
prefix_attrs)
if (token1->type == CPP_SEMICOLON)
{
- cp_lexer_consume_token (parser->lexer);
+ location_t semicolon_loc
+ = cp_lexer_consume_token (parser->lexer)->location;
/* A declaration consisting of a single semicolon is invalid
- * before C++11. Allow it unless we're being pedantic. */
- if (cxx_dialect < cxx11)
- pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
+ before C++11. Allow it unless we're being pedantic. */
+ maybe_warn_extra_semi (semicolon_loc, extra_semi_kind::decl);
return;
}
else if (cp_lexer_nth_token_is (parser->lexer,
@@ -28121,19 +28176,27 @@ cp_parser_member_declaration (cp_parser* parser)
struct S { ; };
- [class.mem]
+ [class.mem] used to say
Each member-declaration shall declare at least one member
- name of the class. */
+ name of the class.
+
+ but since DR 1693:
+
+ A member-declaration does not declare new members of the class
+ if it is
+ -- [...]
+ -- an empty-declaration.
+ For any other member-declaration, each declared entity that is not
+ an unnamed bit-field is a member of the class, and each such
+ member-declaration shall either declare at least one member name of
+ the class or declare at least one unnamed bit-field. */
if (!decl_specifiers.any_specifiers_p)
{
cp_token *token = cp_lexer_peek_token (parser->lexer);
- if (cxx_dialect < cxx11 && !in_system_header_at (token->location))
- {
- gcc_rich_location richloc (token->location);
- richloc.add_fixit_remove ();
- pedwarn (&richloc, OPT_Wpedantic, "extra %<;%>");
- }
+ // ??? Should we check !in_system_header_at in maybe_warn_extra_semi?
The system header check seems to date back to PR 12479. Apparently it
has not ever proved necessary for the other cases.
Furthermore, all pedwarns are now ignored in system headers unless
-Wsystem-headers is specified, in which case it would seem useful to
report this.
So, my inclination is to drop the system header check entirely at this
point.
+ if (!in_system_header_at (token->location))
+ maybe_warn_extra_semi (token->location, extra_semi_kind::member);
Jason
