[C2y] Add test coverage for WG14 N3370 #115054
Merged
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
This paper added case ranges in switch statements, which is a GNU extension Clang has supported since at least Clang 3.0. It updates the diagnostics to no longer call this a GNU extension except in C++ mode.
AaronBallman
added
clang:frontend
|
@llvm/pr-subscribers-clang Author: Aaron Ballman (AaronBallman) ChangesThis paper added case ranges in switch statements, which is a GNU extension Clang has supported since at least Clang 3.0. It updates the diagnostics to no longer call this a GNU extension except in C++ mode. Full diff: https://github.com/llvm/llvm-project/pull/115054.diff 4 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 78510e61a639fa..0da509280068ad 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -203,8 +203,13 @@ def err_expected_equal_designator : Error<"expected '=' or another designator">;
def ext_gnu_old_style_field_designator : ExtWarn<
"use of GNU old-style field designator extension">,
InGroup<GNUDesignator>;
-def ext_gnu_case_range : Extension<"use of GNU case range extension">,
- InGroup<GNUCaseRange>;
+def ext_gnu_case_range : Extension<
+ "case ranges are a GNU extension">, InGroup<GNUCaseRange>;
+def warn_c23_compat_case_range : Warning<
+ "case ranges are incompatible with C standards before C2y">,
+ DefaultIgnore, InGroup<CPre2yCompat>;
+def ext_c2y_case_range : Extension<
+ "case ranges are a C2y extension">, InGroup<C2y>;
// Generic errors.
def err_expected_expression : Error<"expected expression">;
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 6470e55e521add..9ba3b112254933 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -890,7 +890,15 @@ StmtResult Parser::ParseCaseStatement(ParsedStmtContext StmtCtx,
SourceLocation DotDotDotLoc;
ExprResult RHS;
if (TryConsumeToken(tok::ellipsis, DotDotDotLoc)) {
- Diag(DotDotDotLoc, diag::ext_gnu_case_range);
+ // In C++, this is a GNU extension. In C, it's a C2y extension.
+ unsigned DiagId;
+ if (getLangOpts().CPlusPlus)
+ DiagId = diag::ext_gnu_case_range;
+ else if (getLangOpts().C2y)
+ DiagId = diag::warn_c23_compat_case_range;
+ else
+ DiagId = diag::ext_c2y_case_range;
+ Diag(DotDotDotLoc, DiagId);
RHS = ParseCaseExpression(CaseLoc);
if (RHS.isInvalid()) {
if (!SkipUntil(tok::colon, tok::r_brace, StopAtSemi | StopBeforeMatch))
diff --git a/clang/test/C/C2y/n3370.c b/clang/test/C/C2y/n3370.c
new file mode 100644
index 00000000000000..0d654fb4ad28b9
--- /dev/null
+++ b/clang/test/C/C2y/n3370.c
@@ -0,0 +1,105 @@
+// RUN: %clang_cc1 -verify=expected,c-expected -std=c2y -Wall -pedantic %s
+// RUN: %clang_cc1 -verify=expected,c-expected,ped -std=c23 -Wall -pedantic %s
+// RUN: %clang_cc1 -verify=expected,cxx-expected,gnu -Wall -pedantic -x c++ %s
+// RUN: %clang_cc1 -verify=expected,c-expected,pre -std=c2y -Wpre-c2y-compat -Wall -pedantic %s
+
+/* WG14 N3370: Yes
+ * Case range expressions v3.1
+ *
+ * This introduces the ability to specify closed ranges in case statements in a
+ * switch statement. This was already a well-supported Clang extension before
+ * it was standardized.
+ */
+
+void correct(int i) {
+ constexpr int j = 100, k = 200;
+ switch (i) {
+ case 12 ... 14: break; /* gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ // Implementations are encouraged to diagnose empty ranges.
+ case 15 ... 11: break; /* expected-warning {{empty case range specified}}
+ gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ // This is not an empty range, it's a range of a single value.
+ case 10 ... 10: break; /* gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ case j ... k: break; /* gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ }
+}
+
+void incorrect(int i) { // cxx-expected-note 2 {{declared here}}
+ switch (i) {
+ // The values have to be integer constant expressions. Note that when the
+ // initial value in the range is an error, we don't issue the warnings about
+ // extensions or incompatibility.
+ case i ... 10: break; /* c-expected-error {{expression is not an integer constant expression}}
+ cxx-expected-error {{case value is not a constant expression}}
+ cxx-expected-note {{function parameter 'i' with unknown value cannot be used in a constant expression}}
+ */
+ case 10 ... i: break; /* c-expected-error {{expression is not an integer constant expression}}
+ cxx-expected-error {{case value is not a constant expression}}
+ cxx-expected-note {{function parameter 'i' with unknown value cannot be used in a constant expression}}
+ gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ case 1.3f ... 10: break; /* c-expected-error {{integer constant expression must have integer type, not 'float'}}
+ cxx-expected-error {{conversion from 'float' to 'int' is not allowed in a converted constant expression}}
+ */
+ case 10 ... "a": break; /* c-expected-error {{integer constant expression must have integer type, not 'char[2]'}}
+ cxx-expected-error {{value of type 'const char[2]' is not implicitly convertible to 'int'}}
+ gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ }
+
+ switch (i) {
+ // Cannot have multiple cases covering the same value.
+ // FIXME: diagnostic quality here is poor. The "previous case" note is
+ // showing up on a subsequent line (I'd expect the error and note to be
+ // reversed), and "duplicate case value 20" is showing up on a line where
+ // there is no duplicate value 20 to begin with.
+ case 10 ... 20: break; /* expected-error {{duplicate case value '11'}}
+ expected-note {{previous case defined here}}
+ gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ case 11: break; /* expected-note {{previous case defined here}}
+ */
+ case 11 ... 14: break; /* expected-error {{duplicate case value '20'}}
+ gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ }
+
+ // The values specified by the range shall not change as a result of
+ // conversion to the promoted type of the controlling expression.
+ // FIXME: the overflow warnings seem like they probably should also trigger
+ // in C++ as they do in C.
+ switch ((unsigned char)i) {
+ case 254 ... 256: break; /* c-expected-warning {{overflow converting case value to switch condition type (256 to 0)}}
+ gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ case 257 ... 258: break; /* c-expected-warning {{overflow converting case value to switch condition type (257 to 1)}}
+ c-expected-warning {{overflow converting case value to switch condition type (258 to 2)}}
+ gnu-warning {{case ranges are a GNU extension}}
+ ped-warning {{case ranges are a C2y extension}}
+ pre-warning {{case ranges are incompatible with C standards before C2y}}
+ */
+ }
+}
+
diff --git a/clang/www/c_status.html b/clang/www/c_status.html
index c896b6124b6ab2..8b677095cee182 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -236,7 +236,7 @@ <h2 id="c2y">C2y implementation status</h2>
<tr>
<td>Case range expressions v3.1</td>
<td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3370.htm">N3370</a></td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="full" align="center">Yes</td>
</tr>
<tr>
<td>New _Lengthof() operator (v4)</td>
|
erichkeane
reviewed
Nov 5, 2024
| */ | ||
| case 11: break; /* expected-note {{previous case defined here}} | ||
| */ | ||
| case 11 ... 14: break; /* expected-error {{duplicate case value '20'}} |
I'm not opposed, but we've always supported this feature. Similar to |
There is a minor value (at least for messaging!) for a release note that says "we looked at implementing N3370, saw it was implemented, so added sufficient testing to validate it, so now you can use it without getting GNU-ext warnings in C" |
Sirraide
approved these changes
Nov 6, 2024
There was a problem hiding this comment.
Changes lgtm.
I don’t really have a strong opinion on the release note: on the one hand, writing one for this does feel a bit like we’d be writing a release note for C2y itself rather than for Clang, but on the other, I guess adding something like ‘Clang now diagnoses case ranges as a Clang extension rather than a GNU extension’ would be an option. Iirc I added a similar release note when I changed some variadic macro warnings.
|
I've fixed up a failing test caught by precommit CI and added a release note. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This paper added case ranges in switch statements, which is a GNU extension Clang has supported since at least Clang 3.0.
It updates the diagnostics to no longer call this a GNU extension except in C++ mode.