[clang] 8874ada - [clang] Fix -Wreturn-type false positive in @try statements

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-11-28T12:56:46-05:00
New Revision: 8874ada906f6a2ae0c4aa978581a0c9c26b89ffd

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

LOG: [clang] Fix -Wreturn-type false positive in @try statements

After 04f30795f16638, -Wreturn-type has an effect on functions that
contain @try/@catch statements. CheckFallThrough() was missing
a case for ObjCAtTryStmts, leading to a false positive.

(What about the other two places in CheckFallThrough() that handle
CXXTryStmt but not ObjCAtTryStmts?

- I think the last use of CXXTryStmt is dead in practice: 04c6851cd made it so
  that calls never add edges to try bodies, and the CFG block for a try
  statement is always an empty block containing just the try element itself as
  terminator (the try body itself is part of the normal flow of the function
  and not connected to the block for the try statement itself. The try statment
  cfg block is only connected to the catch bodies, and only reachable from
  throw expressions within the try body.)

- The first use of CXXTryStmt might be important. It looks similar to
  the code that adds all cfg blocks for try statements as roots of
  the reachability graph for the reachability warnings, but I can't
  find a way to trigger it. So I'm omitting it for now. The CXXTryStmt
  code path seems to only be hit by try statements that are function
  bodies without a surrounding compound statements
  (`f() try { ... } catch ...`), and those don't exist for ObjC
  @try statements.
)

Fixes PR52473.

Differential Revfision: https://reviews.llvm.org/D114660

Added: 
    clang/test/SemaObjC/return-noreturn.m

Modified: 
    clang/lib/Sema/AnalysisBasedWarnings.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 8544a4fccf4c8..b4dcc9759b990 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -464,7 +464,7 @@ static ControlFlowKind CheckFallThrough(AnalysisDeclContext &AC) {
     // No more CFGElements in the block?
     if (ri == re) {
       const Stmt *Term = B.getTerminatorStmt();
-      if (Term && isa<CXXTryStmt>(Term)) {
+      if (Term && (isa<CXXTryStmt>(Term) || isa<ObjCAtTryStmt>(Term))) {
         HasAbnormalEdge = true;
         continue;
       }

diff  --git a/clang/test/SemaObjC/return-noreturn.m b/clang/test/SemaObjC/return-noreturn.m
new file mode 100644
index 0000000000000..c7735ca211e6d
--- /dev/null
+++ b/clang/test/SemaObjC/return-noreturn.m
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fsyntax-only -fobjc-exceptions -verify -Wreturn-type -Wmissing-noreturn
+
+id f(id self) {
+} // expected-warning {{non-void function does not return a value}}
+
+id f2(id self) {
+  @try {
+    @throw (id)0;
+  } @catch (id) {
+  }
+  return (id)0;
+}
+