[clang] de401ac - [clang][Sema] Avoid duplicate diagnostics for unreachable fallthrough attribute

[Aaron Ballman via cfe-commits]

Author: Takuya Shimizu
Date: 2023-03-14T08:43:35-04:00
New Revision: de401ac2a4ab74392b8234aa3a517481a1e2762c

URL: https://github.com/llvm/llvm-project/commit/de401ac2a4ab74392b8234aa3a517481a1e2762c
DIFF: https://github.com/llvm/llvm-project/commit/de401ac2a4ab74392b8234aa3a517481a1e2762c.diff

LOG: [clang][Sema] Avoid duplicate diagnostics for unreachable fallthrough attribute

This patch checks whether -Wunreachable-code-fallthrough is enabled
when clang encounters unreachable fallthrough attributes and, if so,
suppresses code will never be executed warning to avoid duplicate
warnings.

Fixes https://github.com/llvm/llvm-project/issues/60416
Differential Revision: https://reviews.llvm.org/D145842

Added: 
    clang/test/Sema/warn-unreachable-fallthrough.c

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Analysis/Analyses/ReachableCode.h
    clang/lib/Analysis/ReachableCode.cpp
    clang/lib/Sema/AnalysisBasedWarnings.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a8c6f2f733d78..b466dfc5b84b0 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -167,6 +167,9 @@ Improvements to Clang's diagnostics
   ``<built-in>``.
 - Clang constexpr evaluator now provides a more concise diagnostic when calling
   function pointer that is known to be null.
+- Clang now avoids duplicate warnings on unreachable ``[[fallthrough]];`` statements
+  previously issued from ``-Wunreachable-code`` and ``-Wunreachable-code-fallthrough``
+  by prioritizing ``-Wunreachable-code-fallthrough``.
 
 Bug Fixes in This Version
 -------------------------

diff  --git a/clang/include/clang/Analysis/Analyses/ReachableCode.h b/clang/include/clang/Analysis/Analyses/ReachableCode.h
index 514b9458d331c..f1b63f74b6c80 100644
--- a/clang/include/clang/Analysis/Analyses/ReachableCode.h
+++ b/clang/include/clang/Analysis/Analyses/ReachableCode.h
@@ -48,11 +48,9 @@ class Callback {
   virtual void anchor();
 public:
   virtual ~Callback() {}
-  virtual void HandleUnreachable(UnreachableKind UK,
-                                 SourceLocation L,
-                                 SourceRange ConditionVal,
-                                 SourceRange R1,
-                                 SourceRange R2) = 0;
+  virtual void HandleUnreachable(UnreachableKind UK, SourceLocation L,
+                                 SourceRange ConditionVal, SourceRange R1,
+                                 SourceRange R2, bool HasFallThroughAttr) = 0;
 };
 
 /// ScanReachableFromBlock - Mark all blocks reachable from Start.

diff  --git a/clang/lib/Analysis/ReachableCode.cpp b/clang/lib/Analysis/ReachableCode.cpp
index 5cc63bb17b09e..1bf0d9aec8620 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -12,6 +12,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/Analyses/ReachableCode.h"
+#include "clang/AST/Attr.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
@@ -629,6 +630,10 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B,
     UK = reachable_code::UK_Return;
   }
 
+  const auto *AS = dyn_cast<AttributedStmt>(S);
+  bool HasFallThroughAttr =
+      AS && hasSpecificAttr<FallThroughAttr>(AS->getAttrs());
+
   SourceRange SilenceableCondVal;
 
   if (UK == reachable_code::UK_Other) {
@@ -645,8 +650,9 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B,
         R2 = Inc->getSourceRange();
       }
 
-      CB.HandleUnreachable(reachable_code::UK_Loop_Increment,
-                           Loc, SourceRange(), SourceRange(Loc, Loc), R2);
+      CB.HandleUnreachable(reachable_code::UK_Loop_Increment, Loc,
+                           SourceRange(), SourceRange(Loc, Loc), R2,
+                           HasFallThroughAttr);
       return;
     }
 
@@ -665,7 +671,7 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B,
 
   SourceRange R1, R2;
   SourceLocation Loc = GetUnreachableLoc(S, R1, R2);
-  CB.HandleUnreachable(UK, Loc, SilenceableCondVal, R1, R2);
+  CB.HandleUnreachable(UK, Loc, SilenceableCondVal, R1, R2, HasFallThroughAttr);
 }
 
 //===----------------------------------------------------------------------===//

diff  --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 436743564c258..f5c48ed89e93d 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -66,11 +66,17 @@ namespace {
   public:
     UnreachableCodeHandler(Sema &s) : S(s) {}
 
-    void HandleUnreachable(reachable_code::UnreachableKind UK,
-                           SourceLocation L,
-                           SourceRange SilenceableCondVal,
-                           SourceRange R1,
-                           SourceRange R2) override {
+    void HandleUnreachable(reachable_code::UnreachableKind UK, SourceLocation L,
+                           SourceRange SilenceableCondVal, SourceRange R1,
+                           SourceRange R2, bool HasFallThroughAttr) override {
+      // If the diagnosed code is `[[fallthrough]];` and
+      // `-Wunreachable-code-fallthrough` is  enabled, suppress `code will never
+      // be executed` warning to avoid generating diagnostic twice
+      if (HasFallThroughAttr &&
+          !S.getDiagnostics().isIgnored(diag::warn_unreachable_fallthrough_attr,
+                                        SourceLocation()))
+        return;
+
       // Avoid reporting multiple unreachable code diagnostics that are
       // triggered by the same conditional value.
       if (PreviousSilenceableCondVal.isValid() &&

diff  --git a/clang/test/Sema/warn-unreachable-fallthrough.c b/clang/test/Sema/warn-unreachable-fallthrough.c
new file mode 100644
index 0000000000000..e048a9e7e612f
--- /dev/null
+++ b/clang/test/Sema/warn-unreachable-fallthrough.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c2x -Wunreachable-code-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c2x -Wunreachable-code %s
+// RUN: %clang_cc1 -fsyntax-only -verify=code -std=c2x -Wunreachable-code -Wno-unreachable-code-fallthrough %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c2x -Wno-unreachable-code -Wunreachable-code-fallthrough %s
+
+int n;
+void f(void){
+     switch (n){
+         [[fallthrough]]; // expected-warning{{fallthrough annotation in unreachable code}}
+                          // code-warning at -1{{never be executed}}
+         case 1:;
+     }
+}