https://github.com/youssef47048 updated https://github.com/llvm/llvm-project/pull/180447
>From e08bb789446d1aa95f88b353df46abafd5505228 Mon Sep 17 00:00:00 2001 From: Youssef Mohamed <[email protected]> Date: Mon, 9 Feb 2026 00:27:29 +0200 Subject: [PATCH] [Clang] Fix inverted diagnostic location for duplicate default label When a switch statement has two default labels separated by break statements, the "multiple default labels" error was pointing to the first default instead of the second one. The fallthrough case was already working correctly. After doing investigation, The root cause was in how the `ParseCaseStatement` and `ParseDefaultStatement` register themselves on the switch case list. `ParseCaseStatement` calls `ActOnCaseStmt` (which calls `addSwitchCase`) *before* parsing its sub-statement, while `ParseDefaultStatement` calls `ActOnDefaultStmt` (which calls `addSwitchCase`) *after* parsing its sub-statement. because `addSwitchCase` is (LIFO), this produce different linked list orderings depending on whether labels are separated by break(the standard switch case) or fallthrough. consider this example: ``` void test2(int z) { switch(z) { default: // expected-note {{previous case defined here}} case 1: case 2: default: break; // expected-error {{multiple default labels in one switch}} } } ``` Each statement finishes completely beforethe next one starts, so all three addSwitchCase calls happen in top-to-bottom order, and the LIFO prepend reverses them. The loop in `ActOnFinishSwitchStmt` then sees the second default first and treats it as `TheDefaultStmt`, emitting the error on the first one which is wrong. Consider this example also: ``` void f3(int z) { switch(z) { default: // expected-note {{previous case defined here}} case 1: default: // expected-error {{multiple default labels in one switch}} break; } } ``` In this case, the parsing is recursive: the outer default's sub-statement is the next label. `ParseCaseStatement` registers itself before recursing, and the inner `ParseDefaultStatement` registers after its sub-statement. The outer default registers last. This produces a list order that happens to be correct. The proposed solution is to split `ActOnDefaultStmt` the same way [ `ActOnCaseStmt` / `ActOnCaseStmtBody`] are already split: create the `DefaultStmt` and add it to the switch case list immediately after parsing the 'default:' tokens, then attach the sub-statement afterward via a new `ActOnDefaultStmtBody`. This makes the registration order consistent with `ParseCaseStatement` and gives `addSwitchCase` a uniform prepend order regardless of breaks or fallthroughs. Fixes #180429. --- clang/include/clang/Sema/Sema.h | 4 ++-- clang/lib/Parse/ParseStmt.cpp | 10 ++++++++-- clang/lib/Sema/SemaStmt.cpp | 11 ++++++++--- clang/test/Sema/switch-default.cpp | 9 +++++++++ clang/test/SemaCXX/switch.cpp | 9 +++++++++ 5 files changed, 36 insertions(+), 7 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index fe4616d89df89..76edd761df81e 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -11015,8 +11015,8 @@ class Sema final : public SemaBase { void ActOnCaseStmtBody(Stmt *CaseStmt, Stmt *SubStmt); StmtResult ActOnDefaultStmt(SourceLocation DefaultLoc, - SourceLocation ColonLoc, Stmt *SubStmt, - Scope *CurScope); + SourceLocation ColonLoc, Scope *CurScope); + void ActOnDefaultStmtBody(Stmt *S, Stmt *SubStmt); StmtResult ActOnLabelStmt(SourceLocation IdentLoc, LabelDecl *TheDecl, SourceLocation ColonLoc, Stmt *SubStmt); diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index 1a45ed66950be..78a39cc060b9e 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -937,6 +937,12 @@ StmtResult Parser::ParseDefaultStatement(ParsedStmtContext StmtCtx) { ColonLoc = ExpectedLoc; } + StmtResult Default = + Actions.ActOnDefaultStmt(DefaultLoc, ColonLoc, getCurScope()); + + if (Default.isInvalid()) + return ParseStatement(/*TrailingElseLoc=*/nullptr, StmtCtx); + StmtResult SubStmt; if (Tok.is(tok::r_brace)) { @@ -953,8 +959,8 @@ StmtResult Parser::ParseDefaultStatement(ParsedStmtContext StmtCtx) { SubStmt = Actions.ActOnNullStmt(ColonLoc); DiagnoseLabelFollowedByDecl(*this, SubStmt.get()); - return Actions.ActOnDefaultStmt(DefaultLoc, ColonLoc, - SubStmt.get(), getCurScope()); + Actions.ActOnDefaultStmtBody(Default.get(), SubStmt.get()); + return Default; } StmtResult Parser::ParseCompoundStatement(bool isStmtExpr) { diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index ba5ba80d6a0bc..55cf19ab7df9c 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -566,10 +566,10 @@ void Sema::ActOnCaseStmtBody(Stmt *S, Stmt *SubStmt) { StmtResult Sema::ActOnDefaultStmt(SourceLocation DefaultLoc, SourceLocation ColonLoc, - Stmt *SubStmt, Scope *CurScope) { + Scope *CurScope) { if (getCurFunction()->SwitchStack.empty()) { Diag(DefaultLoc, diag::err_default_not_in_switch); - return SubStmt; + return StmtError(); } if (LangOpts.OpenACC && @@ -579,11 +579,16 @@ Sema::ActOnDefaultStmt(SourceLocation DefaultLoc, SourceLocation ColonLoc, return StmtError(); } - DefaultStmt *DS = new (Context) DefaultStmt(DefaultLoc, ColonLoc, SubStmt); + DefaultStmt *DS = new (Context) DefaultStmt(DefaultLoc, ColonLoc, + /*SubStmt=*/nullptr); getCurFunction()->SwitchStack.back().getPointer()->addSwitchCase(DS); return DS; } +void Sema::ActOnDefaultStmtBody(Stmt *S, Stmt *SubStmt) { + cast<DefaultStmt>(S)->setSubStmt(SubStmt); +} + StmtResult Sema::ActOnLabelStmt(SourceLocation IdentLoc, LabelDecl *TheDecl, SourceLocation ColonLoc, Stmt *SubStmt) { diff --git a/clang/test/Sema/switch-default.cpp b/clang/test/Sema/switch-default.cpp index 32d03dae88273..5cf27b59a6410 100644 --- a/clang/test/Sema/switch-default.cpp +++ b/clang/test/Sema/switch-default.cpp @@ -16,6 +16,15 @@ int f2(int a) { return a; } +void f3(int z) { + switch(z) { + default: // expected-note {{previous case defined here}} + case 1: + default: // expected-error {{multiple default labels in one switch}} + break; + } +} + // Warn even completely covered Enum cases(GCC compatibility). enum E { A, B }; enum E check_enum(enum E e) { diff --git a/clang/test/SemaCXX/switch.cpp b/clang/test/SemaCXX/switch.cpp index c37a75bd3e8e2..e41b6941a4535 100644 --- a/clang/test/SemaCXX/switch.cpp +++ b/clang/test/SemaCXX/switch.cpp @@ -14,6 +14,15 @@ void test() { } } +void test2(int z) { + switch(z) { + default: // expected-note {{previous case defined here}} + case 1: + case 2: + default: break; // expected-error {{multiple default labels in one switch}} + } +} + // PR5518 struct A { operator int(); // expected-note{{conversion to integral type}} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
