[cfe-commits] r117220 - in /cfe/trunk: lib/Analysis/CFG.cpp test/Sema/warn-unreachable.c

[Zhongxing Xu]

On Tue, Oct 26, 2010 at 6:02 AM, Ted Kremenek <kremenek at apple.com> wrote:

>
> On Oct 24, 2010, at 1:21 AM, Marcin Swiderski wrote:
>
> Modified: cfe/trunk/lib/Analysis/CFG.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=117220&r1=117219&r2=117220&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Analysis/CFG.cpp (original)
> +++ cfe/trunk/lib/Analysis/CFG.cpp Sun Oct 24 03:21:40 2010
> @@ -912,15 +912,17 @@
>       AppendStmt(Block, B, asc);
>     }
>
> -    // If visiting RHS causes us to finish 'Block' and the LHS doesn't
> -    // create a new block, then we should return RBlock.  Otherwise
> -    // we'll incorrectly return NULL.
> -    CFGBlock *RBlock = Visit(B->getRHS());
> -    CFGBlock *LBlock = Visit(B->getLHS(),
> AddStmtChoice::AsLValueNotAlwaysAdd);
> -    return LBlock ? LBlock : RBlock;
> +    Visit(B->getLHS(), AddStmtChoice::AsLValueNotAlwaysAdd);
> +    return Visit(B->getRHS());
>   }
>
>
> Hi Marcin,
>
> Shouldn't we invert the check that we had before?  Visit(B->getRHS()) isn't
> guaranteed to return a non-NULL block if visiting the LHS resulted in
> finishing up a CFGBlock.  That was the purpose of the previous logic (when
> the LHS was visited first).
>
>
Only statements containing control flow could cause 'Block' to be NULL, e.g.
DoStmt, WhileStmt, CastStmt. They do occur in the RHS of assignments, but
they do not occur in the LHS of assignments. So I think it's safe here. Or I
could miss something?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20101026/93e6141d/attachment.html>