From e1f53884e483c92c497eab5f12f479c136f6ba94 Mon Sep 17 00:00:00 2001 From: Ted Kremenek Date: Sat, 29 Mar 2014 00:35:20 +0000 Subject: [PATCH] Improve -Wunreachable-code to provide a means to indicate code is intentionally marked dead via if((0)). Taking a hint from -Wparentheses, use an extra '()' as a sigil that a dead condition is intentionally dead. For example: if ((0)) { dead } When this sigil is found, do not emit a dead code warning. When the analysis sees: if (0) it suggests inserting '()' as a Fix-It. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@205069 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../clang/Analysis/Analyses/ReachableCode.h | 4 +- include/clang/Analysis/CFG.h | 6 +- include/clang/Basic/DiagnosticSemaKinds.td | 2 + lib/Analysis/CFG.cpp | 5 +- lib/Analysis/ReachableCode.cpp | 62 ++++++++++++++----- lib/Sema/AnalysisBasedWarnings.cpp | 15 ++++- test/Sema/warn-unreachable.c | 35 ++++++++++- 7 files changed, 107 insertions(+), 22 deletions(-) diff --git a/include/clang/Analysis/Analyses/ReachableCode.h b/include/clang/Analysis/Analyses/ReachableCode.h index a328ea2c8f..90a6d014f5 100644 --- a/include/clang/Analysis/Analyses/ReachableCode.h +++ b/include/clang/Analysis/Analyses/ReachableCode.h @@ -50,7 +50,9 @@ class Callback { public: virtual ~Callback() {} virtual void HandleUnreachable(UnreachableKind UK, - SourceLocation L, SourceRange R1, + SourceLocation L, + SourceRange ConditionVal, + SourceRange R1, SourceRange R2) = 0; }; diff --git a/include/clang/Analysis/CFG.h b/include/clang/Analysis/CFG.h index c05d440ef3..123d1443a2 100644 --- a/include/clang/Analysis/CFG.h +++ b/include/clang/Analysis/CFG.h @@ -623,10 +623,10 @@ public: CFGTerminator getTerminator() { return Terminator; } const CFGTerminator getTerminator() const { return Terminator; } - Stmt *getTerminatorCondition(); + Stmt *getTerminatorCondition(bool StripParens = true); - const Stmt *getTerminatorCondition() const { - return const_cast(this)->getTerminatorCondition(); + const Stmt *getTerminatorCondition(bool StripParens = true) const { + return const_cast(this)->getTerminatorCondition(StripParens); } const Stmt *getLoopTarget() const { return LoopTarget; } diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index f7efbbd2ef..704bf1f105 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -373,6 +373,8 @@ def warn_unreachable_return : Warning< def warn_unreachable_loop_increment : Warning< "loop will run at most once (loop increment never executed)">, InGroup, DefaultIgnore; +def note_unreachable_silence : Note< + "silence by adding parentheses to mark code as explicitly dead">; /// Built-in functions. def ext_implicit_lib_function_decl : ExtWarn< diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp index ad5cf69cd7..13ac92e86f 100644 --- a/lib/Analysis/CFG.cpp +++ b/lib/Analysis/CFG.cpp @@ -4112,7 +4112,7 @@ void CFGBlock::printTerminator(raw_ostream &OS, TPrinter.print(getTerminator()); } -Stmt *CFGBlock::getTerminatorCondition() { +Stmt *CFGBlock::getTerminatorCondition(bool StripParens) { Stmt *Terminator = this->Terminator; if (!Terminator) return NULL; @@ -4171,6 +4171,9 @@ Stmt *CFGBlock::getTerminatorCondition() { return Terminator; } + if (!StripParens) + return E; + return E ? E->IgnoreParens() : NULL; } diff --git a/lib/Analysis/ReachableCode.cpp b/lib/Analysis/ReachableCode.cpp index 53e03de64e..7c8ea39dc5 100644 --- a/lib/Analysis/ReachableCode.cpp +++ b/lib/Analysis/ReachableCode.cpp @@ -133,10 +133,17 @@ static bool isConfigurationValue(const ValueDecl *D, Preprocessor &PP); /// those blocks. static bool isConfigurationValue(const Stmt *S, Preprocessor &PP, - bool IncludeIntegers = true) { + SourceRange *SilenceableCondVal = nullptr, + bool IncludeIntegers = true, + bool WrappedInParens = false) { if (!S) return false; + // Special case looking for the sigil '()' around an integer literal. + if (const ParenExpr *PE = dyn_cast(S)) + return isConfigurationValue(PE->getSubExpr(), PP, SilenceableCondVal, + IncludeIntegers, true); + if (const Expr *Ex = dyn_cast(S)) S = Ex->IgnoreParenCasts(); @@ -148,9 +155,15 @@ static bool isConfigurationValue(const Stmt *S, } case Stmt::DeclRefExprClass: return isConfigurationValue(cast(S)->getDecl(), PP); - case Stmt::IntegerLiteralClass: - return IncludeIntegers ? isExpandedFromConfigurationMacro(S, PP) - : false; + case Stmt::IntegerLiteralClass: { + const IntegerLiteral *E = cast(S); + if (IncludeIntegers) { + if (SilenceableCondVal && !SilenceableCondVal->getBegin().isValid()) + *SilenceableCondVal = E->getSourceRange(); + return WrappedInParens || isExpandedFromConfigurationMacro(E, PP); + } + return false; + } case Stmt::MemberExprClass: return isConfigurationValue(cast(S)->getMemberDecl(), PP); case Stmt::ObjCBoolLiteralExprClass: @@ -163,13 +176,18 @@ static bool isConfigurationValue(const Stmt *S, // values if they are used in a logical or comparison operator // (not arithmetic). IncludeIntegers &= (B->isLogicalOp() || B->isComparisonOp()); - return isConfigurationValue(B->getLHS(), PP, IncludeIntegers) || - isConfigurationValue(B->getRHS(), PP, IncludeIntegers); + return isConfigurationValue(B->getLHS(), PP, SilenceableCondVal, + IncludeIntegers) || + isConfigurationValue(B->getRHS(), PP, SilenceableCondVal, + IncludeIntegers); } case Stmt::UnaryOperatorClass: { const UnaryOperator *UO = cast(S); + if (SilenceableCondVal) + *SilenceableCondVal = UO->getSourceRange(); return UO->getOpcode() == UO_LNot && - isConfigurationValue(UO->getSubExpr(), PP); + isConfigurationValue(UO->getSubExpr(), PP, SilenceableCondVal, + IncludeIntegers, WrappedInParens); } default: return false; @@ -204,11 +222,13 @@ static bool shouldTreatSuccessorsAsReachable(const CFGBlock *B, if (isa(Term)) return true; // Specially handle '||' and '&&'. - if (isa(Term)) + if (isa(Term)) { return isConfigurationValue(Term, PP); + } } - return isConfigurationValue(B->getTerminatorCondition(), PP); + const Stmt *Cond = B->getTerminatorCondition(/* stripParens */ false); + return isConfigurationValue(Cond, PP); } static unsigned scanFromBlock(const CFGBlock *Start, @@ -314,9 +334,9 @@ namespace { const Stmt *findDeadCode(const CFGBlock *Block); void reportDeadCode(const CFGBlock *B, - const Stmt *S, - clang::reachable_code::Callback &CB); - }; + const Stmt *S, + clang::reachable_code::Callback &CB); + }; } void DeadCodeScan::enqueue(const CFGBlock *block) { @@ -529,6 +549,8 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B, UK = reachable_code::UK_Return; } + SourceRange SilenceableCondVal; + if (UK == reachable_code::UK_Other) { // Check if the dead code is part of the "loop target" of // a for/for-range loop. This is the block that contains @@ -544,14 +566,26 @@ void DeadCodeScan::reportDeadCode(const CFGBlock *B, } CB.HandleUnreachable(reachable_code::UK_Loop_Increment, - Loc, SourceRange(Loc, Loc), R2); + Loc, SourceRange(), SourceRange(Loc, Loc), R2); return; } + + // Check if the dead block has a predecessor whose branch has + // a configuration value that *could* be modified to + // silence the warning. + CFGBlock::const_pred_iterator PI = B->pred_begin(); + if (PI != B->pred_end()) { + if (const CFGBlock *PredBlock = PI->getPossiblyUnreachableBlock()) { + const Stmt *TermCond = + PredBlock->getTerminatorCondition(/* strip parens */ false); + isConfigurationValue(TermCond, PP, &SilenceableCondVal); + } + } } SourceRange R1, R2; SourceLocation Loc = GetUnreachableLoc(S, R1, R2); - CB.HandleUnreachable(UK, Loc, R1, R2); + CB.HandleUnreachable(UK, Loc, SilenceableCondVal, R1, R2); } //===----------------------------------------------------------------------===// diff --git a/lib/Sema/AnalysisBasedWarnings.cpp b/lib/Sema/AnalysisBasedWarnings.cpp index efaf966562..75d7060e2d 100644 --- a/lib/Sema/AnalysisBasedWarnings.cpp +++ b/lib/Sema/AnalysisBasedWarnings.cpp @@ -66,7 +66,9 @@ namespace { UnreachableCodeHandler(Sema &s) : S(s) {} void HandleUnreachable(reachable_code::UnreachableKind UK, - SourceLocation L, SourceRange R1, + SourceLocation L, + SourceRange SilenceableCondVal, + SourceRange R1, SourceRange R2) override { unsigned diag = diag::warn_unreachable; switch (UK) { @@ -84,6 +86,17 @@ namespace { } S.Diag(L, diag) << R1 << R2; + + SourceLocation Open = SilenceableCondVal.getBegin(); + if (Open.isValid()) { + SourceLocation Close = SilenceableCondVal.getEnd(); + Close = S.PP.getLocForEndOfToken(Close); + if (Close.isValid()) { + S.Diag(Open, diag::note_unreachable_silence) + << FixItHint::CreateInsertion(Open, "/* DISABLES CODE */ (") + << FixItHint::CreateInsertion(Close, ")"); + } + } } }; } diff --git a/test/Sema/warn-unreachable.c b/test/Sema/warn-unreachable.c index 9cd87a7343..0d22965957 100644 --- a/test/Sema/warn-unreachable.c +++ b/test/Sema/warn-unreachable.c @@ -257,7 +257,7 @@ int test_config_constant(int x) { calledFun(); // no-warning return 1; } - if (!1) { + if (!1) { // expected-note {{silence by adding parentheses to mark code as explicitly dead}} calledFun(); // expected-warning {{will never be executed}} return 1; } @@ -268,7 +268,7 @@ int test_config_constant(int x) { if (x > 10) return CONFIG_CONSTANT ? calledFun() : calledFun(); // no-warning else - return 1 ? + return 1 ? // expected-note {{silence by adding parentheses to mark code as explicitly dead}} calledFun() : calledFun(); // expected-warning {{will never be executed}} } @@ -365,3 +365,34 @@ int test_break_preceded_by_noreturn_SUPPRESSED(int i) { } #pragma clang diagnostic pop + +// Test "silencing" with parentheses. +void test_with_paren_silencing(int x) { + if (0) calledFun(); // expected-warning {{will never be executed}} expected-note {{silence by adding parentheses to mark code as explicitly dead}} + if ((0)) calledFun(); // no-warning + + if (1) // expected-note {{silence by adding parentheses to mark code as explicitly dead}} + calledFun(); + else + calledFun(); // expected-warning {{will never be executed}} + + if ((1)) + calledFun(); + else + calledFun(); // no-warning + + if (!1) // expected-note {{silence by adding parentheses to mark code as explicitly dead}} + calledFun(); // expected-warning {{code will never be executed}} + else + calledFun(); + + if ((!1)) + calledFun(); // no-warning + else + calledFun(); + + if (!(1)) + calledFun(); // no-warning + else + calledFun(); +} -- 2.40.0