llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Adinesh Reddy (Adinesh-Reddy) <details> <summary>Changes</summary> ## Summary This PR fixes issue #<!-- -->230802, where Clang can report a false-positive `-Wimplicit-fallthrough` warning for consecutive switch labels when an attributed statement is present. ## What was the problem? While checking whether execution falls through from one switch case to another, Clang compares statements with switch labels. However, when a label is wrapped inside an `AttributedStmt`, the comparison may fail because Clang sees the wrapper instead of the actual statement. This can cause Clang to report a fallthrough warning even when the cases are consecutive labels and no actual fallthrough occurs. ## What did I change? - Added a helper function to skip `AttributedStmt` wrappers and access the underlying statement. - Used this helper in the fallthrough analysis where Clang checks switch labels and their relationships. - Added regression tests to cover the issue and check that genuine fallthrough warnings are still reported. ## Testing - Ran `clang/test/SemaCXX/switch-implicit-fallthrough.cpp` using `llvm-lit`. - Ran `clang/test/Sema/attr-likelihood.c` to check that the changes do not break related tests. - Tested additional cases involving consecutive labels and genuine fallthrough. All the above tests passed. Fixes #<!-- -->230802. --- Full diff: https://github.com/llvm/llvm-project/pull/234848.diff 2 Files Affected: - (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+11-3) - (modified) clang/test/SemaCXX/switch-implicit-fallthrough.cpp (+24) ``````````diff diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp index 21af6ed5527ffc..718e45a9ccb572 100644 --- a/clang/lib/Sema/AnalysisBasedWarnings.cpp +++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp @@ -1276,11 +1276,13 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor { continue; // Switch statement, good. const SwitchCase *SW = dyn_cast_or_null<SwitchCase>(P->getLabel()); - if (SW && SW->getSubStmt() == B.getLabel() && P->begin() == P->end()) + if (SW && stripAttributedStmts(SW->getSubStmt()) == B.getLabel() && + P->begin() == P->end()) continue; // Previous case label has no statements, good. const LabelStmt *L = dyn_cast_or_null<LabelStmt>(P->getLabel()); - if (L && L->getSubStmt() == B.getLabel() && P->begin() == P->end()) + if (L && stripAttributedStmts(L->getSubStmt()) == B.getLabel() && + P->begin() == P->end()) continue; // Case label is preceded with a normal label, good. if (!ReachableBlocks.count(P)) { @@ -1355,6 +1357,12 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor { private: + static const Stmt *stripAttributedStmts(const Stmt *S) { + while (const auto *AS = dyn_cast_or_null<AttributedStmt>(S)) + S = AS->getSubStmt(); + return S; + } + static const AttributedStmt *asFallThroughAttr(const Stmt *S) { if (const AttributedStmt *AS = dyn_cast_or_null<AttributedStmt>(S)) { if (hasSpecificAttr<FallThroughAttr>(AS->getAttrs())) @@ -1373,7 +1381,7 @@ class FallthroughMapper : public DynamicRecursiveASTVisitor { // case X: {} case Y: // case X: ; case Y: if (const SwitchCase *SW = dyn_cast_or_null<SwitchCase>(B.getLabel())) - if (!isa<SwitchCase>(SW->getSubStmt())) + if (!isa<SwitchCase>(stripAttributedStmts(SW->getSubStmt()))) return SW->getSubStmt(); return nullptr; diff --git a/clang/test/SemaCXX/switch-implicit-fallthrough.cpp b/clang/test/SemaCXX/switch-implicit-fallthrough.cpp index 750663717b66a4..5ca4eeb9ac9473 100644 --- a/clang/test/SemaCXX/switch-implicit-fallthrough.cpp +++ b/clang/test/SemaCXX/switch-implicit-fallthrough.cpp @@ -255,6 +255,30 @@ int fallthrough_covered_enums(Enum e) { // Fallthrough annotations in local classes used to generate "fallthrough // annotation does not directly precede switch label" warning. + +int fallthrough_attributed_labels(int n) { + switch (n) { + [[unlikely]] case 0: + [[unlikely]] case 1: + [[likely]] case 2: + case 3: + [[unlikely]] default: + return 1; + } + switch (n) { + case 4: + n++; + [[unlikely]] case 5: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}} + n++; + break; + [[unlikely]] case 6: + n++; + [[unlikely]] default: // expected-warning{{unannotated fall-through between switch labels}} expected-note{{insert '[[clang::fallthrough]];' to silence this warning}} expected-note{{insert 'break;' to avoid fall-through}} + return 2; + } + return n; +} + void fallthrough_in_local_class() { class C { void f(int x) { `````````` </details> https://github.com/llvm/llvm-project/pull/234848 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
