lebedev.ri created this revision.
lebedev.ri added reviewers: rsmith, rtrieu, aaron.ballman.
lebedev.ri added a project: clang.
Let's suppose the `-Weverything` is passed.
Given code like
void F() {}
;
If the code is compiled with `-std=c++03`, it would diagnose that extra sema:
<source>:2:1: warning: extra ';' outside of a function is a C++11 extension
[-Wc++11-extra-semi]
;
^~
If the code is compiled with `-std=c++11`, it also would diagnose that extra
sema:
<source>:2:1: warning: extra ';' outside of a function is incompatible with
C++98 [-Wc++98-compat-pedantic]
;
^~
But, let's suppose the C++11 or higher is used, and the used does not care
about `-Wc++98-compat-pedantic`, so he disables that diagnostic.
And that silences the complaint about extra `;` too.
And there is no way to re-enable that particular diagnostic, passing
`-Wextra-semi` does nothing...
Now, there is also a related `no newline at end of file` diagnostic, which is
also emitted by `-Wc++98-compat-pedantic`
<source>:2:2: warning: C++98 requires newline at end of file
[-Wc++98-compat-pedantic]
;
^
But unlike the previous case, if `-Wno-c++98-compat-pedantic` is passed, that
diagnostic stays displayed:
<source>:2:2: warning: no newline at end of file [-Wnewline-eof]
;
^
This diff adds `warn_extra_semi` (under `-Wextra-semi`), which allows to
diagnose
that extra semi even if it is not invalid, and the `-Wc++98-compat-pedantic` is
disabled.
This does not answer the questionability of that new warning.
It is **disabled by default**.
This simply allows to diagnose it.
Testing: `$ ninja check-clang-sema check-clang-semacxx check-clang-parser`
Thoughts?
Repository:
rC Clang
https://reviews.llvm.org/D43162
Files:
include/clang/Basic/DiagnosticParseKinds.td
lib/Parse/Parser.cpp
test/Parser/cxx-extra-semi.cpp
test/SemaCXX/extra-semi.cpp
Index: test/SemaCXX/extra-semi.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/extra-semi.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -verify -std=c++98 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++03 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++11 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++17 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++2a -Wextra-semi %s
+// RUN: %clang_cc1 -verify -Weverything -Wno-c++98-compat %s
+// RUN: %clang_cc1 -verify -Weverything -Wno-c++98-compat-pedantic %s
+
+// Last two RUN lines check that -Wextra-semi, which was enabled by
+// -Weverything will still be issued even though -Wc++98-compat contains
+// diag::warn_cxx98_compat_top_level_semi, which would be silenced by -Wno-.
+
+void F();
+
+void F() {}
+; // expected-warning {{extra ';' outside of a function}}
+
+namespace ns {
+class C {
+ void F() const;
+};
+}
+; // expected-warning {{extra ';' outside of a function}}
+
+void ns::C::F() const {}
+; // expected-warning {{extra ';' outside of a function}}
Index: test/Parser/cxx-extra-semi.cpp
===================================================================
--- test/Parser/cxx-extra-semi.cpp
+++ test/Parser/cxx-extra-semi.cpp
@@ -38,4 +38,7 @@
#if __cplusplus < 201103L
// expected-warning@-3{{extra ';' outside of a function is a C++11 extension}}
// expected-warning@-3{{extra ';' outside of a function is a C++11 extension}}
+#elif !defined(PEDANTIC)
+// expected-warning@-6{{extra ';' outside of a function}}
+// expected-warning@-6{{extra ';' outside of a function}}
#endif
Index: lib/Parse/Parser.cpp
===================================================================
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -191,12 +191,20 @@
// C++11 allows extra semicolons at namespace scope, but not in any of the
// other contexts.
if (Kind == OutsideFunction && getLangOpts().CPlusPlus) {
+ unsigned DiagID;
+
if (getLangOpts().CPlusPlus11)
- Diag(StartLoc, diag::warn_cxx98_compat_top_level_semi)
- << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
+ DiagID = diag::warn_cxx98_compat_top_level_semi;
else
- Diag(StartLoc, diag::ext_extra_semi_cxx11)
- << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
+ DiagID = diag::ext_extra_semi_cxx11;
+
+ // Prefer the pedantic compatibility warning over the generic,
+ // non-extension, user-requested "extra semi" warning.
+ if (Diags.isIgnored(DiagID, EndLoc))
+ DiagID = diag::warn_extra_semi;
+
+ Diag(StartLoc, DiagID)
+ << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
return;
}
Index: include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -47,6 +47,9 @@
"inside instance variable list|"
"after member function definition}0">,
InGroup<ExtraSemi>;
+def warn_extra_semi : Warning<
+ "extra ';' outside of a function">,
+ InGroup<ExtraSemi>, DefaultIgnore;
def ext_extra_semi_cxx11 : Extension<
"extra ';' outside of a function is a C++11 extension">,
InGroup<CXX11ExtraSemi>;
Index: test/SemaCXX/extra-semi.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/extra-semi.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -verify -std=c++98 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++03 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++11 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++17 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++2a -Wextra-semi %s
+// RUN: %clang_cc1 -verify -Weverything -Wno-c++98-compat %s
+// RUN: %clang_cc1 -verify -Weverything -Wno-c++98-compat-pedantic %s
+
+// Last two RUN lines check that -Wextra-semi, which was enabled by
+// -Weverything will still be issued even though -Wc++98-compat contains
+// diag::warn_cxx98_compat_top_level_semi, which would be silenced by -Wno-.
+
+void F();
+
+void F() {}
+; // expected-warning {{extra ';' outside of a function}}
+
+namespace ns {
+class C {
+ void F() const;
+};
+}
+; // expected-warning {{extra ';' outside of a function}}
+
+void ns::C::F() const {}
+; // expected-warning {{extra ';' outside of a function}}
Index: test/Parser/cxx-extra-semi.cpp
===================================================================
--- test/Parser/cxx-extra-semi.cpp
+++ test/Parser/cxx-extra-semi.cpp
@@ -38,4 +38,7 @@
#if __cplusplus < 201103L
// expected-warning@-3{{extra ';' outside of a function is a C++11 extension}}
// expected-warning@-3{{extra ';' outside of a function is a C++11 extension}}
+#elif !defined(PEDANTIC)
+// expected-warning@-6{{extra ';' outside of a function}}
+// expected-warning@-6{{extra ';' outside of a function}}
#endif
Index: lib/Parse/Parser.cpp
===================================================================
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -191,12 +191,20 @@
// C++11 allows extra semicolons at namespace scope, but not in any of the
// other contexts.
if (Kind == OutsideFunction && getLangOpts().CPlusPlus) {
+ unsigned DiagID;
+
if (getLangOpts().CPlusPlus11)
- Diag(StartLoc, diag::warn_cxx98_compat_top_level_semi)
- << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
+ DiagID = diag::warn_cxx98_compat_top_level_semi;
else
- Diag(StartLoc, diag::ext_extra_semi_cxx11)
- << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
+ DiagID = diag::ext_extra_semi_cxx11;
+
+ // Prefer the pedantic compatibility warning over the generic,
+ // non-extension, user-requested "extra semi" warning.
+ if (Diags.isIgnored(DiagID, EndLoc))
+ DiagID = diag::warn_extra_semi;
+
+ Diag(StartLoc, DiagID)
+ << FixItHint::CreateRemoval(SourceRange(StartLoc, EndLoc));
return;
}
Index: include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -47,6 +47,9 @@
"inside instance variable list|"
"after member function definition}0">,
InGroup<ExtraSemi>;
+def warn_extra_semi : Warning<
+ "extra ';' outside of a function">,
+ InGroup<ExtraSemi>, DefaultIgnore;
def ext_extra_semi_cxx11 : Extension<
"extra ';' outside of a function is a C++11 extension">,
InGroup<CXX11ExtraSemi>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
