This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG04f30795f166: [clang] Implement CFG construction for @try
and @catch (authored by thakis).
Herald added a project: clang.
Changed prior to commit:
https://reviews.llvm.org/D112287?vs=381450&id=382293#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112287/new/
https://reviews.llvm.org/D112287
Files:
clang/lib/Analysis/CFG.cpp
clang/test/Sema/warn-unreachable.c
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 -fsyntax-only -fobjc-exceptions -fcxx-exceptions -verify -Wunreachable-code %s
+
+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 -fsyntax-only -fobjc-exceptions -verify -Wunreachable-code %s
+
+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/test/Sema/warn-unreachable.c
===================================================================
--- clang/test/Sema/warn-unreachable.c
+++ clang/test/Sema/warn-unreachable.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify -fblocks -Wunreachable-code-aggressive -Wno-unused-value -Wno-covered-switch-default -I %S/Inputs
+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wunreachable-code-aggressive -Wno-unused-value -Wno-covered-switch-default -I %S/Inputs %s
// RUN: %clang_cc1 -fsyntax-only -fblocks -Wunreachable-code-aggressive -Wno-unused-value -Wno-covered-switch-default -fdiagnostics-parseable-fixits -I %S/Inputs %s 2>&1 | FileCheck %s
#include "warn-unreachable.h"
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 a C++ try, an Objective-C @try, or an 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));
@@ -3253,8 +3255,7 @@
Succ = SEHTrySuccessor;
// Save the current "__try" context.
- SaveAndRestore<CFGBlock *> save_try(TryTerminatedBlock,
- NewTryTerminatedBlock);
+ SaveAndRestore<CFGBlock *> SaveTry(TryTerminatedBlock, NewTryTerminatedBlock);
cfg->addTryDispatchBlock(TryTerminatedBlock);
// Save the current value for the __leave target.
@@ -3700,11 +3701,6 @@
return addStmt(S->getSynchExpr());
}
-CFGBlock *CFGBuilder::VisitObjCAtTryStmt(ObjCAtTryStmt *S) {
- // FIXME
- return NYS();
-}
-
CFGBlock *CFGBuilder::VisitPseudoObjectExpr(PseudoObjectExpr *E) {
autoCreateBlock();
@@ -3865,16 +3861,37 @@
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) {
- // FIXME: This isn't complete. We basically treat @throw like a return
- // statement.
-
// If we were in the middle of a block we stop processing that block.
if (badCFG)
return nullptr;
@@ -3882,14 +3899,78 @@
// 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 I = 0, E = Terminator->getNumCatchStmts(); I != E; ++I) {
+ // The code after the try is the implicit successor.
+ Succ = TrySuccessor;
+ ObjCAtCatchStmt *CS = Terminator->getCatchStmt(I);
+ 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 *> SaveTry(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);
@@ -4328,7 +4409,8 @@
if (badCFG)
return nullptr;
TrySuccessor = Block;
- } else TrySuccessor = Succ;
+ } else
+ TrySuccessor = Succ;
CFGBlock *PrevTryTerminatedBlock = TryTerminatedBlock;
@@ -4364,7 +4446,7 @@
Succ = TrySuccessor;
// Save the current "try" context.
- SaveAndRestore<CFGBlock*> save_try(TryTerminatedBlock, NewTryTerminatedBlock);
+ SaveAndRestore<CFGBlock *> SaveTry(TryTerminatedBlock, NewTryTerminatedBlock);
cfg->addTryDispatchBlock(TryTerminatedBlock);
assert(Terminator->getTryBlock() && "try must contain a non-NULL body");
@@ -5317,13 +5399,11 @@
Terminator->getCond()->printPretty(OS, Helper, Policy);
}
- void VisitCXXTryStmt(CXXTryStmt *CS) {
- OS << "try ...";
- }
+ void VisitCXXTryStmt(CXXTryStmt *) { OS << "try ..."; }
- void VisitSEHTryStmt(SEHTryStmt *CS) {
- OS << "__try ...";
- }
+ void VisitObjCAtTryStmt(ObjCAtTryStmt *) { OS << "@try ..."; }
+
+ void VisitSEHTryStmt(SEHTryStmt *CS) { OS << "__try ..."; }
void VisitAbstractConditionalOperator(AbstractConditionalOperator* C) {
if (Stmt *Cond = C->getCond())
@@ -5639,7 +5719,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";
@@ -5698,6 +5779,13 @@
else
OS << "...";
OS << ")";
+ } else if (ObjCAtCatchStmt *CS = dyn_cast<ObjCAtCatchStmt>(Label)) {
+ OS << "@catch (";
+ if (const VarDecl *PD = CS->getCatchParamDecl())
+ PD->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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
