Skip to content

[Clang] [C2y] Implement N3355 ‘Named Loops’ #152870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

Sirraide
Copy link
Member

@Sirraide Sirraide commented Aug 9, 2025

This implements support for named loops for C2y. We also make them available in earlier language modes and in C++ as an extension, with support for templates and constant evaluation (though I haven’t added support for it to the experimental interpreter because this patch is already quite long and I’m not too familiar w/ it candidly).

The basic approach is that we treat break label and continue label more or less like goto label; that is, we defer all the checking to JumpDiagnostics.cpp. This does mean that we run the jump checker on the entire function if it contains a labeled break/continue. The alternative from what I can tell would have been to create the loop statements early before we parse their body; that’s what I tried originally, but it was shaping up to be quite the large refactor. Also, breaking out of a nested loop isn’t that common anyway, so I don’t think running the jump checker on the entire function is really that big of a deal because of that.

@AaronBallman mentioned that it would be nice to have a link from e.g. a WhileStmt back to any label(s) naming it for AST matchers; I haven’t implemented anything to facilitate that yet, but my idea for that would be to add a LabelDecl* to WhileStmt, ForStmt, etc. (basically anything that can be named by a label), and then when we are done parsing a LabelStmt, we check if its child is e.g. a WhileStmt (skipping any intervening LabelStmts), and if so, we store that statement in that WhileStmt. That is, if we have a: b: c: while, then we’d ultimately store the decl for the outermost label (that being a here) in the WhileStmt; the remaining labels can be obtained by walking the AST.

Unfortunately, that does mean that every WhileStmt etc. would need to have an extra Decl*, which would be nullptr in the overwhelming majority of cases. I guess we could try and sort of pass a IsNamedByLabel flag down throughout the parser when we start parsing a LabelStmt, which might let us put it in the trailing data where it can just be omitted entirely if there is no label, but I haven’t thought this through in any amount of detail.

Alternatively, we can just not support that and require people to write labelStmt(hasName("b"), whileLoop()) or whatever (I candidly don’t know the proper AST matcher syntax for that because I basically never use AST matchers). Aaron pointed out that that doesn’t work if there are multiple labels (e.g. if we’re trying to match a loop w/ the name b in a: b: c: while), but I’m not sure whether multiple labels in a row naming the same statement is really a use case we care to support.

@Sirraide Sirraide added c2y clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen IR generation bugs: mangling, exceptions, etc. labels Aug 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 9, 2025

@llvm/pr-subscribers-clang-codegen

Author: None (Sirraide)

Changes

This implements support for named loops for C2y. We also make them available in earlier language modes and in C++ as an extension, with support for templates and constant evaluation (though I haven’t added support for it to the experimental interpreter because this patch is already quite long and I’m not too familiar w/ it candidly).

The basic approach is that we treat break label and continue label more or less like goto label; that is, we defer all the checking to JumpDiagnostics.cpp. This does mean that we run the jump checker on the entire function if it contains a labeled break/continue. The alternative from what I can tell would have been to create the loop statements early before we parse their body; that’s what I tried originally, but it was shaping up to be quite the large refactor. Also, breaking out of a nested loop isn’t that common anyway, so I don’t think running the jump checker on the entire function is really that big of a deal because of that.

@AaronBallman mentioned that it would be nice to have a link from e.g. a WhileStmt back to any label(s) naming it for AST matchers; I haven’t implemented anything to facilitate that yet, but my idea for that would be to add a LabelDecl* to WhileStmt, ForStmt, etc. (basically anything that can be named by a label), and then when we are done parsing a LabelStmt, we check if its child is e.g. a WhileStmt (skipping any intervening LabelStmts), and if so, we store that statement in that WhileStmt. That is, if we have a: b: c: while, then we’d ultimately store the decl for the outermost label (that being a here) in the WhileStmt; the remaining labels can be obtained by walking the AST.

Unfortunately, that does mean that every WhileStmt etc. would need to have an extra Decl*, which would be nullptr in the overwhelming majority of cases. I guess we could try and sort of pass a IsNamedByLabel flag down throughout the parser when we start parsing a LabelStmt, which might let us put it in the trailing data where it can just be omitted entirely if there is no label, but I haven’t thought this through in any amount of detail.

Alternatively, we can just not support that and require people to write labelStmt(hasName("b"), whileLoop()) or whatever (I candidly don’t know the proper AST matcher syntax for that because I basically never use AST matchers). Aaron pointed out that that doesn’t work if there are multiple labels (e.g. if we’re trying to match a loop w/ the name b in a: b: c: while), but I’m not sure whether multiple labels in a row naming the same statement is really a use case we care to support.


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

44 Files Affected:

  • (modified) clang-tools-extra/clangd/XRefs.cpp (+2-2)
  • (modified) clang/docs/LanguageExtensions.rst (+1)
  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/AST/JSONNodeDumper.h (+1)
  • (modified) clang/include/clang/AST/Stmt.h (+66-47)
  • (modified) clang/include/clang/AST/TextNodeDumper.h (+1)
  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+8)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+5)
  • (modified) clang/include/clang/Basic/StmtNodes.td (+5-2)
  • (modified) clang/include/clang/Parse/Parser.h (+4)
  • (modified) clang/include/clang/Sema/ScopeInfo.h (+10-3)
  • (modified) clang/include/clang/Sema/Sema.h (+4-2)
  • (modified) clang/lib/AST/ASTImporter.cpp (+18-8)
  • (modified) clang/lib/AST/ExprConstant.cpp (+66-17)
  • (modified) clang/lib/AST/JSONNodeDumper.cpp (+7)
  • (modified) clang/lib/AST/Stmt.cpp (+7)
  • (modified) clang/lib/AST/StmtPrinter.cpp (+11-2)
  • (modified) clang/lib/AST/TextNodeDumper.cpp (+20)
  • (modified) clang/lib/CodeGen/CGObjC.cpp (+1-1)
  • (modified) clang/lib/CodeGen/CGStmt.cpp (+21-7)
  • (modified) clang/lib/CodeGen/CGStmtOpenMP.cpp (+3-3)
  • (modified) clang/lib/CodeGen/CodeGenFunction.h (+6-2)
  • (modified) clang/lib/Parse/ParseStmt.cpp (+20-4)
  • (modified) clang/lib/Sema/JumpDiagnostics.cpp (+140-26)
  • (modified) clang/lib/Sema/SemaStmt.cpp (+28-6)
  • (modified) clang/lib/Sema/TreeTransform.h (+20-2)
  • (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+10-5)
  • (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+12-4)
  • (modified) clang/lib/Tooling/Syntax/BuildTree.cpp (+2-4)
  • (added) clang/test/AST/ast-dump-labeled-break-continue-json.c (+326)
  • (added) clang/test/AST/ast-dump-labeled-break-continue.c (+41)
  • (added) clang/test/AST/ast-print-labeled-break-continue.c (+29)
  • (added) clang/test/CodeGen/labeled-break-continue.c (+281)
  • (added) clang/test/CodeGenCXX/labeled-break-continue.cpp (+221)
  • (added) clang/test/CodeGenObjC/labeled-break-continue.m (+174)
  • (modified) clang/test/OpenMP/for_loop_messages.cpp (+20)
  • (added) clang/test/Parser/labeled-break-continue.c (+10)
  • (modified) clang/test/Sema/__try.c (+16)
  • (added) clang/test/Sema/labeled-break-continue.c (+146)
  • (added) clang/test/SemaCXX/labeled-break-continue-constexpr.cpp (+169)
  • (added) clang/test/SemaCXX/labeled-break-continue.cpp (+51)
  • (added) clang/test/SemaObjC/labeled-break-continue.m (+39)
  • (modified) clang/test/SemaOpenACC/no-branch-in-out.c (+23)
  • (modified) clang/www/c_status.html (+1-1)
diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 83a8b7289aec3..a98b17bd33475 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -1106,11 +1106,11 @@ class FindControlFlow : public RecursiveASTVisitor<FindControlFlow> {
     return true;
   }
   bool VisitBreakStmt(BreakStmt *B) {
-    found(Break, B->getBreakLoc());
+    found(Break, B->getKwLoc());
     return true;
   }
   bool VisitContinueStmt(ContinueStmt *C) {
-    found(Continue, C->getContinueLoc());
+    found(Continue, C->getKwLoc());
     return true;
   }
   bool VisitSwitchCase(SwitchCase *C) {
diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst
index b5bb198ca637a..7949a6adfb5f6 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1709,6 +1709,7 @@ Attributes (N2335)                                                             C
 ``#embed`` (N3017)                                                             C23           C89, C++
 Octal literals prefixed with ``0o`` or ``0O``                                  C2y           C89, C++
 ``_Countof`` (N3369, N3469)                                                    C2y           C89
+Named Loops (N3355)                                                            C2y           C89, C++
 ============================================= ================================ ============= =============
 
 Builtin type aliases
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0e9fcaa5fac6a..4d8ea6a15e30b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -103,6 +103,8 @@ C Language Changes
 
 C2y Feature Support
 ^^^^^^^^^^^^^^^^^^^
+- Clang now supports `N3355 <https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3355.htm>`_ Named Loops. This feature
+  is also available in earlier language modes and in C++ as an extension.
 
 C23 Feature Support
 ^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/JSONNodeDumper.h b/clang/include/clang/AST/JSONNodeDumper.h
index 570662b58ccf0..1c0467a45b36a 100644
--- a/clang/include/clang/AST/JSONNodeDumper.h
+++ b/clang/include/clang/AST/JSONNodeDumper.h
@@ -334,6 +334,7 @@ class JSONNodeDumper
   void VisitStringLiteral(const StringLiteral *SL);
   void VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *BLE);
 
+  void VisitLoopControlStmt(const LoopControlStmt *LS);
   void VisitIfStmt(const IfStmt *IS);
   void VisitSwitchStmt(const SwitchStmt *SS);
   void VisitCaseStmt(const CaseStmt *CS);
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index a5b0d5053003f..45930eef4f91c 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -277,24 +277,14 @@ class alignas(void *) Stmt {
     SourceLocation GotoLoc;
   };
 
-  class ContinueStmtBitfields {
-    friend class ContinueStmt;
+  class LoopControlStmtBitfields {
+    friend class LoopControlStmt;
 
     LLVM_PREFERRED_TYPE(StmtBitfields)
     unsigned : NumStmtBits;
 
-    /// The location of the "continue".
-    SourceLocation ContinueLoc;
-  };
-
-  class BreakStmtBitfields {
-    friend class BreakStmt;
-
-    LLVM_PREFERRED_TYPE(StmtBitfields)
-    unsigned : NumStmtBits;
-
-    /// The location of the "break".
-    SourceLocation BreakLoc;
+    /// The location of the "continue"/"break".
+    SourceLocation KwLoc;
   };
 
   class ReturnStmtBitfields {
@@ -1325,8 +1315,7 @@ class alignas(void *) Stmt {
     DoStmtBitfields DoStmtBits;
     ForStmtBitfields ForStmtBits;
     GotoStmtBitfields GotoStmtBits;
-    ContinueStmtBitfields ContinueStmtBits;
-    BreakStmtBitfields BreakStmtBits;
+    LoopControlStmtBitfields LoopControlStmtBits;
     ReturnStmtBitfields ReturnStmtBits;
     SwitchCaseBitfields SwitchCaseBits;
 
@@ -3056,25 +3045,42 @@ class IndirectGotoStmt : public Stmt {
   }
 };
 
-/// ContinueStmt - This represents a continue.
-class ContinueStmt : public Stmt {
+/// Base class for BreakStmt and ContinueStmt.
+class LoopControlStmt : public Stmt {
+  /// If this is a labeled break/continue, the label whose statement we're
+  /// targeting.
+  LabelDecl *TargetLabel = nullptr;
+
+  /// Location of the label, if any.
+  SourceLocation Label;
+
+protected:
+  LoopControlStmt(StmtClass Class, SourceLocation Loc) : Stmt(Class) {
+    setKwLoc(Loc);
+  }
+
+  LoopControlStmt(StmtClass Class, EmptyShell ES) : Stmt(Class, ES) {}
+
 public:
-  ContinueStmt(SourceLocation CL) : Stmt(ContinueStmtClass) {
-    setContinueLoc(CL);
+  SourceLocation getKwLoc() const { return LoopControlStmtBits.KwLoc; }
+  void setKwLoc(SourceLocation L) { LoopControlStmtBits.KwLoc = L; }
+
+  SourceLocation getBeginLoc() const { return getKwLoc(); }
+  SourceLocation getEndLoc() const {
+    return isLabeled() ? getLabelLoc() : getKwLoc();
   }
 
-  /// Build an empty continue statement.
-  explicit ContinueStmt(EmptyShell Empty) : Stmt(ContinueStmtClass, Empty) {}
+  bool isLabeled() const { return TargetLabel; }
 
-  SourceLocation getContinueLoc() const { return ContinueStmtBits.ContinueLoc; }
-  void setContinueLoc(SourceLocation L) { ContinueStmtBits.ContinueLoc = L; }
+  SourceLocation getLabelLoc() const { return Label; }
+  void setLabelLoc(SourceLocation L) { Label = L; }
 
-  SourceLocation getBeginLoc() const { return getContinueLoc(); }
-  SourceLocation getEndLoc() const { return getContinueLoc(); }
+  LabelDecl *getLabelDecl() const { return TargetLabel; }
+  void setLabelDecl(LabelDecl *S) { TargetLabel = S; }
 
-  static bool classof(const Stmt *T) {
-    return T->getStmtClass() == ContinueStmtClass;
-  }
+  /// If this is a labeled break/continue, get the loop or switch statement
+  /// that this targets.
+  Stmt *getLabelTarget() const;
 
   // Iterators
   child_range children() {
@@ -3084,35 +3090,48 @@ class ContinueStmt : public Stmt {
   const_child_range children() const {
     return const_child_range(const_child_iterator(), const_child_iterator());
   }
+
+  static bool classof(const Stmt *T) {
+    StmtClass Class = T->getStmtClass();
+    return Class == ContinueStmtClass || Class == BreakStmtClass;
+  }
 };
 
-/// BreakStmt - This represents a break.
-class BreakStmt : public Stmt {
+/// ContinueStmt - This represents a continue.
+class ContinueStmt : public LoopControlStmt {
 public:
-  BreakStmt(SourceLocation BL) : Stmt(BreakStmtClass) {
-    setBreakLoc(BL);
+  ContinueStmt(SourceLocation CL) : LoopControlStmt(ContinueStmtClass, CL) {}
+  ContinueStmt(SourceLocation CL, SourceLocation LabelLoc, LabelDecl *Target)
+      : LoopControlStmt(ContinueStmtClass, CL) {
+    setLabelLoc(LabelLoc);
+    setLabelDecl(Target);
   }
 
-  /// Build an empty break statement.
-  explicit BreakStmt(EmptyShell Empty) : Stmt(BreakStmtClass, Empty) {}
-
-  SourceLocation getBreakLoc() const { return BreakStmtBits.BreakLoc; }
-  void setBreakLoc(SourceLocation L) { BreakStmtBits.BreakLoc = L; }
-
-  SourceLocation getBeginLoc() const { return getBreakLoc(); }
-  SourceLocation getEndLoc() const { return getBreakLoc(); }
+  /// Build an empty continue statement.
+  explicit ContinueStmt(EmptyShell Empty)
+      : LoopControlStmt(ContinueStmtClass, Empty) {}
 
   static bool classof(const Stmt *T) {
-    return T->getStmtClass() == BreakStmtClass;
+    return T->getStmtClass() == ContinueStmtClass;
   }
+};
 
-  // Iterators
-  child_range children() {
-    return child_range(child_iterator(), child_iterator());
+/// BreakStmt - This represents a break.
+class BreakStmt : public LoopControlStmt {
+public:
+  BreakStmt(SourceLocation BL) : LoopControlStmt(BreakStmtClass, BL) {}
+  BreakStmt(SourceLocation CL, SourceLocation LabelLoc, LabelDecl *Target)
+      : LoopControlStmt(BreakStmtClass, CL) {
+    setLabelLoc(LabelLoc);
+    setLabelDecl(Target);
   }
 
-  const_child_range children() const {
-    return const_child_range(const_child_iterator(), const_child_iterator());
+  /// Build an empty break statement.
+  explicit BreakStmt(EmptyShell Empty)
+      : LoopControlStmt(BreakStmtClass, Empty) {}
+
+  static bool classof(const Stmt *T) {
+    return T->getStmtClass() == BreakStmtClass;
   }
 };
 
diff --git a/clang/include/clang/AST/TextNodeDumper.h b/clang/include/clang/AST/TextNodeDumper.h
index 1917a8ac29f05..324d9bc26aae0 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -255,6 +255,7 @@ class TextNodeDumper
   void VisitExpressionTemplateArgument(const TemplateArgument &TA);
   void VisitPackTemplateArgument(const TemplateArgument &TA);
 
+  void VisitLoopControlStmt(const LoopControlStmt *L);
   void VisitIfStmt(const IfStmt *Node);
   void VisitSwitchStmt(const SwitchStmt *Node);
   void VisitWhileStmt(const WhileStmt *Node);
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 0042afccba2c8..6f2498d3bc7c3 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -215,6 +215,14 @@ def warn_c23_compat_case_range : Warning<
   DefaultIgnore, InGroup<CPre2yCompat>;
 def ext_c2y_case_range : Extension<
   "case ranges are a C2y extension">, InGroup<C2y>;
+def warn_c2y_labeled_break_continue
+    : Warning<"labeled %select{'break'|'continue'}0 is incompatible with C "
+              "standards before C2y">,
+      DefaultIgnore,
+      InGroup<CPre2yCompat>;
+def ext_c2y_labeled_break_continue
+    : Extension<"labeled %select{'break'|'continue'}0 is a C2y extension">,
+      InGroup<C2y>;
 
 // Generic errors.
 def err_expected_expression : Error<"expected expression">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index cf23594201143..94647a033d497 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10796,6 +10796,11 @@ def err_continue_not_in_loop : Error<
   "'continue' statement not in loop statement">;
 def err_break_not_in_loop_or_switch : Error<
   "'break' statement not in loop or switch statement">;
+def err_break_continue_label_not_found
+    : Error<"'%select{continue|break}0' label does not name an enclosing "
+            "%select{loop|loop or 'switch'}0">;
+def err_continue_switch
+    : Error<"label of 'continue' refers to a switch statement">;
 def warn_loop_ctrl_binds_to_inner : Warning<
   "'%0' is bound to current loop, GCC binds it to the enclosing loop">,
   InGroup<GccCompat>;
diff --git a/clang/include/clang/Basic/StmtNodes.td b/clang/include/clang/Basic/StmtNodes.td
index c9c173f5c7469..046ef4f30e232 100644
--- a/clang/include/clang/Basic/StmtNodes.td
+++ b/clang/include/clang/Basic/StmtNodes.td
@@ -16,8 +16,6 @@ def DoStmt : StmtNode<Stmt>;
 def ForStmt : StmtNode<Stmt>;
 def GotoStmt : StmtNode<Stmt>;
 def IndirectGotoStmt : StmtNode<Stmt>;
-def ContinueStmt : StmtNode<Stmt>;
-def BreakStmt : StmtNode<Stmt>;
 def ReturnStmt : StmtNode<Stmt>;
 def DeclStmt  : StmtNode<Stmt>;
 def SwitchCase : StmtNode<Stmt, 1>;
@@ -26,6 +24,11 @@ def DefaultStmt : StmtNode<SwitchCase>;
 def CapturedStmt : StmtNode<Stmt>;
 def SYCLKernelCallStmt : StmtNode<Stmt>;
 
+// Break/continue.
+def LoopControlStmt : StmtNode<Stmt, 1>;
+def ContinueStmt : StmtNode<LoopControlStmt>;
+def BreakStmt : StmtNode<LoopControlStmt>;
+
 // Statements that might produce a value (for example, as the last non-null
 // statement in a GNU statement-expression).
 def ValueStmt : StmtNode<Stmt, 1>;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index e9437e6d46366..7add07c79fc64 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -7458,6 +7458,7 @@ class Parser : public CodeCompletionHandler {
   /// \verbatim
   ///       jump-statement:
   ///         'continue' ';'
+  /// [C2y]   'continue' identifier ';'
   /// \endverbatim
   ///
   /// Note: this lets the caller parse the end ';'.
@@ -7468,6 +7469,7 @@ class Parser : public CodeCompletionHandler {
   /// \verbatim
   ///       jump-statement:
   ///         'break' ';'
+  /// [C2y]   'break' identifier ';'
   /// \endverbatim
   ///
   /// Note: this lets the caller parse the end ';'.
@@ -7484,6 +7486,8 @@ class Parser : public CodeCompletionHandler {
   /// \endverbatim
   StmtResult ParseReturnStatement();
 
+  StmtResult ParseBreakOrContinueStatement(bool IsContinue);
+
   StmtResult ParsePragmaLoopHint(StmtVector &Stmts, ParsedStmtContext StmtCtx,
                                  SourceLocation *TrailingElseLoc,
                                  ParsedAttributes &Attrs);
diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h
index 94b247a689c2d..2a46edc478591 100644
--- a/clang/include/clang/Sema/ScopeInfo.h
+++ b/clang/include/clang/Sema/ScopeInfo.h
@@ -124,6 +124,9 @@ class FunctionScopeInfo {
   /// Whether this function contains any indirect gotos.
   bool HasIndirectGoto : 1;
 
+  /// Whether this function contains any labeled break or continue statements.
+  bool HasLabeledBreakOrContinue : 1;
+
   /// Whether this function contains any statement marked with
   /// \c [[clang::musttail]].
   bool HasMustTail : 1;
@@ -391,7 +394,8 @@ class FunctionScopeInfo {
 public:
   FunctionScopeInfo(DiagnosticsEngine &Diag)
       : Kind(SK_Function), HasBranchProtectedScope(false),
-        HasBranchIntoScope(false), HasIndirectGoto(false), HasMustTail(false),
+        HasBranchIntoScope(false), HasIndirectGoto(false),
+        HasLabeledBreakOrContinue(false), HasMustTail(false),
         HasDroppedStmt(false), HasOMPDeclareReductionCombiner(false),
         HasFallthroughStmt(false), UsesFPIntrin(false),
         HasPotentialAvailabilityViolations(false), ObjCShouldCallSuper(false),
@@ -436,6 +440,8 @@ class FunctionScopeInfo {
     HasBranchIntoScope = true;
   }
 
+  void setHasLabeledBreakOrContinue() { HasLabeledBreakOrContinue = true; }
+
   void setHasBranchProtectedScope() {
     HasBranchProtectedScope = true;
   }
@@ -485,8 +491,9 @@ class FunctionScopeInfo {
   }
 
   bool NeedsScopeChecking() const {
-    return !HasDroppedStmt && (HasIndirectGoto || HasMustTail ||
-                               (HasBranchProtectedScope && HasBranchIntoScope));
+    return !HasDroppedStmt &&
+           (HasIndirectGoto || HasMustTail || HasLabeledBreakOrContinue ||
+            (HasBranchProtectedScope && HasBranchIntoScope));
   }
 
   // Add a block introduced in this function.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5211373367677..7db36a64679d3 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -11033,8 +11033,10 @@ class Sema final : public SemaBase {
                            LabelDecl *TheDecl);
   StmtResult ActOnIndirectGotoStmt(SourceLocation GotoLoc,
                                    SourceLocation StarLoc, Expr *DestExp);
-  StmtResult ActOnContinueStmt(SourceLocation ContinueLoc, Scope *CurScope);
-  StmtResult ActOnBreakStmt(SourceLocation BreakLoc, Scope *CurScope);
+  StmtResult ActOnContinueStmt(SourceLocation ContinueLoc, Scope *CurScope,
+                               LabelDecl *Label, SourceLocation LabelLoc);
+  StmtResult ActOnBreakStmt(SourceLocation BreakLoc, Scope *CurScope,
+                            LabelDecl *Label, SourceLocation LabelLoc);
 
   struct NamedReturnInfo {
     const VarDecl *Candidate;
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 8e2927bdc8d6f..79583b68b4112 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -7407,18 +7407,28 @@ ExpectedStmt ASTNodeImporter::VisitIndirectGotoStmt(IndirectGotoStmt *S) {
       ToGotoLoc, ToStarLoc, ToTarget);
 }
 
+template <typename StmtClass>
+static ExpectedStmt ImportLoopControlStmt(ASTNodeImporter &NodeImporter,
+                                          ASTImporter &Importer, StmtClass *S) {
+  Error Err = Error::success();
+  auto ToLoc = NodeImporter.importChecked(Err, S->getKwLoc());
+  auto ToLabelLoc = S->isLabeled()
+                        ? NodeImporter.importChecked(Err, S->getLabelLoc())
+                        : SourceLocation();
+  auto ToDecl = S->isLabeled()
+                    ? NodeImporter.importChecked(Err, S->getLabelDecl())
+                    : nullptr;
+  if (Err)
+    return std::move(Err);
+  return new (Importer.getToContext()) StmtClass(ToLoc, ToLabelLoc, ToDecl);
+}
+
 ExpectedStmt ASTNodeImporter::VisitContinueStmt(ContinueStmt *S) {
-  ExpectedSLoc ToContinueLocOrErr = import(S->getContinueLoc());
-  if (!ToContinueLocOrErr)
-    return ToContinueLocOrErr.takeError();
-  return new (Importer.getToContext()) ContinueStmt(*ToContinueLocOrErr);
+  return ImportLoopControlStmt(*this, Importer, S);
 }
 
 ExpectedStmt ASTNodeImporter::VisitBreakStmt(BreakStmt *S) {
-  auto ToBreakLocOrErr = import(S->getBreakLoc());
-  if (!ToBreakLocOrErr)
-    return ToBreakLocOrErr.takeError();
-  return new (Importer.getToContext()) BreakStmt(*ToBreakLocOrErr);
+  return ImportLoopControlStmt(*this, Importer, S);
 }
 
 ExpectedStmt ASTNodeImporter::VisitReturnStmt(ReturnStmt *S) {
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 3679327da7b0c..264153f7508d7 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -894,6 +894,11 @@ namespace {
     /// declaration whose initializer is being evaluated, if any.
     APValue *EvaluatingDeclValue;
 
+    /// Stack of loops and 'switch' statements which we're currently
+    /// breaking/continuing; null entries are used to mark unlabeled
+    /// break/continue.
+    SmallVector<Stmt *> BreakContinueStack;
+
     /// Set of objects that are currently being constructed.
     llvm::DenseMap<ObjectUnderConstruction, ConstructionPhase>
         ObjectsUnderConstruction;
@@ -5385,6 +5390,44 @@ static EvalStmtResult EvaluateStmt(StmtResult &Result, EvalInfo &Info,
                                    const Stmt *S,
                                    const SwitchCase *SC = nullptr);
 
+/// Helper to implement labeled break/continue. Returns 'true' if the evaluation
+/// result should be propagated up. Otherwise, it sets the evaluation result
+/// to either Continue to continue the current loop, or Succeeded to break it.
+static bool ShouldPropagateBreakContinue(EvalInfo &Info,
+                                         const Stmt *LoopOrSwitch,
+                                         ArrayRef<BlockScopeRAII *> Scopes,
+                                         EvalStmtResult &ESR) {
+  bool IsSwitch = isa<SwitchStmt>(LoopOrSwitch);
+
+  // For loops, map Succeeded to Continue so we don't have to check for both.
+  if (!IsSwitch && ESR == ESR_Succeeded) {
+    ESR = ESR_Continue;
+    return false;
+  }
+
+  if (ESR != ESR_Break && ESR != ESR_Continue)
+    return false;
+
+  // Are we breaking out of or continuing this statement?
+  bool CanBreakOrContinue = !IsSwitch || ESR == ESR_Break;
+  Stmt *StackTop = Info.BreakContinueStack.back();
+  if (CanBreakOrContinue && (StackTop == nullptr || StackTop == LoopOrSwitch)) {
+    Info.BreakContinueStack.pop_back();
+    if (ESR == ESR_Break)
+      ESR = ESR_Succeeded;
+    return false;
+  }
+
+  // We're not. Propagate the result up.
+  for (BlockScopeRAII *S : Scopes) {
+    if (!S->destroy()) {
+      ESR = ESR_Failed;
+      break;
+    }
+  }
+  return true;
+}
+
 /// Evaluate the body of a loop, and translate the result as appropriate.
 static EvalStmtResult EvaluateLoopBody(StmtResult &Result, EvalInfo &Info,
                                        const Stmt *Body,
@@ -5395,18 +5438,7 @@ static EvalStmtResult EvaluateLoopBody(StmtResult &Result, EvalInfo &Info,
   if (ESR != ESR_Failed && ESR != ESR_CaseNotFound && !Scope.destroy())
     ESR = ESR_Failed;
 
-  switch (ESR) {
-  case ESR_Break:
-    return ESR_Succeeded;
-  case ESR_Succeeded:
-  case ESR_Continue:
-    return ESR_Continue;
-  case ESR_Failed:
-  case ESR_Returned:
-  case ESR_CaseNotFound:
-    return ESR;
-  }
-  llvm_unreachable("Invalid EvalStmtResult!");
+  return ESR;
 }
 
 /// Evaluate a switch statement.
@@ -5472,10 +5504,12 @@ static EvalStmtResult EvaluateSwitch(StmtResult &Result, EvalInfo &Info,
   EvalStmtResult ESR = EvaluateStmt(Result, Info, SS->getBody(), Found);
   if (ESR != ESR_Failed && ESR ...
[truncated]

@cor3ntin
Copy link
Contributor

Thanks for working on that; I will review later.

I have some concerns about exposing that more widely than in C2y for now
We are quite early in the C2y cycle, there have been proposals altering the design, and C++ has express interest in doing something maybe similar.
We should gather deployment experience before backing us, and committees in a corner.

@Sirraide
Copy link
Member Author

We are quite early in the C2y cycle, there have been proposals altering the design, and C++ has express interest in doing something maybe similar.
We should gather deployment experience before backing us, and committees in a corner.

Sure; I just wanted to make sure this implementation also works for C++ because I wanted to avoid having to refactor this if/when C++ also gets this as a feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c2y clang:codegen IR generation bugs: mangling, exceptions, etc. clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants