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 <rich...@metafoo.co.uk> wrote: > Seems sensible to put this in clang 4. > > ---------- Forwarded message ---------- > From: Richard Smith via cfe-commits <cfe-commits@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@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
