[Clang] [C++26] Expansion Statements (Part 9: Control Flow)

[Sirraide]

This disallows certain switch-case constructs and labels in expansion statements.

GNU local labels are supported as an extension. My reasoning for allowing this is that:

1. I’d argue it makes sense: the main reason for disallowing labels is that you’d end up w/ more than one label w/ the same name in a single function, and jumping to such a label from outside an expansion statement becomes rather ill-defined as a result (which instantiation does it jump to? what if the statement expands to nothing at all?). Both of these issues don’t exist with local labels since they aren’t scoped to the entire function and as a result can only be jumped to from inside their respective instantiation.
2. Codegen and constant evaluation for this basically just works.
3. Disallowing this in the most straight-forward manner (e.g. by just dropping the label) leads to rather poor QOI and a lot of diagnostics, so even if we wanted to disallow local labels... we’d basically still have to do everything I’m doing in this patch in addition to then issuing a diagnostic for good QOI.

[Sirraide]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• [Clang] [C++26] Expansion Statements (Part 11: Final Touches and Tests) #169690
• [Clang] [C++26] Expansion Statements (Part 10: Expansion Limit) #169689
• [Clang] [C++26] Expansion Statements (Part 9: Control Flow) #169688 👈 (View in Graphite)
• [Clang] [C++26] Expansion Statements (Part 8: Codegen) #169687
• [Clang] [C++26] Expansion Statements (Part 7: Constexpr support and tests) #169686
• [Clang] [C++26] Expansion Statements (Part 6: Destructuring Expansion Statements) #169685
• [Clang] [C++26] Expansion Statements (Part 5: Iterating Expansion Statements) #169684
• [Clang] [NFC] Expansion Statements (Part 4: for-range and ParseScope refactor) #169683
• [Clang] [C++26] Expansion Statements (Part 3: Enumerating Expansion Statements) #169682
• [Clang] [C++26] Expansion Statements (Part 2: Parsing and Parser Tests) #169681
• [Clang] [C++26] Expansion Statements (Part 1: AST) #169680
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: None (Sirraide)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/169688.diff

7 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+6)
• (modified) clang/include/clang/Sema/ScopeInfo.h (+5-1)
• (modified) clang/include/clang/Sema/Sema.h (+2-1)
• (modified) clang/lib/Parse/ParseStmt.cpp (+9-2)
• (modified) clang/lib/Sema/SemaLookup.cpp (+38-9)
• (modified) clang/lib/Sema/SemaStmt.cpp (+28-2)
• (added) clang/test/SemaCXX/cxx2c-expansion-stmts-control-flow.cpp (+117)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 0ddaa461deff5..5115c175849e1 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3706,6 +3706,12 @@ def err_expansion_stmt_invalid_init : Error<
   "cannot expand expression of type %0">;
 def err_expansion_stmt_lambda : Error<
   "cannot expand lambda closure type">;
+def err_expansion_stmt_case : Error<
+  "%select{'case'|'default'}0 belongs to 'switch' outside enclosing expansion statement">;
+def note_enclosing_switch_statement_here : Note<
+  "switch statement is here">;
+def err_expansion_stmt_label : Error<
+  "labels are not allowed in expansion statements">;
 
 def err_attribute_patchable_function_entry_invalid_section
     : Error<"section argument to 'patchable_function_entry' attribute is not "
diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h
index 4f4d38c961140..2a410bd2eab91 100644
--- a/clang/include/clang/Sema/ScopeInfo.h
+++ b/clang/include/clang/Sema/ScopeInfo.h
@@ -202,7 +202,11 @@ class FunctionScopeInfo {
 public:
   /// A SwitchStmt, along with a flag indicating if its list of case statements
   /// is incomplete (because we dropped an invalid one while parsing).
-  using SwitchInfo = llvm::PointerIntPair<SwitchStmt*, 1, bool>;
+  struct SwitchInfo : llvm::PointerIntPair<SwitchStmt *, 1, bool> {
+    DeclContext *EnclosingDC;
+    SwitchInfo(SwitchStmt *Switch, DeclContext *DC)
+        : PointerIntPair(Switch, false), EnclosingDC(DC) {}
+  };
 
   /// SwitchStack - This is the current set of active switch statements in the
   /// block.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b102544342416..82fc2875e2abf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9519,7 +9519,8 @@ class Sema final : public SemaBase {
   /// of an __label__ label name, otherwise it is a normal label definition
   /// or use.
   LabelDecl *LookupOrCreateLabel(IdentifierInfo *II, SourceLocation IdentLoc,
-                                 SourceLocation GnuLabelLoc = SourceLocation());
+                                 SourceLocation GnuLabelLoc = SourceLocation(),
+                                 bool ForLabelStmt = false);
 
   /// Perform a name lookup for a label with the specified name; this does not
   /// create a new label if the lookup fails.
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 39751c79c6852..7b0e0ff17733b 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -715,8 +715,9 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributes &Attrs,
   // identifier ':' statement
   SourceLocation ColonLoc = ConsumeToken();
 
-  LabelDecl *LD = Actions.LookupOrCreateLabel(IdentTok.getIdentifierInfo(),
-                                              IdentTok.getLocation());
+  LabelDecl *LD = Actions.LookupOrCreateLabel(
+      IdentTok.getIdentifierInfo(), IdentTok.getLocation(), /*GnuLabelLoc=*/{},
+      /*ForLabelStmt=*/true);
 
   // Read label attributes, if present.
   StmtResult SubStmt;
@@ -760,6 +761,12 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributes &Attrs,
 
   DiagnoseLabelFollowedByDecl(*this, SubStmt.get());
 
+  // If a label cannot appear here, just return the underlying statement.
+  if (!LD) {
+    Attrs.clear();
+    return SubStmt.get();
+  }
+
   Actions.ProcessDeclAttributeList(Actions.CurScope, LD, Attrs);
   Attrs.clear();
 
diff --git a/clang/lib/Sema/SemaLookup.cpp b/clang/lib/Sema/SemaLookup.cpp
index 88dcd27d45ad2..576ec6c80770e 100644
--- a/clang/lib/Sema/SemaLookup.cpp
+++ b/clang/lib/Sema/SemaLookup.cpp
@@ -4463,7 +4463,8 @@ LabelDecl *Sema::LookupExistingLabel(IdentifierInfo *II, SourceLocation Loc) {
 }
 
 LabelDecl *Sema::LookupOrCreateLabel(IdentifierInfo *II, SourceLocation Loc,
-                                     SourceLocation GnuLabelLoc) {
+                                     SourceLocation GnuLabelLoc,
+                                     bool ForLabelStmt) {
   if (GnuLabelLoc.isValid()) {
     // Local label definitions always shadow existing labels.
     auto *Res = LabelDecl::Create(Context, CurContext, Loc, II, GnuLabelLoc);
@@ -4472,15 +4473,43 @@ LabelDecl *Sema::LookupOrCreateLabel(IdentifierInfo *II, SourceLocation Loc,
     return cast<LabelDecl>(Res);
   }
 
-  // Not a GNU local label.
-  LabelDecl *Res = LookupExistingLabel(II, Loc);
-  if (!Res) {
-    // If not forward referenced or defined already, create the backing decl.
-    Res = LabelDecl::Create(Context, CurContext, Loc, II);
-    Scope *S = CurScope->getFnParent();
-    assert(S && "Not in a function?");
-    PushOnScopeChains(Res, S, true);
+  LabelDecl *Existing = LookupExistingLabel(II, Loc);
+
+  // C++26 [stmt.label]p4 An identifier label shall not be enclosed by an
+  // expansion-statement.
+  //
+  // As an extension, we allow GNU local labels since they are logically
+  // scoped to the containing block, which prevents us from ending up with
+  // multiple copies of the same label in a function after instantiation.
+  //
+  // While allowing this is slightly more complicated, it also has the nice
+  // side-effect of avoiding otherwise rather horrible diagnostics you'd get
+  // when trying to use '__label__' if we didn't support this.
+  if (ForLabelStmt && CurContext->isExpansionStmt()) {
+    if (Existing && Existing->isGnuLocal())
+      return Existing;
+
+    // Drop the label from the AST as creating it anyway would cause us to
+    // either issue various unhelpful diagnostics (if we were to declare
+    // it in the function decl context) or shadow a valid label with the
+    // same name outside the expansion statement.
+    Diag(Loc, diag::err_expansion_stmt_label);
+    return nullptr;
   }
+
+  if (Existing)
+    return Existing;
+
+  // Declare non-local labels outside any expansion statements; this is required
+  // to support jumping out of an expansion statement.
+  ContextRAII Ctx{*this, CurContext->getEnclosingNonExpansionStatementContext(),
+                  /*NewThisContext=*/false};
+
+  // Not a GNU local label. Create the backing decl.
+  auto *Res = LabelDecl::Create(Context, CurContext, Loc, II);
+  Scope *S = CurScope->getFnParent();
+  assert(S && "Not in a function?");
+  PushOnScopeChains(Res, S, true);
   return Res;
 }
 
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 47c8f9ab6725c..78114fa097f16 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -528,6 +528,25 @@ Sema::ActOnCaseExpr(SourceLocation CaseLoc, ExprResult Val) {
   return CheckAndFinish(Val.get());
 }
 
+static bool DiagnoseSwitchCaseInExpansionStmt(Sema &S, SourceLocation KwLoc,
+                                              bool IsDefault) {
+  // C++26 [stmt.expand] The compound-statement of an expansion-statement is a
+  // control-flow-limited statement.
+  //
+  // We diagnose this here rather than in JumpDiagnostics because those run
+  // after the expansion statement is instantiated, at which point we will have
+  // have already complained about duplicate case labels, which is not exactly
+  // great QOI.
+  if (S.CurContext->isExpansionStmt() &&
+      S.getCurFunction()->SwitchStack.back().EnclosingDC != S.CurContext) {
+    S.Diag(KwLoc, diag::err_expansion_stmt_case) << IsDefault;
+    S.Diag(S.getCurFunction()->SwitchStack.back().getPointer()->getSwitchLoc(),
+           diag::note_enclosing_switch_statement_here);
+    return true;
+  }
+  return false;
+}
+
 StmtResult
 Sema::ActOnCaseStmt(SourceLocation CaseLoc, ExprResult LHSVal,
                     SourceLocation DotDotDotLoc, ExprResult RHSVal,
@@ -547,6 +566,9 @@ Sema::ActOnCaseStmt(SourceLocation CaseLoc, ExprResult LHSVal,
     return StmtError();
   }
 
+  if (DiagnoseSwitchCaseInExpansionStmt(*this, CaseLoc, false))
+    return StmtError();
+
   if (LangOpts.OpenACC &&
       getCurScope()->isInOpenACCComputeConstructScope(Scope::SwitchScope)) {
     Diag(CaseLoc, diag::err_acc_branch_in_out_compute_construct)
@@ -572,6 +594,9 @@ Sema::ActOnDefaultStmt(SourceLocation DefaultLoc, SourceLocation ColonLoc,
     return SubStmt;
   }
 
+  if (DiagnoseSwitchCaseInExpansionStmt(*this, DefaultLoc, true))
+    return StmtError();
+
   if (LangOpts.OpenACC &&
       getCurScope()->isInOpenACCComputeConstructScope(Scope::SwitchScope)) {
     Diag(DefaultLoc, diag::err_acc_branch_in_out_compute_construct)
@@ -1196,8 +1221,9 @@ StmtResult Sema::ActOnStartOfSwitchStmt(SourceLocation SwitchLoc,
 
   auto *SS = SwitchStmt::Create(Context, InitStmt, Cond.get().first, CondExpr,
                                 LParenLoc, RParenLoc);
+  SS->setSwitchLoc(SwitchLoc);
   getCurFunction()->SwitchStack.push_back(
-      FunctionScopeInfo::SwitchInfo(SS, false));
+      FunctionScopeInfo::SwitchInfo(SS, CurContext));
   return SS;
 }
 
@@ -1313,7 +1339,7 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
     BodyStmt = new (Context) NullStmt(BodyStmt->getBeginLoc());
   }
 
-  SS->setBody(BodyStmt, SwitchLoc);
+  SS->setBody(BodyStmt);
 
   Expr *CondExpr = SS->getCond();
   if (!CondExpr) return StmtError();
diff --git a/clang/test/SemaCXX/cxx2c-expansion-stmts-control-flow.cpp b/clang/test/SemaCXX/cxx2c-expansion-stmts-control-flow.cpp
new file mode 100644
index 0000000000000..83a87f74a6e1d
--- /dev/null
+++ b/clang/test/SemaCXX/cxx2c-expansion-stmts-control-flow.cpp
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 %s -std=c++2c -fsyntax-only -fblocks -verify
+
+void g(int);
+
+void label() {
+  template for (auto x : {1, 2}) {
+    invalid1:; // expected-error {{labels are not allowed in expansion statements}}
+    invalid2:; // expected-error {{labels are not allowed in expansion statements}}
+    goto invalid1; // expected-error {{use of undeclared label 'invalid1'}}
+  }
+
+  template for (auto x : {1, 2}) {
+    (void) [] {
+      template for (auto x : {1, 2}) {
+        invalid3:; // expected-error {{labels are not allowed in expansion statements}}
+      }
+      ok:;
+    };
+
+    (void) ^{
+      template for (auto x : {1, 2}) {
+        invalid4:; // expected-error {{labels are not allowed in expansion statements}}
+      }
+      ok:;
+    };
+
+    struct X {
+      void f() {
+        ok:;
+      }
+    };
+  }
+
+  // GNU local labels are allowed.
+  template for (auto x : {1, 2}) {
+    __label__ a;
+    if (x == 1) goto a;
+    a:;
+    if (x == 1) goto a;
+  }
+
+  // Likewise, jumping *out* of an expansion statement is fine.
+  template for (auto x : {1, 2}) {
+    if (x == 1) goto lbl;
+    g(x);
+  }
+  lbl:;
+  template for (auto x : {1, 2}) {
+    if (x == 1) goto lbl;
+    g(x);
+  }
+
+  // Jumping into one is not possible, as local labels aren't visible
+  // outside the block that declares them, and non-local labels are invalid.
+  goto exp1; // expected-error {{use of undeclared label 'exp1'}}
+  goto exp3; // expected-error {{use of undeclared label 'exp3'}}
+  template for (auto x : {1, 2}) {
+    __label__ exp1, exp2;
+    exp1:;
+    exp2:;
+    exp3:; // expected-error {{labels are not allowed in expansion statements}}
+  }
+  goto exp2; // expected-error {{use of undeclared label 'exp2'}}
+
+  // Allow jumping from inside an expansion statement to a local label in
+  // one of its parents.
+  out1:;
+  template for (auto x : {1, 2}) {
+    __label__ x, y;
+    x:
+    goto out1;
+    goto out2;
+    template for (auto x : {3, 4}) {
+      goto x;
+      goto y;
+      goto out1;
+      goto out2;
+    }
+    y:
+  }
+  out2:;
+}
+
+
+void case_default(int i) {
+  switch (i) { // expected-note 3 {{switch statement is here}}
+    template for (auto x : {1, 2}) {
+      case 1:; // expected-error {{'case' belongs to 'switch' outside enclosing expansion statement}}
+        template for (auto x : {1, 2}) {
+          case 2:; // expected-error {{'case' belongs to 'switch' outside enclosing expansion statement}}
+        }
+      default: // expected-error {{'default' belongs to 'switch' outside enclosing expansion statement}}
+        switch (i) {  // expected-note {{switch statement is here}}
+          case 3:;
+          default:
+            template for (auto x : {1, 2}) {
+              case 4:; // expected-error {{'case' belongs to 'switch' outside enclosing expansion statement}}
+            }
+        }
+    }
+  }
+
+  template for (auto x : {1, 2}) {
+    switch (i) {
+      case 1:;
+      default:
+    }
+  }
+
+  // Ensure that we diagnose this even if the statements would be discarded.
+  switch (i) { // expected-note 2 {{switch statement is here}}
+    template for (auto x : {}) {
+      case 1:; // expected-error {{'case' belongs to 'switch' outside enclosing expansion statement}}
+      default:; // expected-error {{'default' belongs to 'switch' outside enclosing expansion statement}}
+    }
+  }
+}