[cfe-commits] [patch] remove redundant 'if'

[Ted Kremenek]

Looks good to me.  I believe 'Succ' is guaranteed to be non-NULL here.

While you're at it, we should probably change:

  if (Block) FinishBlock(B)

to

  if (B)
    FinishBlock(B)



On Feb 21, 2010, at 1:34 AM, Zhongxing Xu wrote:

> In CFG.cpp:267, B must be non-null because we have
> 
> if (!B)
>  B = Succ;
> 
> above.
> 
> Index: lib/Analysis/CFG.cpp
> ===================================================================
> --- lib/Analysis/CFG.cpp	(版本 96732)
> +++ lib/Analysis/CFG.cpp	(工作副本)
> @@ -264,44 +264,43 @@
>   if (!B)
>     B = Succ;
> 
> -  if (B) {
> -    // Finalize the last constructed block.  This usually involves
> reversing the
> -    // order of the statements in the block.
> -    if (Block) FinishBlock(B);
> +  assert(B);
> +  // Finalize the last constructed block.  This usually involves reversing the
> +  // order of the statements in the block.
> +  if (Block) FinishBlock(B);
> 
> -    // Backpatch the gotos whose label -> block mappings we didn't know when we
> -    // encountered them.
> -    for (BackpatchBlocksTy::iterator I = BackpatchBlocks.begin(),
> +  // Backpatch the gotos whose label -> block mappings we didn't know when we
> +  // encountered them.
> +  for (BackpatchBlocksTy::iterator I = BackpatchBlocks.begin(),
>          E = BackpatchBlocks.end(); I != E; ++I ) {
> 
> -      CFGBlock* B = *I;
> -      GotoStmt* G = cast<GotoStmt>(B->getTerminator());
> -      LabelMapTy::iterator LI = LabelMap.find(G->getLabel());
> +    CFGBlock* B = *I;
> +    GotoStmt* G = cast<GotoStmt>(B->getTerminator());
> +    LabelMapTy::iterator LI = LabelMap.find(G->getLabel());
> 
> -      // If there is no target for the goto, then we are looking at an
> -      // incomplete AST.  Handle this by not registering a successor.
> -      if (LI == LabelMap.end()) continue;
> +    // If there is no target for the goto, then we are looking at an
> +    // incomplete AST.  Handle this by not registering a successor.
> +    if (LI == LabelMap.end()) continue;
> 
> -      AddSuccessor(B, LI->second);
> -    }
> +    AddSuccessor(B, LI->second);
> +  }
> 
> -    // Add successors to the Indirect Goto Dispatch block (if we have one).
> -    if (CFGBlock* B = cfg->getIndirectGotoBlock())
> -      for (LabelSetTy::iterator I = AddressTakenLabels.begin(),
> +  // Add successors to the Indirect Goto Dispatch block (if we have one).
> +  if (CFGBlock* B = cfg->getIndirectGotoBlock())
> +    for (LabelSetTy::iterator I = AddressTakenLabels.begin(),
>            E = AddressTakenLabels.end(); I != E; ++I ) {
> 
> -        // Lookup the target block.
> -        LabelMapTy::iterator LI = LabelMap.find(*I);
> +      // Lookup the target block.
> +      LabelMapTy::iterator LI = LabelMap.find(*I);
> 
> -        // If there is no target block that contains label, then we are looking
> -        // at an incomplete AST.  Handle this by not registering a successor.
> -        if (LI == LabelMap.end()) continue;
> +      // If there is no target block that contains label, then we are looking
> +      // at an incomplete AST.  Handle this by not registering a successor.
> +      if (LI == LabelMap.end()) continue;
> 
> -        AddSuccessor(B, LI->second);
> -      }
> +      AddSuccessor(B, LI->second);
> +    }
> 
> -    Succ = B;
> -  }
> +  Succ = B;
> 
>   // Create an empty entry block that has no predecessors.
>   cfg->setEntry(createBlock());
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits