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

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: hans.
thakis requested review of this revision.

After 04f30795f16638 <https://reviews.llvm.org/rG04f30795f1663843b219393040946136786aa591>, -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.


https://reviews.llvm.org/D114660

Files:
  clang/lib/Sema/AnalysisBasedWarnings.cpp
  clang/test/SemaObjC/return-noreturn.m


Index: clang/test/SemaObjC/return-noreturn.m
===================================================================
--- /dev/null
+++ 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;
+}
+
Index: clang/lib/Sema/AnalysisBasedWarnings.cpp
===================================================================
--- clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -464,7 +464,7 @@
     // 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;
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114660.390132.patch
Type: text/x-patch
Size: 982 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211127/8a4b199c/attachment.bin>