-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[clang] Allow extra semicolons inside classes also in C++ pedantic mode. #172209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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|" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a test for this diagnostic line. |
||
| "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|" | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a test for this diagnostic line. |
||
| "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">, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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()) | ||
|
Comment on lines
+224
to
+227
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a way to preserve the semantics of Also, I think with
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Of course there are separate diagnostic groups for these... no in that case you can’t use |
||
| << 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)); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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;; | ||
| }; | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you also add some tests w/ |
||
| ; | ||
| ; ;; | ||
|
|
||
| #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 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a test for this diagnostic line.