thakis created this revision.
thakis added a reviewer: rnk.
thakis requested review of this revision.
@finally is still not implemented.
With this, clang can emit -Wreturn-type warnings for functions containing
@try/@catch (but not yet @finally), and -Wunreachable-code also works for those
functions.
The implementation is similar to D36914 <https://reviews.llvm.org/D36914>.
Part of PR46693.
https://reviews.llvm.org/D112287
Files:
clang/lib/Analysis/CFG.cpp
clang/test/Sema/warn-unreachable.m
clang/test/Sema/warn-unreachable.mm
clang/test/SemaObjC/try-catch.m
Index: clang/test/SemaObjC/try-catch.m
===================================================================
--- clang/test/SemaObjC/try-catch.m
+++ clang/test/SemaObjC/try-catch.m
@@ -34,7 +34,12 @@
@try {}
// the exception name is optional (weird)
@catch (NSException *) {}
-}
+} // expected-warning {{non-void function does not return a value}}
+
+- (NSDictionary *)anotherFunction {
+ @try {}
+ @finally {}
+} // FIXME: This should warn about a missing return too.
@end
int foo() {
Index: clang/test/Sema/warn-unreachable.mm
===================================================================
--- /dev/null
+++ clang/test/Sema/warn-unreachable.mm
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 %s -fsyntax-only -fobjc-exceptions -fcxx-exceptions -verify -Wunreachable-code
+
+void f();
+
+void g3() {
+ try {
+ @try {
+ f();
+ throw 4; // caught by @catch, not by outer c++ catch.
+ f(); // expected-warning {{will never be executed}}
+ } @catch (...) {
+ }
+ f(); // not-unreachable
+ } catch (...) {
+ }
+}
Index: clang/test/Sema/warn-unreachable.m
===================================================================
--- /dev/null
+++ clang/test/Sema/warn-unreachable.m
@@ -0,0 +1,55 @@
+// RUN: %clang_cc1 %s -fsyntax-only -fobjc-exceptions -verify -Wunreachable-code
+
+void f();
+
+void g1() {
+ @try {
+ f();
+ @throw @"";
+ f(); // expected-warning{{will never be executed}}
+ } @catch(id i) {
+ f();
+ }
+
+ // Completely empty.
+ @try {
+ } @catch(...) {
+ }
+
+ @try {
+ f();
+ return;
+ } @catch(id i = nil) { // Catch block should not be marked as unreachable.
+ // Empty @catch body.
+ }
+}
+
+void g2() {
+ @try {
+ // Nested @try.
+ @try {
+ f();
+ @throw @"";
+ f(); // expected-warning{{will never be executed}}
+ } @catch(...) {
+ }
+ f();
+ @throw @"";
+ f(); // expected-warning{{will never be executed}}
+ } @catch(...) {
+ f();
+ }
+}
+
+void g3() {
+ @try {
+ @try {
+ f();
+ } @catch (...) {
+ @throw @""; // should exit outer try
+ }
+ @throw @"";
+ f(); // expected-warning{{never be executed}}
+ } @catch (...) {
+ }
+}
Index: clang/lib/Analysis/CFG.cpp
===================================================================
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -482,8 +482,10 @@
CFGBlock *SwitchTerminatedBlock = nullptr;
CFGBlock *DefaultCaseBlock = nullptr;
- // This can point either to a try or a __try block. The frontend forbids
- // mixing both kinds in one function, so having one for both is enough.
+ // This can point to either to a C++ try, an Objective-C @try, or a SEH __try.
+ // try and @try can be mixed and generally work the same.
+ // The frontend forbids mixing SEH __try with either try or @try.
+ // So having one for all three is enough.
CFGBlock *TryTerminatedBlock = nullptr;
// Current position in local scope.
@@ -2286,7 +2288,7 @@
return VisitObjCAtCatchStmt(cast<ObjCAtCatchStmt>(S));
case Stmt::ObjCAutoreleasePoolStmtClass:
- return VisitObjCAutoreleasePoolStmt(cast<ObjCAutoreleasePoolStmt>(S));
+ return VisitObjCAutoreleasePoolStmt(cast<ObjCAutoreleasePoolStmt>(S));
case Stmt::ObjCAtSynchronizedStmtClass:
return VisitObjCAtSynchronizedStmt(cast<ObjCAtSynchronizedStmt>(S));
@@ -3700,11 +3702,6 @@
return addStmt(S->getSynchExpr());
}
-CFGBlock *CFGBuilder::VisitObjCAtTryStmt(ObjCAtTryStmt *S) {
- // FIXME
- return NYS();
-}
-
CFGBlock *CFGBuilder::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
autoCreateBlock();
@@ -3865,10 +3862,35 @@
return EntryConditionBlock;
}
-CFGBlock *CFGBuilder::VisitObjCAtCatchStmt(ObjCAtCatchStmt *S) {
- // FIXME: For now we pretend that @catch and the code it contains does not
- // exit.
- return Block;
+CFGBlock *CFGBuilder::VisitObjCAtCatchStmt(ObjCAtCatchStmt *CS) {
+ // ObjCAtCatchStmt are treated like labels, so they are the first statement
+ // in a block.
+
+ // Save local scope position because in case of exception variable ScopePos
+ // won't be restored when traversing AST.
+ SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
+
+ if (CS->getCatchBody())
+ addStmt(CS->getCatchBody());
+
+ CFGBlock *CatchBlock = Block;
+ if (!CatchBlock)
+ CatchBlock = createBlock();
+
+ appendStmt(CatchBlock, CS);
+
+ // Also add the ObjCAtCatchStmt as a label, like with regular labels.
+ CatchBlock->setLabel(CS);
+
+ // Bail out if the CFG is bad.
+ if (badCFG)
+ return nullptr;
+
+ // We set Block to NULL to allow lazy creation of a new block (if necessary)
+ Block = nullptr;
+
+ return CatchBlock;
+
}
CFGBlock *CFGBuilder::VisitObjCAtThrowStmt(ObjCAtThrowStmt *S) {
@@ -3882,14 +3904,77 @@
// Create the new block.
Block = createBlock(false);
- // The Exit block is the only successor.
- addSuccessor(Block, &cfg->getExit());
+ if (TryTerminatedBlock)
+ // The current try statement is the only successor.
+ addSuccessor(Block, TryTerminatedBlock);
+ else
+ // otherwise the Exit block is the only successor.
+ addSuccessor(Block, &cfg->getExit());
// Add the statement to the block. This may create new blocks if S contains
// control-flow (short-circuit operations).
return VisitStmt(S, AddStmtChoice::AlwaysAdd);
}
+CFGBlock *CFGBuilder::VisitObjCAtTryStmt(ObjCAtTryStmt *Terminator) {
+ // "@try"/"@catch" is a control-flow statement. Thus we stop processing the
+ // current block.
+ CFGBlock *TrySuccessor = nullptr;
+
+ if (Block) {
+ if (badCFG)
+ return nullptr;
+ TrySuccessor = Block;
+ } else TrySuccessor = Succ;
+
+ // FIXME: Implement @finally support.
+ if (Terminator->getFinallyStmt())
+ return NYS();
+
+ CFGBlock *PrevTryTerminatedBlock = TryTerminatedBlock;
+
+ // Create a new block that will contain the try statement.
+ CFGBlock *NewTryTerminatedBlock = createBlock(false);
+ // Add the terminator in the try block.
+ NewTryTerminatedBlock->setTerminator(Terminator);
+
+ bool HasCatchAll = false;
+ for (unsigned h = 0; h < Terminator->getNumCatchStmts(); ++h) {
+ // The code after the try is the implicit successor.
+ Succ = TrySuccessor;
+ ObjCAtCatchStmt *CS = Terminator->getCatchStmt(h);
+ if (CS->hasEllipsis()) {
+ HasCatchAll = true;
+ }
+ Block = nullptr;
+ CFGBlock *CatchBlock = VisitObjCAtCatchStmt(CS);
+ if (!CatchBlock)
+ return nullptr;
+ // Add this block to the list of successors for the block with the try
+ // statement.
+ addSuccessor(NewTryTerminatedBlock, CatchBlock);
+ }
+
+ // FIXME: This needs updating when @finally support is added.
+ if (!HasCatchAll) {
+ if (PrevTryTerminatedBlock)
+ addSuccessor(NewTryTerminatedBlock, PrevTryTerminatedBlock);
+ else
+ addSuccessor(NewTryTerminatedBlock, &cfg->getExit());
+ }
+
+ // The code after the try is the implicit successor.
+ Succ = TrySuccessor;
+
+ // Save the current "try" context.
+ SaveAndRestore<CFGBlock*> save_try(TryTerminatedBlock, NewTryTerminatedBlock);
+ cfg->addTryDispatchBlock(TryTerminatedBlock);
+
+ assert(Terminator->getTryBody() && "try must contain a non-NULL body");
+ Block = nullptr;
+ return addStmt(Terminator->getTryBody());
+}
+
CFGBlock *CFGBuilder::VisitObjCMessageExpr(ObjCMessageExpr *ME,
AddStmtChoice asc) {
findConstructionContextsForArguments(ME);
@@ -5317,10 +5402,14 @@
Terminator->getCond()->printPretty(OS, Helper, Policy);
}
- void VisitCXXTryStmt(CXXTryStmt *CS) {
+ void VisitCXXTryStmt(CXXTryStmt *) {
OS << "try ...";
}
+ void VisitObjCAtTryStmt(ObjCAtTryStmt *) {
+ OS << "@try ...";
+ }
+
void VisitSEHTryStmt(SEHTryStmt *CS) {
OS << "__try ...";
}
@@ -5639,7 +5728,8 @@
}
case CFGElement::Kind::TemporaryDtor: {
- const CXXBindTemporaryExpr *BT = E.castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
+ const CXXBindTemporaryExpr *BT =
+ E.castAs<CFGTemporaryDtor>().getBindTemporaryExpr();
OS << "~";
BT->getType().print(OS, PrintingPolicy(Helper.getLangOpts()));
OS << "() (Temporary object destructor)\n";
@@ -5701,6 +5791,14 @@
else
OS << "...";
OS << ")";
+ } else if (ObjCAtCatchStmt *CS = dyn_cast<ObjCAtCatchStmt>(Label)) {
+ OS << "@catch (";
+ if (CS->getCatchParamDecl())
+ CS->getCatchParamDecl()->print(OS, PrintingPolicy(Helper.getLangOpts()),
+ 0);
+ else
+ OS << "...";
+ OS << ")";
} else if (SEHExceptStmt *ES = dyn_cast<SEHExceptStmt>(Label)) {
OS << "__except (";
ES->getFilterExpr()->printPretty(OS, &Helper,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
