[clang] Allow extra semicolons inside classes also in C++ pedantic mode. #172209
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With DR 1693 and DR 3079 C++ also allows empty declaration, i.e. extra semicolons, inside class member declaration lists in all contexts. This removes warnings for such extra semicolons from -pedantic but keeps them as a C++98 compatibility warning as well as -Wextra-semi, analogously to extra semicolons at namespace scope. fixes llvm#155538
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang Author: None (keinflue) ChangesWith DR 1693 and DR 3079 C++ also allows empty declaration, i.e. extra semicolons, inside class member declaration lists in all contexts (as far as I can tell). This removes warnings for such extra semicolons from -pedantic but keeps them as a C++98 compatibility warning as well as -Wextra-semi, analogously to extra semicolons at namespace scope. fixes #155538 Full diff: https://github.com/llvm/llvm-project/pull/172209.diff 6 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index feaf92ad4415f..1578713943665 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -211,6 +211,10 @@ C++17 Feature Support
Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Implement DR1693 and DR3079 by allowing extra semicolons inside class member
+ declaration lists even in -pedantic mode. The warnings are still available
+ with -Wextra-semi. (#GH155538)
+
C Language Changes
------------------
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 442a90ec2472d..9ce9748762b65 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -40,8 +40,12 @@ let CategoryName = "Parse Issue" in {
def ext_empty_translation_unit : Extension<
"ISO C requires a translation unit to contain at least one declaration">,
InGroup<DiagGroup<"empty-translation-unit">>;
-def warn_cxx98_compat_top_level_semi : Warning<
- "extra ';' outside of a function is incompatible with C++98">,
+def warn_cxx98_compat_extra_semi : Warning<
+ "%select{|||multiple }0extra ';' %select{"
+ "outside of a function|"
+ "inside a %1|"
+ "inside instance variable list|"
+ "after member function definition}0 is incompatible with C++98">,
InGroup<CXX98CompatExtraSemi>, DefaultIgnore;
def ext_extra_semi : Extension<
"extra ';' %select{"
@@ -51,7 +55,11 @@ def ext_extra_semi : Extension<
"after member function definition}0">,
InGroup<ExtraSemi>;
def ext_extra_semi_cxx11 : Extension<
- "extra ';' outside of a function is a C++11 extension">,
+ "%select{|||multiple }0extra ';' %select{"
+ "outside of a function|"
+ "inside a %1|"
+ "inside instance variable list|"
+ "after member function definition}0 is a C++11 extension">,
InGroup<CXX11ExtraSemi>;
def warn_extra_semi_after_mem_fn_def : Warning<
"extra ';' after member function definition">,
diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp
index 7b425dd3dda43..6325238d6ef75 100644
--- a/clang/lib/Parse/Parser.cpp
+++ b/clang/lib/Parse/Parser.cpp
@@ -205,27 +205,40 @@ void Parser::ConsumeExtraSemi(ExtraSemiKind Kind, DeclSpec::TST TST) {
ConsumeToken();
}
+ if (Kind == ExtraSemiKind::AfterMemberFunctionDefinition &&
+ !HadMultipleSemis) {
+ // A single semicolon is valid after a member function definition.
+ Diag(StartLoc, diag::warn_extra_semi_after_mem_fn_def)
+ << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
+ return;
+ }
+
// C++11 allows extra semicolons at namespace scope, but not in any of the
// other contexts.
- if (Kind == ExtraSemiKind::OutsideFunction && getLangOpts().CPlusPlus) {
+ // DR 1693 and DR 3079 extend this to class scope as well.
+ if ((Kind == ExtraSemiKind::OutsideFunction ||
+ Kind == ExtraSemiKind::InsideStruct ||
+ Kind == ExtraSemiKind::AfterMemberFunctionDefinition) &&
+ getLangOpts().CPlusPlus) {
if (getLangOpts().CPlusPlus11)
- Diag(StartLoc, diag::warn_cxx98_compat_top_level_semi)
+ Diag(StartLoc, diag::warn_cxx98_compat_extra_semi)
+ << Kind
+ << DeclSpec::getSpecifierName(
+ TST, Actions.getASTContext().getPrintingPolicy())
<< FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
else
Diag(StartLoc, diag::ext_extra_semi_cxx11)
+ << Kind
+ << DeclSpec::getSpecifierName(
+ TST, Actions.getASTContext().getPrintingPolicy())
<< FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
return;
}
- if (Kind != ExtraSemiKind::AfterMemberFunctionDefinition || HadMultipleSemis)
- Diag(StartLoc, diag::ext_extra_semi)
- << Kind
- << DeclSpec::getSpecifierName(
- TST, Actions.getASTContext().getPrintingPolicy())
- << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
- else
- // A single semicolon is valid after a member function definition.
- Diag(StartLoc, diag::warn_extra_semi_after_mem_fn_def)
+ Diag(StartLoc, diag::ext_extra_semi)
+ << Kind
+ << DeclSpec::getSpecifierName(TST,
+ Actions.getASTContext().getPrintingPolicy())
<< FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
}
diff --git a/clang/test/Parser/cxx-class.cpp b/clang/test/Parser/cxx-class.cpp
index a6757f39146a1..44665026c4fa0 100644
--- a/clang/test/Parser/cxx-class.cpp
+++ b/clang/test/Parser/cxx-class.cpp
@@ -20,11 +20,16 @@ class C {
; // ok, one extra ';' is permitted
void m() {
int l = 2;
- };; // expected-warning{{extra ';' after member function definition}}
-
+ };;
+#if __cplusplus < 201103L
+ // expected-warning@-2{{extra ';' after member function definition is a C++11 extension}}
+#endif
template<typename T> void mt(T) { }
;
- ; // expected-warning{{extra ';' inside a class}}
+ ;
+#if __cplusplus < 201103L
+ // expected-warning@-2{{extra ';' inside a class is a C++11 extension}}
+#endif
virtual int vf() const volatile = 0;
diff --git a/clang/test/Parser/cxx-extra-semi.cpp b/clang/test/Parser/cxx-extra-semi.cpp
index de14bc0354d80..9bd69e18e9f02 100644
--- a/clang/test/Parser/cxx-extra-semi.cpp
+++ b/clang/test/Parser/cxx-extra-semi.cpp
@@ -5,6 +5,40 @@
// RUN: %clang_cc1 -x c++ -Wextra-semi -fixit %t
// RUN: %clang_cc1 -x c++ -Wextra-semi -Werror %t
+#if __cplusplus >= 201103L && defined(PEDANTIC)
+// In C++11 extra semicolons inside classes are allowed via defect reports.
+// expected-no-diagnostics
+class A {
+ void A1();
+ void A2() { };
+ void A2b() { };;
+ ;
+ void A2c() { }
+ ;
+ void A3() { }; ;;
+ ;;;;;;;
+ ;
+ ; ;; ; ;;;
+ ; ; ; ; ;;
+ void A4();
+
+ union {
+ ;
+ int a;
+ ;
+ };
+};
+
+union B {
+ int a1;
+ int a2;;
+};
+
+;
+; ;;
+
+#else
+
class A {
void A1();
void A2() { };
@@ -13,19 +47,25 @@ class A {
// -pedantic is specified, since one semicolon is technically permitted.
// expected-warning@-4{{extra ';' after member function definition}}
#endif
- void A2b() { };; // expected-warning{{extra ';' after member function definition}}
+ void A2b() { };; // expected-warning{{multiple extra ';' after member function definition}}
; // expected-warning{{extra ';' inside a class}}
void A2c() { }
;
#ifndef PEDANTIC
// expected-warning@-2{{extra ';' after member function definition}}
#endif
- void A3() { }; ;; // expected-warning{{extra ';' after member function definition}}
+ void A3() { }; ;; // expected-warning{{multiple extra ';' after member function definition}}
;;;;;;; // expected-warning{{extra ';' inside a class}}
; // expected-warning{{extra ';' inside a class}}
; ;; ; ;;; // expected-warning{{extra ';' inside a class}}
; ; ; ; ;; // expected-warning{{extra ';' inside a class}}
void A4();
+
+ union {
+ ; // expected-warning{{extra ';' inside a union}}
+ int a;
+ ; // expected-warning{{extra ';' inside a union}}
+ };
};
union B {
@@ -42,3 +82,5 @@ union B {
// expected-warning@-6{{extra ';' outside of a function is incompatible with C++98}}
// expected-warning@-6{{extra ';' outside of a function is incompatible with C++98}}
#endif
+
+#endif
diff --git a/clang/test/Parser/cxx0x-decl.cpp b/clang/test/Parser/cxx0x-decl.cpp
index 69a8d8a46557d..fd56c487ecdfa 100644
--- a/clang/test/Parser/cxx0x-decl.cpp
+++ b/clang/test/Parser/cxx0x-decl.cpp
@@ -62,15 +62,6 @@ namespace OpaqueEnumDecl {
int decltype(f())::*ptr_mem_decltype;
-class ExtraSemiAfterMemFn {
- // Due to a peculiarity in the C++11 grammar, a deleted or defaulted function
- // is permitted to be followed by either one or two semicolons.
- void f() = delete // expected-error {{expected ';' after delete}}
- void g() = delete; // ok
- void h() = delete;; // ok
- void i() = delete;;; // expected-error {{extra ';' after member function definition}}
-};
-
int *const const p = 0; // expected-error {{duplicate 'const' declaration specifier}}
const const int *q = 0; // expected-error {{duplicate 'const' declaration specifier}}
|
Contributor
|
Please add tests to "clang/test/CXX/drs/", then run "clang/www/make_cxx_dr_status" to update "clang/www/cxx_dr_status.html". |
Contributor
Author
|
Added the test cases, however the DR status page doesn't seem to generate with an error message: I am not sure whether I can just change the marking. From looking at the issue it is not clear to me that the test cases are still appropriate. |
cor3ntin
reviewed
Dec 15, 2025
Contributor
cor3ntin
left a comment
cor3ntin
left a comment
There was a problem hiding this comment.
Given that the change was made in a core issue - therefore allowing us to backport it, my inclination would be to remove the compatibility warnings all together and just emit a non-extension warning in all language modes. I suspect it would be more useful to users
WDYT @shafik @erichkeane ?
Comment on lines
+224
to
+227
| Diag(StartLoc, diag::warn_cxx98_compat_extra_semi) | ||
| << Kind | ||
| << DeclSpec::getSpecifierName( | ||
| TST, Actions.getASTContext().getPrintingPolicy()) |
Member
There was a problem hiding this comment.
The warning text is the same for both diagnostics here so that should work; in that case, the diagnostic definition in the TD file needs to be updated to use CXX11Compat (@keinflue grep for that if you’re unsure how to use it)
Contributor
Author
There was a problem hiding this comment.
Is there a way to preserve the semantics of -Wc++11-extra-semi and -Wc++98-compat-extra-semi with this or what should happen with these flags?
Also, I think with -Wextra-semi given explicitly by the user, there should still be warnings. However, the diagnostic ext_extra_semi is currently an Extension (instead of Warning) for semantics in C. Is there a way to reuse it for this purpose or do I need to duplicate it again either way?
Member
There was a problem hiding this comment.
Is there a way to preserve the semantics of
-Wc++11-extra-semiand-Wc++98-compat-extra-semiwith this or what should happen with these flags?
Of course there are separate diagnostic groups for these... no in that case you can’t use CXXCompat for this; that might be the the reason why I didn’t migrate these warnings to use that
Sirraide
reviewed
Dec 15, 2025
| int a1; | ||
| int a2;; | ||
| }; | ||
|
|
Member
There was a problem hiding this comment.
Can you also add some tests w/ =default/=delete/=0 here
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With DR 1693 and DR 3079 C++ also allows empty declaration, i.e. extra semicolons, inside class member declaration lists in all contexts (as far as I can tell).
This removes warnings for such extra semicolons from -pedantic but keeps them as a C++98 compatibility warning as well as -Wextra-semi, analogously to extra semicolons at namespace scope.
fixes #155538