r291964 - PR31631: fix bad CFG (and bogus warnings) when an if-statement has an init-statement and has binary operator as its condition.
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 13 15:58:16 PST 2017
Merged in r291978.
(Please keep the list cc'd on merge requests.)
Thanks,
Hans
On Fri, Jan 13, 2017 at 2:28 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> Seems sensible to put this in clang 4.
>
> ---------- Forwarded message ----------
> From: Richard Smith via cfe-commits <cfe-commits at lists.llvm.org>
> Date: 13 January 2017 at 14:16
> Subject: r291964 - PR31631: fix bad CFG (and bogus warnings) when an
> if-statement has an init-statement and has binary operator as its condition.
> To: cfe-commits at lists.llvm.org
>
>
> Author: rsmith
> Date: Fri Jan 13 16:16:41 2017
> New Revision: 291964
>
> URL: http://llvm.org/viewvc/llvm-project?rev=291964&view=rev
> Log:
> PR31631: fix bad CFG (and bogus warnings) when an if-statement has an
> init-statement and has binary operator as its condition.
>
> Modified:
> cfe/trunk/lib/Analysis/CFG.cpp
> cfe/trunk/test/SemaCXX/uninitialized.cpp
>
> Modified: cfe/trunk/lib/Analysis/CFG.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=291964&r1=291963&r2=291964&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/CFG.cpp (original)
> +++ cfe/trunk/lib/Analysis/CFG.cpp Fri Jan 13 16:16:41 2017
> @@ -2175,19 +2175,15 @@ CFGBlock *CFGBuilder::VisitIfStmt(IfStmt
> SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
>
> // Create local scope for C++17 if init-stmt if one exists.
> - if (Stmt *Init = I->getInit()) {
> - LocalScope::const_iterator BeginScopePos = ScopePos;
> + if (Stmt *Init = I->getInit())
> addLocalScopeForStmt(Init);
> - addAutomaticObjDtors(ScopePos, BeginScopePos, I);
> - }
>
> // Create local scope for possible condition variable.
> // Store scope position. Add implicit destructor.
> - if (VarDecl *VD = I->getConditionVariable()) {
> - LocalScope::const_iterator BeginScopePos = ScopePos;
> + if (VarDecl *VD = I->getConditionVariable())
> addLocalScopeForVarDecl(VD);
> - addAutomaticObjDtors(ScopePos, BeginScopePos, I);
> - }
> +
> + addAutomaticObjDtors(ScopePos, save_scope_pos.get(), I);
>
> // The block we were processing is now finished. Make it the successor
> // block.
> @@ -2256,36 +2252,39 @@ CFGBlock *CFGBuilder::VisitIfStmt(IfStmt
> // removes infeasible paths from the control-flow graph by having the
> // control-flow transfer of '&&' or '||' go directly into the then/else
> // blocks directly.
> - if (!I->getConditionVariable())
> - if (BinaryOperator *Cond =
> - dyn_cast<BinaryOperator>(I->getCond()->IgnoreParens()))
> - if (Cond->isLogicalOp())
> - return VisitLogicalOperator(Cond, I, ThenBlock, ElseBlock).first;
> -
> - // Now create a new block containing the if statement.
> - Block = createBlock(false);
> + BinaryOperator *Cond =
> + I->getConditionVariable()
> + ? nullptr
> + : dyn_cast<BinaryOperator>(I->getCond()->IgnoreParens());
> + CFGBlock *LastBlock;
> + if (Cond && Cond->isLogicalOp())
> + LastBlock = VisitLogicalOperator(Cond, I, ThenBlock, ElseBlock).first;
> + else {
> + // Now create a new block containing the if statement.
> + Block = createBlock(false);
>
> - // Set the terminator of the new block to the If statement.
> - Block->setTerminator(I);
> + // Set the terminator of the new block to the If statement.
> + Block->setTerminator(I);
>
> - // See if this is a known constant.
> - const TryResult &KnownVal = tryEvaluateBool(I->getCond());
> + // See if this is a known constant.
> + const TryResult &KnownVal = tryEvaluateBool(I->getCond());
>
> - // Add the successors. If we know that specific branches are
> - // unreachable, inform addSuccessor() of that knowledge.
> - addSuccessor(Block, ThenBlock, /* isReachable = */ !KnownVal.isFalse());
> - addSuccessor(Block, ElseBlock, /* isReachable = */ !KnownVal.isTrue());
> -
> - // Add the condition as the last statement in the new block. This may
> create
> - // new blocks as the condition may contain control-flow. Any newly
> created
> - // blocks will be pointed to be "Block".
> - CFGBlock *LastBlock = addStmt(I->getCond());
> -
> - // If the IfStmt contains a condition variable, add it and its
> - // initializer to the CFG.
> - if (const DeclStmt* DS = I->getConditionVariableDeclStmt()) {
> - autoCreateBlock();
> - LastBlock = addStmt(const_cast<DeclStmt *>(DS));
> + // Add the successors. If we know that specific branches are
> + // unreachable, inform addSuccessor() of that knowledge.
> + addSuccessor(Block, ThenBlock, /* isReachable = */
> !KnownVal.isFalse());
> + addSuccessor(Block, ElseBlock, /* isReachable = */ !KnownVal.isTrue());
> +
> + // Add the condition as the last statement in the new block. This may
> + // create new blocks as the condition may contain control-flow. Any
> newly
> + // created blocks will be pointed to be "Block".
> + LastBlock = addStmt(I->getCond());
> +
> + // If the IfStmt contains a condition variable, add it and its
> + // initializer to the CFG.
> + if (const DeclStmt* DS = I->getConditionVariableDeclStmt()) {
> + autoCreateBlock();
> + LastBlock = addStmt(const_cast<DeclStmt *>(DS));
> + }
> }
>
> // Finally, if the IfStmt contains a C++17 init-stmt, add it to the CFG.
> @@ -3078,19 +3077,15 @@ CFGBlock *CFGBuilder::VisitSwitchStmt(Sw
> SaveAndRestore<LocalScope::const_iterator> save_scope_pos(ScopePos);
>
> // Create local scope for C++17 switch init-stmt if one exists.
> - if (Stmt *Init = Terminator->getInit()) {
> - LocalScope::const_iterator BeginScopePos = ScopePos;
> + if (Stmt *Init = Terminator->getInit())
> addLocalScopeForStmt(Init);
> - addAutomaticObjDtors(ScopePos, BeginScopePos, Terminator);
> - }
>
> // Create local scope for possible condition variable.
> // Store scope position. Add implicit destructor.
> - if (VarDecl *VD = Terminator->getConditionVariable()) {
> - LocalScope::const_iterator SwitchBeginScopePos = ScopePos;
> + if (VarDecl *VD = Terminator->getConditionVariable())
> addLocalScopeForVarDecl(VD);
> - addAutomaticObjDtors(ScopePos, SwitchBeginScopePos, Terminator);
> - }
> +
> + addAutomaticObjDtors(ScopePos, save_scope_pos.get(), Terminator);
>
> if (Block) {
> if (badCFG)
>
> Modified: cfe/trunk/test/SemaCXX/uninitialized.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/uninitialized.cpp?rev=291964&r1=291963&r2=291964&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/uninitialized.cpp (original)
> +++ cfe/trunk/test/SemaCXX/uninitialized.cpp Fri Jan 13 16:16:41 2017
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -Wno-unused-value
> -Wno-unused-lambda-capture -std=c++11 -verify %s
> +// RUN: %clang_cc1 -fsyntax-only -Wall -Wuninitialized -Wno-unused-value
> -Wno-unused-lambda-capture -std=c++1z -verify %s
>
> // definitions for std::move
> namespace std {
> @@ -1437,3 +1437,13 @@ void array_capture(bool b) {
> [fname]{};
> }
> }
> +
> +void if_switch_init_stmt(int k) {
> + if (int n = 0; (n == k || k > 5)) {}
> +
> + if (int n; (n == k || k > 5)) {} // expected-warning {{uninitialized}}
> expected-note {{initialize}}
> +
> + switch (int n = 0; (n == k || k > 5)) {} // expected-warning {{boolean}}
> +
> + switch (int n; (n == k || k > 5)) {} // expected-warning
> {{uninitialized}} expected-note {{initialize}} expected-warning {{boolean}}
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list