r204307 - [-Wunreachable-code] Simplify and broad -Wunreachable-code-return, including nontrivial returns.
Ted Kremenek
kremenek at apple.com
Wed Mar 19 23:07:31 PDT 2014
Author: kremenek
Date: Thu Mar 20 01:07:30 2014
New Revision: 204307
URL: http://llvm.org/viewvc/llvm-project?rev=204307&view=rev
Log:
[-Wunreachable-code] Simplify and broad -Wunreachable-code-return, including nontrivial returns.
The exception is return statements that include control-flow,
which are clearly doing something "interesting".
99% of the cases I examined for -Wunreachable-code that fired
on return statements were not interesting enough to warrant
being in -Wunreachable-code by default. Thus the move to
include them in -Wunreachable-code-return.
This simplifies a bunch of logic, including removing the ad hoc
logic to look for std::string literals.
Modified:
cfe/trunk/include/clang/Analysis/Analyses/ReachableCode.h
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Analysis/ReachableCode.cpp
cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
cfe/trunk/test/SemaCXX/warn-unreachable.cpp
Modified: cfe/trunk/include/clang/Analysis/Analyses/ReachableCode.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/Analyses/ReachableCode.h?rev=204307&r1=204306&r2=204307&view=diff
==============================================================================
--- cfe/trunk/include/clang/Analysis/Analyses/ReachableCode.h (original)
+++ cfe/trunk/include/clang/Analysis/Analyses/ReachableCode.h Thu Mar 20 01:07:30 2014
@@ -39,7 +39,7 @@ namespace reachable_code {
/// Classifications of unreachable code.
enum UnreachableKind {
- UK_TrivialReturn,
+ UK_Return,
UK_Break,
UK_Other
};
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=204307&r1=204306&r2=204307&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Mar 20 01:07:30 2014
@@ -362,7 +362,7 @@ def warn_suggest_noreturn_block : Warnin
// Unreachable code.
def warn_unreachable : Warning<
- "will never be executed">,
+ "code will never be executed">,
InGroup<UnreachableCode>, DefaultIgnore;
def warn_unreachable_break : Warning<
"'break' will never be executed">,
Modified: cfe/trunk/lib/Analysis/ReachableCode.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/ReachableCode.cpp?rev=204307&r1=204306&r2=204307&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/ReachableCode.cpp (original)
+++ cfe/trunk/lib/Analysis/ReachableCode.cpp Thu Mar 20 01:07:30 2014
@@ -18,6 +18,7 @@
#include "clang/AST/ExprCXX.h"
#include "clang/AST/ExprObjC.h"
#include "clang/AST/StmtCXX.h"
+#include "clang/AST/ParentMap.h"
#include "clang/Analysis/AnalysisContext.h"
#include "clang/Analysis/CFG.h"
#include "clang/Basic/SourceManager.h"
@@ -37,53 +38,6 @@ static bool isEnumConstant(const Expr *E
return isa<EnumConstantDecl>(DR->getDecl());
}
-static const Expr *stripStdStringCtor(const Expr *Ex) {
- // Go crazy pattern matching an implicit construction of std::string("").
- const ExprWithCleanups *EWC = dyn_cast<ExprWithCleanups>(Ex);
- if (!EWC)
- return 0;
- const CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(EWC->getSubExpr());
- if (!CCE)
- return 0;
- QualType Ty = CCE->getType();
- if (const ElaboratedType *ET = dyn_cast<ElaboratedType>(Ty))
- Ty = ET->getNamedType();
- const TypedefType *TT = dyn_cast<TypedefType>(Ty);
- StringRef Name = TT->getDecl()->getName();
- if (Name != "string")
- return 0;
- if (CCE->getNumArgs() != 1)
- return 0;
- const MaterializeTemporaryExpr *MTE =
- dyn_cast<MaterializeTemporaryExpr>(CCE->getArg(0));
- if (!MTE)
- return 0;
- CXXBindTemporaryExpr *CBT =
- dyn_cast<CXXBindTemporaryExpr>(MTE->GetTemporaryExpr()->IgnoreParenCasts());
- if (!CBT)
- return 0;
- Ex = CBT->getSubExpr()->IgnoreParenCasts();
- CCE = dyn_cast<CXXConstructExpr>(Ex);
- if (!CCE)
- return 0;
- if (CCE->getNumArgs() != 1)
- return 0;
- return dyn_cast<StringLiteral>(CCE->getArg(0)->IgnoreParenCasts());
-}
-
-/// Strip away "sugar" around trivial expressions that are for the
-/// purpose of this analysis considered uninteresting for dead code warnings.
-static const Expr *stripExprSugar(const Expr *Ex) {
- Ex = Ex->IgnoreParenCasts();
- // If 'Ex' is a constructor for a std::string, strip that
- // away. We can only get here if the trivial expression was
- // something like a C string literal, with the std::string
- // just wrapping that value.
- if (const Expr *StdStringVal = stripStdStringCtor(Ex))
- return StdStringVal;
- return Ex;
-}
-
static bool isTrivialExpression(const Expr *Ex) {
Ex = Ex->IgnoreParenCasts();
return isa<IntegerLiteral>(Ex) || isa<StringLiteral>(Ex) ||
@@ -104,7 +58,7 @@ static bool isTrivialDoWhile(const CFGBl
return false;
}
-static bool isTrivialReturn(const CFGBlock *B, const Stmt *S) {
+static bool isDeadReturn(const CFGBlock *B, const Stmt *S) {
// Look to see if the block ends with a 'return', and see if 'S'
// is a substatement. The 'return' may not be the last element in
// the block because of destructors.
@@ -112,13 +66,19 @@ static bool isTrivialReturn(const CFGBlo
I != E; ++I) {
if (Optional<CFGStmt> CS = I->getAs<CFGStmt>()) {
if (const ReturnStmt *RS = dyn_cast<ReturnStmt>(CS->getStmt())) {
- // Determine if we need to lock at the body of the block
- // before the dead return.
if (RS == S)
return true;
if (const Expr *RE = RS->getRetValue()) {
- RE = stripExprSugar(RE->IgnoreParenCasts());
- return RE == S && isTrivialExpression(RE);
+ RE = RE->IgnoreParenCasts();
+ if (RE == S)
+ return true;
+ ParentMap PM(const_cast<Expr*>(RE));
+ // If 'S' is in the ParentMap, it is a subexpression of
+ // the return statement. Note also that we are restricting
+ // to looking at return statements in the same CFGBlock,
+ // so this will intentionally not catch cases where the
+ // return statement contains nested control-flow.
+ return PM.getParent(S);
}
}
break;
@@ -547,28 +507,18 @@ static SourceLocation GetUnreachableLoc(
void DeadCodeScan::reportDeadCode(const CFGBlock *B,
const Stmt *S,
clang::reachable_code::Callback &CB) {
- // The kind of unreachable code found.
+ // Classify the unreachable code found, or suppress it in some cases.
reachable_code::UnreachableKind UK = reachable_code::UK_Other;
- do {
- // Suppress idiomatic cases of calling a noreturn function just
- // before executing a 'break'. If there is other code after the 'break'
- // in the block then don't suppress the warning.
- if (isa<BreakStmt>(S)) {
- UK = reachable_code::UK_Break;
- break;
- }
-
- if (isTrivialDoWhile(B, S))
- return;
-
- // Suppress trivial 'return' statements that are dead.
- if (isTrivialReturn(B, S)) {
- UK = reachable_code::UK_TrivialReturn;
- break;
- }
-
- } while(false);
+ if (isa<BreakStmt>(S)) {
+ UK = reachable_code::UK_Break;
+ }
+ else if (isTrivialDoWhile(B, S)) {
+ return;
+ }
+ else if (isDeadReturn(B, S)) {
+ UK = reachable_code::UK_Return;
+ }
SourceRange R1, R2;
SourceLocation Loc = GetUnreachableLoc(S, R1, R2);
Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=204307&r1=204306&r2=204307&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original)
+++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Thu Mar 20 01:07:30 2014
@@ -73,7 +73,7 @@ namespace {
case reachable_code::UK_Break:
diag = diag::warn_unreachable_break;
break;
- case reachable_code::UK_TrivialReturn:
+ case reachable_code::UK_Return:
diag = diag::warn_unreachable_return;
break;
case reachable_code::UK_Other:
Modified: cfe/trunk/test/SemaCXX/warn-unreachable.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unreachable.cpp?rev=204307&r1=204306&r2=204307&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-unreachable.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unreachable.cpp Thu Mar 20 01:07:30 2014
@@ -218,3 +218,19 @@ int test_treat_non_const_bool_local_as_n
return 0;
}
+class Frobozz {
+public:
+ Frobozz(int x);
+ ~Frobozz();
+};
+
+Frobozz test_return_object(int flag) {
+ return Frobozz(flag);
+ return Frobozz(42); // expected-warning {{'return' will never be executed}}
+}
+
+Frobozz test_return_object_control_flow(int flag) {
+ return Frobozz(flag);
+ return Frobozz(flag ? 42 : 24); // expected-warning {{code will never be executed}}
+}
+
More information about the cfe-commits
mailing list