[Clang] Support case and default labels at end of compound statement

[Izaron]

Summary

Direct continuation of https://reviews.llvm.org/D133887

[Izaron]

clang/include/clang/Basic/DiagnosticParseKinds.td

298–299

This is not an error anymore

clang/lib/Parse/ParseStmt.cpp

932–940

We have copy-pasted this piece of code in 3 places (simple label, case label, default label). Does it make sense to have a method for label diags?

clang/test/Parser/c2x-label.c

12–30

I feel like we may be having redundant tests, because Parser/c2x-label.c and C2x/n2508.c test the same things...

[Izaron]

Fix corresponding FixIt clang test

[aaron.ballman]

clang/lib/Parse/ParseStmt.cpp

932–940

I think a helper function to diagnose this would not be a bad thing.

clang/test/Parser/c2x-label.c

12–30

Yup, we'll definitely have redundancy in the tests -- the tests under the C directory are for (eventually) automatically filling out: https://clang.llvm.org/c_status.html instead of relying on folks to manually modify the HTML every time. So they're mostly "do the examples from the paper work" kind of tests, whereas the Parser/Sema/etc tests are going to test a LOT more edge cases.

[Izaron]

Add helper method DiagnoseLabelAtEndOfCompoundStatement

[aaron.ballman]

LGTM!

clang/include/clang/Basic/DiagnosticParseKinds.td

def warn_missing_selector_name : Warning<
   "%0 used as the name of the previous parameter rather than as part "
   "of the selector">,
   InGroup<DiagGroup<"missing-selector-name">>;
 def note_missing_selector_name : Note<
   "introduce a parameter name to make %0 part of the selector">;
 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 err_switch_label_end_of_compound_statement : Error<
-  "label at end of switch compound statement: expected statement">;
 def ext_c_label_end_of_compound_statement : ExtWarn<
   "label at end of compound statement is a C2x extension">,
    InGroup<C2x>;
 def ext_cxx_label_end_of_compound_statement : ExtWarn<
   "label at end of compound statement is a C++2b extension">,
    InGroup<CXX2b>;
 def warn_c2x_compat_label_end_of_compound_statement : Warning<
   "label at end of compound statement is incompatible with C standards before C2x">,

clang/include/clang/Parse/Parser.h

private:
   StmtResult ParseCaseStatement(ParsedStmtContext StmtCtx,
                                 bool MissingCase = false,
                                 ExprResult Expr = ExprResult());
   StmtResult ParseDefaultStatement(ParsedStmtContext StmtCtx);
   StmtResult ParseCompoundStatement(bool isStmtExpr = false);
   StmtResult ParseCompoundStatement(bool isStmtExpr,
                                     unsigned ScopeFlags);
   void ParseCompoundStatementLeadingPragmas();
+  void DiagnoseLabelAtEndOfCompoundStatement();
   bool ConsumeNullStmt(StmtVector &Stmts);
   StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
   bool ParseParenExprOrCondition(StmtResult *InitStmt,
                                  Sema::ConditionResult &CondResult,
                                  SourceLocation Loc, Sema::ConditionKind CK,
                                  bool MissingOK, SourceLocation *LParenLoc,
                                  SourceLocation *RParenLoc);
   StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc);

clang/lib/Parse/ParseStmt.cpp

else {
           Stmts, StmtCtx, nullptr, EmptyCXX11Attrs, TempAttrs);
       if (!TempAttrs.empty() && !SubStmt.isInvalid())
         SubStmt = Actions.ActOnAttributedStmt(TempAttrs, SubStmt.get());
     }
   }
 
   // The label may have no statement following it
   if (SubStmt.isUnset() && Tok.is(tok::r_brace)) {
-    if (getLangOpts().CPlusPlus) {
-      Diag(Tok, getLangOpts().CPlusPlus2b
-                    ? diag::warn_cxx20_compat_label_end_of_compound_statement
-                    : diag::ext_cxx_label_end_of_compound_statement);
-    } else {
-      Diag(Tok, getLangOpts().C2x
-                    ? diag::warn_c2x_compat_label_end_of_compound_statement
-                    : diag::ext_c_label_end_of_compound_statement);
-    }
+    DiagnoseLabelAtEndOfCompoundStatement();
     SubStmt = Actions.ActOnNullStmt(ColonLoc);
   }
 
   // If we've not parsed a statement yet, parse one now.
   if (!SubStmt.isInvalid() && !SubStmt.isUsable())
     SubStmt = ParseStatement(nullptr, StmtCtx);
 
   // Broken substmt shouldn't prevent the label from being added to the AST.
do {
     }
 
     // Handle all case statements.
   } while (Tok.is(tok::kw_case));
 
   // If we found a non-case statement, start by parsing it.
   StmtResult SubStmt;
 
-  if (Tok.isNot(tok::r_brace)) {
-    SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx);
+  if (Tok.is(tok::r_brace)) {
+    // "switch (X) { case 4: }", is valid and is treated as if label was
+    // followed by a null statement.
+    DiagnoseLabelAtEndOfCompoundStatement();
+    SubStmt = Actions.ActOnNullStmt(ColonLoc);
   } else {
-    // Nicely diagnose the common error "switch (X) { case 4: }", which is
-    // not valid.  If ColonLoc doesn't point to a valid text location, there was
-    // another parsing error, so avoid producing extra diagnostics.
-    if (ColonLoc.isValid()) {
-      SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
-      Diag(AfterColonLoc, diag::err_switch_label_end_of_compound_statement)
-          << FixItHint::CreateInsertion(AfterColonLoc, " ;");
-    }
-    SubStmt = StmtError();
+    SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx);
   }
 
   // Install the body into the most deeply-nested case.
   if (DeepestParsedCaseStmt) {
     // Broken sub-stmt shouldn't prevent forming the case statement properly.
     if (SubStmt.isInvalid())
       SubStmt = Actions.ActOnNullStmt(SourceLocation());
     Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, SubStmt.get());
if (TryConsumeToken(tok::colon, ColonLoc)) {
     Diag(ExpectedLoc, diag::err_expected_after)
         << "'default'" << tok::colon
         << FixItHint::CreateInsertion(ExpectedLoc, ":");
     ColonLoc = ExpectedLoc;
   }
 
   StmtResult SubStmt;
 
-  if (Tok.isNot(tok::r_brace)) {
-    SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx);
+  if (Tok.is(tok::r_brace)) {
+    // "switch (X) {... default: }", is valid and is treated as if label was
+    // followed by a null statement.
+    DiagnoseLabelAtEndOfCompoundStatement();
+    SubStmt = Actions.ActOnNullStmt(ColonLoc);
   } else {
-    // Diagnose the common error "switch (X) {... default: }", which is
-    // not valid.
-    SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
-    Diag(AfterColonLoc, diag::err_switch_label_end_of_compound_statement)
-        << FixItHint::CreateInsertion(AfterColonLoc, " ;");
-    SubStmt = true;
+    SubStmt = ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx);
   }
 
   // Broken sub-stmt shouldn't prevent forming the case statement properly.
   if (SubStmt.isInvalid())
     SubStmt = Actions.ActOnNullStmt(ColonLoc);
 
   return Actions.ActOnDefaultStmt(DefaultLoc, ColonLoc,
                                   SubStmt.get(), getCurScope());
 }
 
 StmtResult Parser::ParseCompoundStatement(bool isStmtExpr) {
   return ParseCompoundStatement(isStmtExpr,
                                 Scope::DeclScope | Scope::CompoundStmtScope);
while (checkForPragmas) {
     default:
       checkForPragmas = false;
       break;
     }
   }
 
 }
 
+void Parser::DiagnoseLabelAtEndOfCompoundStatement() {
+  if (getLangOpts().CPlusPlus) {
+    Diag(Tok, getLangOpts().CPlusPlus2b
+                  ? diag::warn_cxx20_compat_label_end_of_compound_statement
+                  : diag::ext_cxx_label_end_of_compound_statement);
+  } else {
+    Diag(Tok, getLangOpts().C2x
+                  ? diag::warn_c2x_compat_label_end_of_compound_statement
+                  : diag::ext_c_label_end_of_compound_statement);
+  }
+}
+
 /// Consume any extra semi-colons resulting in null statements,
 /// returning true if any tok::semi were consumed.
 bool Parser::ConsumeNullStmt(StmtVector &Stmts) {
   if (!Tok.is(tok::semi))
     return false;
 
   SourceLocation StartLoc = Tok.getLocation();
   SourceLocation EndLoc;

clang/test/AST/ast-dump-stmt.c

 
 label3:
   // CHECK-NEXT: LabelStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:1, col:7> 'label3'
   // CHECK-NEXT: NullStmt 0x{{[^ ]*}} <col:7>
 }
 
 void TestSwitch(int i) {
   switch (i) {
-  // CHECK: SwitchStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:3, line:[[@LINE+32]]:3>
+  // CHECK: SwitchStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:3, line:[[@LINE+37]]:3>
   // CHECK-NEXT: ImplicitCastExpr
   // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:11> 'int' lvalue ParmVar 0x{{[^ ]*}} 'i' 'int'
-  // CHECK-NEXT: CompoundStmt 0x{{[^ ]*}} <col:14, line:[[@LINE+29]]:3>
+  // CHECK-NEXT: CompoundStmt 0x{{[^ ]*}} <col:14, line:[[@LINE+34]]:3>
   case 0:
     break;
   // CHECK-NEXT: CaseStmt 0x{{[^ ]*}} <line:[[@LINE-2]]:3, line:[[@LINE-1]]:5>
   // CHECK-NEXT: ConstantExpr
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:8> 'int' 0
   // CHECK-NEXT: BreakStmt 0x{{[^ ]*}} <line:[[@LINE-4]]:5>
   case 1:
   case 2:
void TestSwitch(int i) {
   case 3 ... 5:
     break;
   // CHECK-NEXT: CaseStmt 0x{{[^ ]*}} <line:[[@LINE-2]]:3, line:[[@LINE-1]]:5> gnu_range
   // CHECK-NEXT: ConstantExpr
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:8> 'int' 3
   // CHECK-NEXT: ConstantExpr
   // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:14> 'int' 5
   // CHECK-NEXT: BreakStmt 0x{{[^ ]*}} <line:[[@LINE-6]]:5>
+  case 6:
+  // CHECK-NEXT: CaseStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:9>
+  // CHECK-NEXT: ConstantExpr
+  // CHECK-NEXT: IntegerLiteral 0x{{[^ ]*}} <col:8> 'int' 6
+  // CHECK-NEXT: NullStmt 0x{{[^ ]*}} <col:9>
+  }
+
+  switch(i){
+  // CHECK: SwitchStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:3, line:[[@LINE+7]]:3>
+  // CHECK-NEXT: ImplicitCastExpr
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:10> 'int' lvalue ParmVar 0x{{[^ ]*}} 'i' 'int'
+  // CHECK-NEXT: CompoundStmt 0x{{[^ ]*}} <col:12, line:[[@LINE+4]]:3>
+  default:
+  // CHECK-NEXT: DefaultStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:10>
+  // CHECK-NEXT: NullStmt 0x{{[^ ]*}} <col:10>
   }
 }
 
 void TestIf(_Bool b) {
   if (b)
     ;
   // CHECK: IfStmt 0x{{[^ ]*}} <line:[[@LINE-2]]:3, line:[[@LINE-1]]:5>
   // CHECK-NEXT: ImplicitCastExpr

clang/test/C/C2x/n2508.c

 // RUN: %clang_cc1 -verify -std=c2x %s
 
-/* WG14 N2508: partial
+// expected-no-diagnostics
+
+/* WG14 N2508: yes
  * Free positioning of labels inside compound statements
  */
 void test() {
   {
   inner:
   }
 
   switch (1) {
-  // FIXME: this should be accepted per C2x 6.8.2p2.
-  case 1: // expected-error {{label at end of switch compound statement: expected statement}}
+  case 1:
   }
 
   {
   multiple: labels: on: a: line:
   }
 
 final:
 }
 

clang/test/FixIt/fixit.c

 
 int commaAtEndOfStatement(void) {
   int a = 1;
   a = 5, // expected-error {{';'}}
   int m = 5, // expected-error {{';'}}
   return 0, // expected-error {{';'}}
 }
 
-int noSemiAfterLabel(int n) {
-  switch (n) {
-    default:
-      return n % 4;
-    case 0:
-    case 1:
-    case 2:
-    // CHECK: /*FOO*/ case 3: ;
-    /*FOO*/ case 3: // expected-error {{expected statement}}
-  }
-  switch (n) {
-    case 1:
-    case 2:
-      return 0;
-    // CHECK: /*BAR*/ default: ;
-    /*BAR*/ default: // expected-error {{expected statement}}
-  }
-  return 1;
-}
-
 struct noSemiAfterStruct // expected-error {{expected ';' after struct}}
 struct noSemiAfterStruct {
   int n // expected-warning {{';'}}
 } // expected-error {{expected ';' after struct}}
 enum noSemiAfterEnum {
   e1
 } // expected-error {{expected ';' after enum}}
 
 int PR17175 __attribute__((visibility(hidden))); // expected-error {{'visibility' attribute requires a string}}

clang/test/Parser/c2x-label.c

 // RUN: %clang_cc1 -fsyntax-only -std=c17 -Wc2x-compat -verify=c17 %s
 // RUN: %clang_cc1 -fsyntax-only -std=c2x -Wpre-c2x-compat -verify=c2x %s
 
-void foo() {
+void test_label_in_func() {
     int x;
 label1:
     x = 1;
 label2: label3: label4:
 } // c17-warning {{label at end of compound statement is a C2x extension}} \
      c2x-warning {{label at end of compound statement is incompatible with C standards before C2x}}
+
+int test_label_in_switch(int v) {
+    switch (v) {
+    case 1:
+        return 1;
+    case 2:
+        return 2;
+    case 3: case 4: case 5:
+    } // c17-warning {{label at end of compound statement is a C2x extension}} \
+         c2x-warning {{label at end of compound statement is incompatible with C standards before C2x}}
+
+    switch (v) {
+    case 6:
+        return 6;
+    default:
+    } // c17-warning {{label at end of compound statement is a C2x extension}} \
+         c2x-warning {{label at end of compound statement is incompatible with C standards before C2x}}
+
+    return 0;
+}

clang/test/Parser/cxx2b-label.cpp

 // RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx2b -std=c++2b -Wpre-c++2b-compat %s
 // RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx20 -std=c++20 %s
 
-void foo() {
+void test_label_in_func() {
 label1:
     int x;
 label2:
     x = 1;
 label3: label4: label5:
 } // cxx20-warning {{label at end of compound statement is a C++2b extension}} \
      cxx2b-warning {{label at end of compound statement is incompatible with C++ standards before C++2b}}
+
+int test_label_in_switch(int v) {
+    switch (v) {
+    case 1:
+        return 1;
+    case 2:
+        return 2;
+    case 3: case 4: case 5:
+    } // cxx20-warning {{label at end of compound statement is a C++2b extension}} \
+         cxx2b-warning {{label at end of compound statement is incompatible with C++ standards before C++2b}}
+
+    switch (v) {
+    case 6:
+        return 6;
+    default:
+    } // cxx20-warning {{label at end of compound statement is a C++2b extension}} \
+         cxx2b-warning {{label at end of compound statement is incompatible with C++ standards before C++2b}}
+
+    return 0;
+}

clang/test/Parser/switch-recovery.cpp

while (x) {
       }
     }
   }
 }
 
 void missing_statement_case(int x) {
   switch (x) {
     case 1:
-    case 0: // expected-error {{label at end of switch compound statement: expected statement}}
-  }
+    case 0:
+  } // expected-warning {{label at end of compound statement is a C++2b extension}}
 }
 
 void missing_statement_default(int x) {
   switch (x) {
     case 0:
-    default: // expected-error {{label at end of switch compound statement: expected statement}}
-  }
+    default:
+  } // expected-warning {{label at end of compound statement is a C++2b extension}}
 }
 
 void pr19022_1() {
   switch (int x)  // expected-error {{variable declaration in condition must have an initializer}}
   case v: ;  // expected-error {{use of undeclared identifier 'v'}}
 }
 
 void pr19022_1a(int x) {
   switch(x) {
-  case 1  // expected-error{{expected ':' after 'case'}} \
-          // expected-error{{label at end of switch compound statement: expected statement}}
-  }
+  case 1  // expected-error{{expected ':' after 'case'}}
+  } // expected-warning {{label at end of compound statement is a C++2b extension}}
 }
 
 void pr19022_1b(int x) {
   switch(x) {
   case v  // expected-error{{use of undeclared identifier 'v'}} \
           // expected-error{{expected ':' after 'case'}}
   } // expected-error{{expected statement}}
  }
int pr19022_4(int x) {
   switch(x) {
   case 1  // expected-error{{expected ':' after 'case'}} expected-note{{previous case defined here}}
   case 1 : return x;  // expected-error{{duplicate case value '1'}}
   }
 }
 
 void pr19022_5(int x) {
   switch(x) {
-  case 1: case // expected-error{{expected ':' after 'case'}} \
-               // expected-error{{expected statement}}
-  }  // expected-error{{expected expression}}
+  case 1: case // expected-error{{expected ':' after 'case'}}
+  }  // expected-error{{expected expression}} \
+     // expected-warning {{label at end of compound statement is a C++2b extension}}
 }
 
 namespace pr19022 {
 int baz5() {}
 bool bar0() {
   switch (int foo0)  //expected-error{{variable declaration in condition must have an initializer}}
   case bar5: ;  // expected-error{{use of undeclared identifier 'bar5'}}
 }

clang/www/c_status.html

<tr>
       <td>Allowing unnamed parameters in function definitions</td>
       <td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2480.pdf">N2480</a></td>
       <td class="full" align="center">Clang 11</td>
     </tr>
     <!-- Aug 2020 Papers -->
     <tr>
       <td>Free positioning of labels inside compound statements</td>
       <td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2508.pdf">N2508</a></td>
-        <td class="partial" align="center">
-          <details><summary>Partial</summary>
-            Clang supports labels at the end of compound statements but does
-            not yet support <code>case</code> or <code>default</code> labels at
-            the end of a switch statement's compound block.
-          </details>
-        </td>
+      <td class="unreleased" align="center">Clang 16</td>
     </tr>
     <tr>
       <td>Clarification request for C17 example of undefined behavior</td>
       <td><a href="https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2517.pdf">N2517</a></td>
       <td class="none" align="center">No</td>
     </tr>
     <tr>
       <td>Querying attribute support</td>