[C23] Complete support for WG14 N2508 #71398

AaronBallman · 2023-11-06T14:10:55Z

In Clang 16, we implemented the ability to add a label at the end of a compound statement. These changes complete the implementation by allowing a label to be followed by a declaration in C.

Note, this seems to have fixed an issue with some OpenMP stand-alone directives not being properly diagnosed as per:
https://www.openmp.org/spec-html/5.1/openmpsu19.html#x34-330002.1.3
(The same requirement exists in OpenMP 5.2 as well.)

In Clang 16, we implemented the ability to add a label at the end of a compound statement. These changes complete the implementation by allowing a label to be followed by a declaration in C. Note, this seems to have fixed an issue with some OpenMP stand-alone directives not being properly diagnosed as per: https://www.openmp.org/spec-html/5.1/openmpsu19.html#x34-330002.1.3 (The same requirement exists in OpenMP 5.2 as well.)

llvmbot · 2023-11-06T14:11:12Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-openmp

Author: Aaron Ballman (AaronBallman)

Changes

In Clang 16, we implemented the ability to add a label at the end of a compound statement. These changes complete the implementation by allowing a label to be followed by a declaration in C.

Note, this seems to have fixed an issue with some OpenMP stand-alone directives not being properly diagnosed as per:
https://www.openmp.org/spec-html/5.1/openmpsu19.html#x34-330002.1.3
(The same requirement exists in OpenMP 5.2 as well.)

Patch is 26.22 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71398.diff

17 Files Affected:

(modified) clang/docs/ReleaseNotes.rst (+22)
(modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+6)
(modified) clang/include/clang/Parse/Parser.h (+3-6)
(modified) clang/lib/Parse/ParseOpenMP.cpp (+2-2)
(modified) clang/lib/Parse/ParseStmt.cpp (+21-13)
(modified) clang/test/C/C2x/n2508.c (+11)
(modified) clang/test/OpenMP/barrier_ast_print.cpp (+16-8)
(modified) clang/test/OpenMP/barrier_messages.cpp (+4-4)
(modified) clang/test/OpenMP/cancel_messages.cpp (+4-2)
(modified) clang/test/OpenMP/cancellation_point_messages.cpp (+4-2)
(modified) clang/test/OpenMP/depobj_messages.cpp (+4-4)
(modified) clang/test/OpenMP/error_message.cpp (+5-5)
(modified) clang/test/OpenMP/flush_messages.cpp (+4-4)
(modified) clang/test/OpenMP/scan_messages.cpp (+6-4)
(modified) clang/test/OpenMP/taskwait_messages.cpp (+4-4)
(modified) clang/test/OpenMP/taskyield_messages.cpp (+4-4)
(modified) clang/www/c_status.html (+1-1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5e19dbea6608486..ae851d754c84831 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -209,6 +209,12 @@ C23 Feature Support
 - Clang now supports ``<stdckdint.h>`` which defines several macros for performing
   checked integer arithmetic. It is also exposed in pre-C23 modes.
 
+- Completed the implementation of
+  `N2508 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2508.pdf>`_. We
+  previously implemented allowing a label at the end of a compound statement,
+  and now we've implemented allowing a label to be followed by a declaration
+  instead of a statement.
+
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 
@@ -545,6 +551,22 @@ Bug Fixes in This Version
   Fixes (`#67687 <https://github.com/llvm/llvm-project/issues/67687>`_)
 - Fix crash from constexpr evaluator evaluating uninitialized arrays as rvalue.
   Fixes (`#67317 <https://github.com/llvm/llvm-project/issues/67317>`_)
+- Clang now properly diagnoses use of stand-alone OpenMP directives after a
+  label (including ``case`` or ``default`` labels).
+
+  Before:
+
+  .. code-block:: c++
+
+  label:
+  #pragma omp barrier // ok
+
+  After:
+
+  .. code-block:: c++
+
+  label:
+  #pragma omp barrier // error: '#pragma omp barrier' cannot be an immediate substatement
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index de180344fcc5c74..2bc72acc1375143 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -299,6 +299,12 @@ def note_missing_selector_name : Note<
 def note_force_empty_selector_name : Note<
   "or insert whitespace before ':' to use %0 as parameter name "
   "and have an empty entry in the selector">;
+def ext_c_label_followed_by_declaration : ExtWarn<
+  "label followed by a declaration is a C23 extension">,
+  InGroup<C23>;
+def warn_c23_compat_label_followed_by_declaration : Warning<
+  "label followed by a declaration is incompatible with C standards before "
+  "C23">, InGroup<CPre23Compat>, DefaultIgnore;
 def ext_c_label_end_of_compound_statement : ExtWarn<
   "label at end of compound statement is a C23 extension">,
    InGroup<C23>;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 30e0352c868637b..22e8c4ceea39cd6 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -410,18 +410,15 @@ class Parser : public CodeCompletionHandler {
 
   /// Flags describing a context in which we're parsing a statement.
   enum class ParsedStmtContext {
-    /// This context permits declarations in language modes where declarations
-    /// are not statements.
-    AllowDeclarationsInC = 0x1,
     /// This context permits standalone OpenMP directives.
-    AllowStandaloneOpenMPDirectives = 0x2,
+    AllowStandaloneOpenMPDirectives = 0x1,
     /// This context is at the top level of a GNU statement expression.
-    InStmtExpr = 0x4,
+    InStmtExpr = 0x2,
 
     /// The context of a regular substatement.
     SubStmt = 0,
     /// The context of a compound-statement.
-    Compound = AllowDeclarationsInC | AllowStandaloneOpenMPDirectives,
+    Compound = AllowStandaloneOpenMPDirectives,
 
     LLVM_MARK_AS_BITMASK_ENUM(InStmtExpr)
   };
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 3e7d8274aeefc52..9b47ec4fbbc51f1 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2670,7 +2670,7 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
   }
   case OMPD_threadprivate: {
     // FIXME: Should this be permitted in C++?
-    if ((StmtCtx & ParsedStmtContext::AllowDeclarationsInC) ==
+    if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
         ParsedStmtContext()) {
       Diag(Tok, diag::err_omp_immediate_directive)
           << getOpenMPDirectiveName(DKind) << 0;
@@ -2689,7 +2689,7 @@ StmtResult Parser::ParseOpenMPDeclarativeOrExecutableDirective(
   }
   case OMPD_allocate: {
     // FIXME: Should this be permitted in C++?
-    if ((StmtCtx & ParsedStmtContext::AllowDeclarationsInC) ==
+    if ((StmtCtx & ParsedStmtContext::AllowStandaloneOpenMPDirectives) ==
         ParsedStmtContext()) {
       Diag(Tok, diag::err_omp_immediate_directive)
           << getOpenMPDirectiveName(DKind) << 0;
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 2531147c23196ae..da991c641130921 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -235,10 +235,7 @@ StmtResult Parser::ParseStatementOrDeclarationAfterAttributes(
     auto IsStmtAttr = [](ParsedAttr &Attr) { return Attr.isStmtAttr(); };
     bool AllAttrsAreStmtAttrs = llvm::all_of(CXX11Attrs, IsStmtAttr) &&
                                 llvm::all_of(GNUAttrs, IsStmtAttr);
-    if ((getLangOpts().CPlusPlus || getLangOpts().MicrosoftExt ||
-         (StmtCtx & ParsedStmtContext::AllowDeclarationsInC) !=
-             ParsedStmtContext()) &&
-        ((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
+    if (((GNUAttributeLoc.isValid() && !(HaveAttrs && AllAttrsAreStmtAttrs)) ||
          isDeclarationStatement())) {
       SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
       DeclGroupPtrTy Decl;
@@ -698,6 +695,19 @@ StmtResult Parser::ParseSEHLeaveStatement() {
   return Actions.ActOnSEHLeaveStmt(LeaveLoc, getCurScope());
 }
 
+static void DiagnoseLabelFollowedByDecl(Parser &P, const Stmt *SubStmt) {
+  // When in C mode (but not Microsoft extensions mode), diagnose use of a
+  // label that is followed by a declaration rather than a statement.
+  if (!P.getLangOpts().CPlusPlus && !P.getLangOpts().MicrosoftExt &&
+      isa<DeclStmt>(SubStmt)) {
+    if (P.getLangOpts().C23)
+      P.Diag(SubStmt->getBeginLoc(),
+             diag::warn_c23_compat_label_followed_by_declaration);
+    else
+      P.Diag(SubStmt->getBeginLoc(), diag::ext_c_label_followed_by_declaration);
+  }
+}
+
 /// ParseLabeledStatement - We have an identifier and a ':' after it.
 ///
 ///       label:
@@ -712,9 +722,7 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributes &Attrs,
   assert(Tok.is(tok::identifier) && Tok.getIdentifierInfo() &&
          "Not an identifier!");
 
-  // The substatement is always a 'statement', not a 'declaration', but is
-  // otherwise in the same context as the labeled-statement.
-  StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC;
+  StmtCtx &= ~ParsedStmtContext::AllowStandaloneOpenMPDirectives;
 
   Token IdentTok = Tok;  // Save the whole token.
   ConsumeToken();  // eat the identifier.
@@ -763,6 +771,8 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributes &Attrs,
   if (SubStmt.isInvalid())
     SubStmt = Actions.ActOnNullStmt(ColonLoc);
 
+  DiagnoseLabelFollowedByDecl(*this, SubStmt.get());
+
   LabelDecl *LD = Actions.LookupOrCreateLabel(IdentTok.getIdentifierInfo(),
                                               IdentTok.getLocation());
   Actions.ProcessDeclAttributeList(Actions.CurScope, LD, Attrs);
@@ -781,9 +791,7 @@ StmtResult Parser::ParseCaseStatement(ParsedStmtContext StmtCtx,
                                       bool MissingCase, ExprResult Expr) {
   assert((MissingCase || Tok.is(tok::kw_case)) && "Not a case stmt!");
 
-  // The substatement is always a 'statement', not a 'declaration', but is
-  // otherwise in the same context as the labeled-statement.
-  StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC;
+  StmtCtx &= ~ParsedStmtContext::AllowStandaloneOpenMPDirectives;
 
   // It is very common for code to contain many case statements recursively
   // nested, as in (but usually without indentation):
@@ -909,6 +917,7 @@ StmtResult Parser::ParseCaseStatement(ParsedStmtContext StmtCtx,
     // Broken sub-stmt shouldn't prevent forming the case statement properly.
     if (SubStmt.isInvalid())
       SubStmt = Actions.ActOnNullStmt(SourceLocation());
+    DiagnoseLabelFollowedByDecl(*this, SubStmt.get());
     Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, SubStmt.get());
   }
 
@@ -924,9 +933,7 @@ StmtResult Parser::ParseCaseStatement(ParsedStmtContext StmtCtx,
 StmtResult Parser::ParseDefaultStatement(ParsedStmtContext StmtCtx) {
   assert(Tok.is(tok::kw_default) && "Not a default stmt!");
 
-  // The substatement is always a 'statement', not a 'declaration', but is
-  // otherwise in the same context as the labeled-statement.
-  StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC;
+  StmtCtx &= ~ParsedStmtContext::AllowStandaloneOpenMPDirectives;
 
   SourceLocation DefaultLoc = ConsumeToken();  // eat the 'default'.
 
@@ -960,6 +967,7 @@ StmtResult Parser::ParseDefaultStatement(ParsedStmtContext StmtCtx) {
   if (SubStmt.isInvalid())
     SubStmt = Actions.ActOnNullStmt(ColonLoc);
 
+  DiagnoseLabelFollowedByDecl(*this, SubStmt.get());
   return Actions.ActOnDefaultStmt(DefaultLoc, ColonLoc,
                                   SubStmt.get(), getCurScope());
 }
diff --git a/clang/test/C/C2x/n2508.c b/clang/test/C/C2x/n2508.c
index 2dee0b668cb4e8c..b47c1274aab965c 100644
--- a/clang/test/C/C2x/n2508.c
+++ b/clang/test/C/C2x/n2508.c
@@ -21,3 +21,14 @@ void test() {
 final:
 }
 
+void test_labels() {
+label:
+  int i = 0;
+
+  switch (i) {
+  case 1:
+    _Static_assert(true);
+  default:
+    _Static_assert(true);
+  }
+}
diff --git a/clang/test/OpenMP/barrier_ast_print.cpp b/clang/test/OpenMP/barrier_ast_print.cpp
index a8db2172c19289d..9b7398b3d5b4324 100644
--- a/clang/test/OpenMP/barrier_ast_print.cpp
+++ b/clang/test/OpenMP/barrier_ast_print.cpp
@@ -17,13 +17,13 @@ T tmain(T argc) {
   static T a;
 #pragma omp barrier
   switch (argc) {
-  case 0:
+  case 0: {
 #pragma omp barrier
-    break;
-  default:
+  } break;
+  default: {
 #pragma omp barrier
 #pragma omp barrier
-    break;
+  } break;
   }
   return a + argc;
 }
@@ -35,11 +35,15 @@ T tmain(T argc) {
 // CHECK-NEXT: #pragma omp barrier
 // CHECK-NEXT: switch (argc) {
 // CHECK-NEXT: case 0:
+// CHECK-NEXT: {
 // CHECK-NEXT: #pragma omp barrier
+// CHECK-NEXT: }
 // CHECK-NEXT: break;
 // CHECK-NEXT: default:
+// CHECK-NEXT: {
 // CHECK-NEXT: #pragma omp barrier
 // CHECK-NEXT: #pragma omp barrier
+// CHECK-NEXT: }
 // CHECK-NEXT: break;
 // CHECK-NEXT: }
 
@@ -49,21 +53,25 @@ int main(int argc, char **argv) {
 #pragma omp barrier
   // CHECK-NEXT: #pragma omp barrier
   switch (argc) {
-  case 0:
+  case 0: {
 #pragma omp barrier
 #pragma omp barrier
-    break;
-  default:
+  } break;
+  default: {
 #pragma omp barrier
-    break;
+  } break;
   }
 // CHECK-NEXT: switch (argc) {
 // CHECK-NEXT: case 0:
+// CHECK-NEXT: {
 // CHECK-NEXT: #pragma omp barrier
 // CHECK-NEXT: #pragma omp barrier
+// CHECK-NEXT: }
 // CHECK-NEXT: break;
 // CHECK-NEXT: default:
+// CHECK-NEXT: {
 // CHECK-NEXT: #pragma omp barrier
+// CHECK-NEXT: }
 // CHECK-NEXT: break;
 // CHECK-NEXT: }
   return tmain(argc) + tmain(argv[0][0]) + a;
diff --git a/clang/test/OpenMP/barrier_messages.cpp b/clang/test/OpenMP/barrier_messages.cpp
index 589b9c711aaae95..1dda66b9b242592 100644
--- a/clang/test/OpenMP/barrier_messages.cpp
+++ b/clang/test/OpenMP/barrier_messages.cpp
@@ -38,7 +38,7 @@ T tmain(T argc) {
   switch (argc) {
 #pragma omp barrier
   case 1:
-#pragma omp barrier
+#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
     break;
   default: {
 #pragma omp barrier
@@ -50,7 +50,7 @@ T tmain(T argc) {
 #pragma omp barrier
     }
 label:
-#pragma omp barrier
+#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
 label1 : {
 #pragma omp barrier
 }
@@ -92,7 +92,7 @@ int main(int argc, char **argv) {
   switch (argc) {
 #pragma omp barrier
   case 1:
-#pragma omp barrier
+#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
     break;
   default: {
 #pragma omp barrier
@@ -104,7 +104,7 @@ int main(int argc, char **argv) {
 #pragma omp barrier
     }
 label:
-#pragma omp barrier
+#pragma omp barrier // expected-error {{'#pragma omp barrier' cannot be an immediate substatement}}
 label1 : {
 #pragma omp barrier
 }
diff --git a/clang/test/OpenMP/cancel_messages.cpp b/clang/test/OpenMP/cancel_messages.cpp
index fd19f1f373ef9b1..0c96beecc04b04e 100644
--- a/clang/test/OpenMP/cancel_messages.cpp
+++ b/clang/test/OpenMP/cancel_messages.cpp
@@ -71,7 +71,8 @@ int main(int argc, char **argv) {
   switch (argc) {
 #pragma omp cancel taskgroup // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
   case 1:
-#pragma omp cancel parallel // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
+#pragma omp cancel parallel // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} \
+                               expected-error {{'#pragma omp cancel' cannot be an immediate substatement}}
     break;
   default: {
 #pragma omp cancel sections // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
@@ -83,7 +84,8 @@ int main(int argc, char **argv) {
 #pragma omp cancel taskgroup // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
     }
 label:
-#pragma omp cancel parallel // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
+#pragma omp cancel parallel // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}} \
+                               expected-error {{'#pragma omp cancel' cannot be an immediate substatement}}
 label1 : {
 #pragma omp cancel sections // expected-error {{orphaned 'omp cancel' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
 }
diff --git a/clang/test/OpenMP/cancellation_point_messages.cpp b/clang/test/OpenMP/cancellation_point_messages.cpp
index 268cab2d80a6e40..e928449ff57a980 100644
--- a/clang/test/OpenMP/cancellation_point_messages.cpp
+++ b/clang/test/OpenMP/cancellation_point_messages.cpp
@@ -71,7 +71,8 @@ int main(int argc, char **argv) {
   switch (argc) {
 #pragma omp cancellation point taskgroup // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
   case 1:
-#pragma omp cancellation point parallel // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
+#pragma omp cancellation point parallel // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} \
+                                           expected-error {{'#pragma omp cancellation point' cannot be an immediate substatement}}
     break;
   default: {
 #pragma omp cancellation point sections // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
@@ -83,7 +84,8 @@ int main(int argc, char **argv) {
 #pragma omp cancellation point taskgroup // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
     }
 label:
-#pragma omp cancellation point parallel // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
+#pragma omp cancellation point parallel // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}} \
+                                           expected-error {{'#pragma omp cancellation point' cannot be an immediate substatement}}
 label1 : {
 #pragma omp cancellation point sections // expected-error {{orphaned 'omp cancellation point' directives are prohibited; perhaps you forget to enclose the directive into a region?}}
 }
diff --git a/clang/test/OpenMP/depobj_messages.cpp b/clang/test/OpenMP/depobj_messages.cpp
index 38617e16cab9c9a..9d750f651d81f39 100644
--- a/clang/test/OpenMP/depobj_messages.cpp
+++ b/clang/test/OpenMP/depobj_messages.cpp
@@ -59,7 +59,7 @@ T tmain(T argc) {
   switch (argc) {
 #pragma omp depobj(x) depend(in:s)
   case 1:
-#pragma omp depobj(x) depend(in:s)
+#pragma omp depobj(x) depend(in:s) // expected-error {{'#pragma omp depobj' cannot be an immediate substatement}}
     break;
   default: {
 #pragma omp depobj(x) depend(in:s)
@@ -71,7 +71,7 @@ T tmain(T argc) {
 #pragma omp depobj(x) depend(in:s)
     }
 label:
-#pragma omp depobj(x) depend(in:s)
+#pragma omp depobj(x) depend(in:s) // expected-error {{'#pragma omp depobj' cannot be an immediate substatement}}
 label1 : {
 #pragma omp depobj(x) depend(in:s)
 }
@@ -124,7 +124,7 @@ omp_depend_t x;
   switch (argc) {
 #pragma omp depobj(x) depend(in:s)
   case 1:
-#pragma omp depobj(x) depend(in:s)
+#pragma omp depobj(x) depend(in:s) // expected-error {{'#pragma omp depobj' cannot be an immediate substatement}}
     break;
   default: {
 #pragma omp depobj(x) depend(in:s)
@@ -136,7 +136,7 @@ omp_depend_t x;
 #pragma omp depobj(x) depend(in:s)
     }
 label:
-#pragma omp depobj(x) depend(in:s)
+#pragma omp depobj(x) depend(in:s) // expected-error {{'#pragma omp depobj' cannot be an immediate substatement}}
 label1 : {
 #pragma omp depobj(x) depend(in:s)
 }
diff --git a/clang/test/OpenMP/error_message.cpp b/clang/test/OpenMP/error_message.cpp
index 40f1db40fb63af2..9796a6443c0100f 100644
--- a/clang/test/OpenMP/error_message.cpp
+++ b/clang/test/OpenMP/error_message.cpp
@@ -32,8 +32,8 @@ T tmain(T argc) {
   }
   switch (argc) {
 #pragma omp error // expected-error {{ERROR}}
-  case 1:
-#pragma omp error // expected-error {{ERROR}}
+  case 1: // FIXME: error without 'at execution' is not a stand-alone directive and so this should be accepted.
+#pragma omp error // expected-error {{'#pragma omp error' cannot be an immediate substatement}}
     break;
   default: {
 #pragma omp error // expected-error {{ERROR}}
@@ -45,7 +45,7 @@ T tmain(T argc) {
 #pragma omp error // expected-error {{ERROR}}
     }
 label:
-#pragma omp error // expected-error {{ERROR}}
+#pragma omp error // expected-error {{'#pragma omp error' cannot be an immediate substatement}}
 label1 : {
 #pragma omp error // expected-error {{ERROR}}
 }
@@ -168,7 +168,7 @@ int main(int argc, char **argv) {
 // expected-error@+1 {{ERROR}}
 #pragma omp error
   case 1:
-// expected-error@+1 {{ERROR}}
+// expected-error@+1 {{'#pragma omp error' cannot be an immediate substatement}}
 #pragma omp error
     break;
   default: {
@@ -183,7 +183,7 @@ int main(int argc, char **argv) {
 #pragma omp error
     }
 label:
-// expected-error@+1 {{ERROR}}
+// expected-error@+1 {{'#pragma omp error' cannot be an immediate substatement}}
 #pragma omp error
 label1 : {
 // expected-error@+1 {{ERROR}}
diff --git a/clang/test/OpenMP/flush_messages.cpp b/clang/test/OpenMP/flush_messages.cpp
index 1be0fb8d590adc3..ad4830b5bf94f96 100644
--- a/clang/test/OpenMP/flush_messages.cpp
+++ b/clang/test/OpenMP/flush_messages.cpp
@@ -43,7 +43,7 @@ T tmain(T argc) {
   switch (argc) {
 #pragma omp flush
   case 1:
-#pragma omp flush
+#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}}
     break;
   default: {
 #pragma omp flush
@@ -55,7 +55,7 @@ T tmain(T argc) {
 #pragma omp flush
     }
 label:
-#pragma omp flush
+#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}}
 label1 : {
 #pragma omp flush
 }
@@ -107,7 +107,7 @@ int main(int argc, char **argv) {
   switch (argc) {
 #pragma omp flush
   case 1:
-#pragma omp flush
+#pragma omp flush // expected-error {{'#pragma omp flush' cannot be an immediate substatement}}
     break;
   default: {
 #pragma omp flush
@@ -119,7 +119,7 @@ int main(int argc, char **argv) {
 #pragma omp flush
     }
 l...
[truncated]

clang/include/clang/Basic/DiagnosticParseKinds.td

mikerice1969 · 2023-11-06T17:50:50Z

clang/test/OpenMP/error_message.cpp

@@ -32,8 +32,8 @@ T tmain(T argc) {
  }
  switch (argc) {
 #pragma omp error // expected-error {{ERROR}}
-  case 1:
-#pragma omp error // expected-error {{ERROR}}
+  case 1: // FIXME: error without 'at execution' is not a stand-alone directive and so this should be accepted.


I would remove this FIXME. I think we want 'error' to be diagnosed. The original meaning of stand-alone was directives that did not has associated user statements. This was before utility/informational directives existed. I think the intention is we only allow executable statements with associated user statements here, not declarative, utility, etc.

SGTM, thank you! This should be done now.

cor3ntin · 2023-11-07T00:27:16Z

clang/lib/Parse/ParseStmt.cpp

-  // The substatement is always a 'statement', not a 'declaration', but is
-  // otherwise in the same context as the labeled-statement.
-  StmtCtx &= ~ParsedStmtContext::AllowDeclarationsInC;
+  StmtCtx &= ~ParsedStmtContext::AllowStandaloneOpenMPDirectives;


I think this deserves a comment.

cor3ntin · 2023-11-07T00:29:40Z

clang/lib/Parse/ParseStmt.cpp

+    if (P.getLangOpts().C23)
+      P.Diag(SubStmt->getBeginLoc(),
+             diag::warn_c23_compat_label_followed_by_declaration);
+    else
+      P.Diag(SubStmt->getBeginLoc(), diag::ext_c_label_followed_by_declaration);
+  }


Suggested change

if (P.getLangOpts().C23)

P.Diag(SubStmt->getBeginLoc(),

diag::warn_c23_compat_label_followed_by_declaration);

else

P.Diag(SubStmt->getBeginLoc(), diag::ext_c_label_followed_by_declaration);

}

P.Diag(SubStmt->getBeginLoc(), P.getLangOpts().C23 ?

diag::warn_c23_compat_label_followed_by_declaration

: diag::ext_c_label_followed_by_declaration);

Fznamznon

LGTM, but someone else must approve.

AaronBallman · 2023-11-17T17:28:01Z

Ping

mikerice1969

LGTM. Thanks!

AaronBallman added 2 commits November 6, 2023 08:54

Merge remote-tracking branch 'origin/main' into aballman-c23-n2508

683ef4e

AaronBallman added openmp c23 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Nov 6, 2023

AaronBallman requested review from zygoloid, jyknight, Fznamznon and mikerice1969 November 6, 2023 14:10

llvmbot added clang Clang issues not falling into any other category clang:openmp OpenMP related changes to Clang labels Nov 6, 2023

AaronBallman removed the openmp label Nov 6, 2023

Fznamznon reviewed Nov 6, 2023

View reviewed changes

clang/include/clang/Basic/DiagnosticParseKinds.td Show resolved Hide resolved

Add test coverage for pedantic and compat diagnostics

532b1ea

mikerice1969 reviewed Nov 6, 2023

View reviewed changes

Remove FIXME comment based on review feedback

b2c9965

cor3ntin reviewed Nov 7, 2023

View reviewed changes

Update based on review feedback; NFC

8981b9e

Fznamznon reviewed Nov 7, 2023

View reviewed changes

mikerice1969 approved these changes Nov 17, 2023

View reviewed changes

Merge branch 'main' into aballman-c23-n2508

2a3f06a

AaronBallman merged commit 8bd06d5 into llvm:main Nov 20, 2023

AaronBallman deleted the aballman-c23-n2508 branch November 20, 2023 15:52

frederick-vs-ja mentioned this pull request Jan 5, 2024

C variable declaration after label is considered invalid #77057

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[C23] Complete support for WG14 N2508 #71398

[C23] Complete support for WG14 N2508 #71398

Uh oh!

AaronBallman commented Nov 6, 2023

Uh oh!

llvmbot commented Nov 6, 2023 •

edited

Loading

Uh oh!

Uh oh!

mikerice1969 Nov 6, 2023

Uh oh!

AaronBallman Nov 6, 2023

Uh oh!

cor3ntin Nov 7, 2023

Uh oh!

AaronBallman Nov 7, 2023

Uh oh!

cor3ntin Nov 7, 2023

Uh oh!

Fznamznon left a comment

Uh oh!

AaronBallman commented Nov 17, 2023

Uh oh!

mikerice1969 left a comment

Uh oh!

Uh oh!

[C23] Complete support for WG14 N2508 #71398

[C23] Complete support for WG14 N2508 #71398

Uh oh!

Conversation

AaronBallman commented Nov 6, 2023

Uh oh!

llvmbot commented Nov 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

mikerice1969 Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

AaronBallman Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

cor3ntin Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

AaronBallman Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

cor3ntin Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

Fznamznon left a comment

Choose a reason for hiding this comment

Uh oh!

AaronBallman commented Nov 17, 2023

Uh oh!

mikerice1969 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvmbot commented Nov 6, 2023 •

edited

Loading