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;
}
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;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
