llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Youssed Mohamed Abdul-Sataar (youssef47048) <details> <summary>Changes</summary> This patch fixes Issue #<!-- -->180429. **The Problem:** Clang's diagnostic logic for multiple `default` labels was order-dependent. In scenarios where the `SwitchCase` list is built in reverse order, the loop would identify the *first* default label as the duplicate rather than the *second* one. **The Fix:** This patch uses `SourceManager::isBeforeInTranslationUnit` to strictly identify which default label appears later in the source code. The error is now always reported on the physically later label, regardless of AST construction order. **Test Plan:** - Added `clang/test/SemaCXX/switch-default-loc.cpp` to verify the fix in C++. - Verified existing `clang/test/Sema/switch.c` passes (ensuring no regression in C). Fixes #<!-- -->180429 --- Full diff: https://github.com/llvm/llvm-project/pull/180447.diff 2 Files Affected: - (modified) clang/lib/Sema/SemaStmt.cpp (+18-4) - (added) clang/test/SemaCXX/switch-default-loc.cpp (+9) ``````````diff diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp index ba5ba80d6a0bc..cf6e4d6df8238 100644 --- a/clang/lib/Sema/SemaStmt.cpp +++ b/clang/lib/Sema/SemaStmt.cpp @@ -1366,11 +1366,26 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch, // dependent. for (SwitchCase *SC = SS->getSwitchCaseList(); SC && !HasDependentValue; SC = SC->getNextSwitchCase()) { - if (DefaultStmt *DS = dyn_cast<DefaultStmt>(SC)) { if (TheDefaultStmt) { - Diag(DS->getDefaultLoc(), diag::err_multiple_default_labels_defined); - Diag(TheDefaultStmt->getDefaultLoc(), diag::note_duplicate_case_prev); + // Determine which default statement is physically later in the source + // code. The one that is later is the duplicate/error. + DefaultStmt *ConstructedLater = DS; + DefaultStmt *ConstructedEarlier = TheDefaultStmt; + + if (SourceMgr.isBeforeInTranslationUnit( + DS->getDefaultLoc(), TheDefaultStmt->getDefaultLoc())) { + ConstructedLater = TheDefaultStmt; + ConstructedEarlier = DS; + } else { + ConstructedLater = DS; + ConstructedEarlier = TheDefaultStmt; + } + + Diag(ConstructedLater->getDefaultLoc(), + diag::err_multiple_default_labels_defined); + Diag(ConstructedEarlier->getDefaultLoc(), + diag::note_duplicate_case_prev); // FIXME: Remove the default statement from the switch block so that // we'll return a valid AST. This requires recursing down the AST and @@ -1379,7 +1394,6 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch, CaseListIsErroneous = true; } TheDefaultStmt = DS; - } else { CaseStmt *CS = cast<CaseStmt>(SC); diff --git a/clang/test/SemaCXX/switch-default-loc.cpp b/clang/test/SemaCXX/switch-default-loc.cpp new file mode 100644 index 0000000000000..c065ba4256939 --- /dev/null +++ b/clang/test/SemaCXX/switch-default-loc.cpp @@ -0,0 +1,9 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +void f(int x) { + switch (x) { + default: break; // expected-note {{previous case defined here}} + case 1: break; + default: break; // expected-error {{multiple default labels in one switch}} + } +} `````````` </details> https://github.com/llvm/llvm-project/pull/180447 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
