[cfe-commits] r125744 - in /cfe/trunk: include/clang/AST/ include/clang/Analysis/Visitors/ include/clang/Basic/ include/clang/Sema/ include/clang/Serialization/ lib/AST/ lib/Analysis/ lib/CodeGen/ lib/Rewrite/ lib/Sema/ lib/Serialization/ lib/StaticAnalyzer/Checkers/ lib/StaticAnalyzer/Core/ test/Analysis/ test/CodeGenCXX/ tools/libclang/

[Benjamin Kramer]

On 17.02.2011, at 11:25, John McCall wrote:

> Author: rjmccall
> Date: Thu Feb 17 04:25:35 2011
> New Revision: 125744
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=125744&view=rev
> Log:
> Change the representation of GNU ?: expressions to use a different expression
> class and to bind the shared value using OpaqueValueExpr.  This fixes an
> unnoticed problem with deserialization of these expressions where the
> deserialized form would lose the vital pointer-equality trait;  or rather,
> it fixes it because this patch also does the right thing for deserializing
> OVEs.
> 
> Change OVEs to not be a "temporary object" in the sense that copy elision is
> permitted.
> 
> This new representation is not totally unawkward to work with, but I think
> that's really part and parcel with the semantics we're modelling here.  In
> particular, it's much easier to fix things like the copy elision bug and to
> make the CFG look right.
> 
> I've tried to update the analyzer to deal with this in at least some          
> obvious cases, and I think we get a much better CFG out, but the printing
> of OpaqueValueExprs probably needs some work.
[…]
> Modified: cfe/trunk/lib/Analysis/CFG.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=125744&r1=125743&r2=125744&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Analysis/CFG.cpp (original)
> +++ cfe/trunk/lib/Analysis/CFG.cpp Thu Feb 17 04:25:35 2011
> @@ -290,7 +290,8 @@
>   CFGBlock *VisitCaseStmt(CaseStmt *C);
>   CFGBlock *VisitChooseExpr(ChooseExpr *C, AddStmtChoice asc);
>   CFGBlock *VisitCompoundStmt(CompoundStmt *C);
> -  CFGBlock *VisitConditionalOperator(ConditionalOperator *C, AddStmtChoice asc);
> +  CFGBlock *VisitConditionalOperator(AbstractConditionalOperator *C,
> +                                     AddStmtChoice asc);
>   CFGBlock *VisitContinueStmt(ContinueStmt *C);
>   CFGBlock *VisitDeclStmt(DeclStmt *DS);
>   CFGBlock *VisitDeclSubExpr(DeclStmt* DS);
> @@ -326,8 +327,9 @@
>   CFGBlock *VisitBinaryOperatorForTemporaryDtors(BinaryOperator *E);
>   CFGBlock *VisitCXXBindTemporaryExprForTemporaryDtors(CXXBindTemporaryExpr *E,
>       bool BindToTemporary);
> -  CFGBlock *VisitConditionalOperatorForTemporaryDtors(ConditionalOperator *E,
> -      bool BindToTemporary);
> +  CFGBlock *
> +  VisitConditionalOperatorForTemporaryDtors(AbstractConditionalOperator *E,
> +                                            bool BindToTemporary);
> 
>   // NYS == Not Yet Supported
>   CFGBlock* NYS() {
> @@ -785,6 +787,9 @@
>     case Stmt::AddrLabelExprClass:
>       return VisitAddrLabelExpr(cast<AddrLabelExpr>(S), asc);
> 
> +    case Stmt::BinaryConditionalOperatorClass:
> +      return VisitConditionalOperator(cast<BinaryConditionalOperator>(S), asc);
> +
>     case Stmt::BinaryOperatorClass:
>       return VisitBinaryOperator(cast<BinaryOperator>(S), asc);
> 
> @@ -1174,8 +1179,11 @@
>   return LastBlock;
> }
> 
> -CFGBlock *CFGBuilder::VisitConditionalOperator(ConditionalOperator *C,
> +CFGBlock *CFGBuilder::VisitConditionalOperator(AbstractConditionalOperator *C,
>                                                AddStmtChoice asc) {
> +  const BinaryConditionalOperator *BCO = dyn_cast<BinaryConditionalOperator>(C);
> +  const OpaqueValueExpr *opaqueValue = (BCO ? BCO->getOpaqueValue() : NULL);
> +
>   // Create the confluence block that will "merge" the results of the ternary
>   // expression.
>   CFGBlock* ConfluenceBlock = Block ? Block : createBlock();
> @@ -1191,9 +1199,10 @@
>   //  e.g: x ?: y is shorthand for: x ? x : y;
>   Succ = ConfluenceBlock;
>   Block = NULL;
> -  CFGBlock* LHSBlock = NULL;
> -  if (C->getLHS()) {
> -    LHSBlock = Visit(C->getLHS(), alwaysAdd);
> +  CFGBlock* LHSBlock = 0;
> +  const Expr *trueExpr = C->getTrueExpr();
> +  if (trueExpr != opaqueValue) {
> +    LHSBlock = Visit(C->getTrueExpr(), alwaysAdd);
>     if (badCFG)
>       return 0;
>     Block = NULL;
> @@ -1201,7 +1210,7 @@
> 
>   // Create the block for the RHS expression.
>   Succ = ConfluenceBlock;
> -  CFGBlock* RHSBlock = Visit(C->getRHS(), alwaysAdd);
> +  CFGBlock* RHSBlock = Visit(C->getFalseExpr(), alwaysAdd);
>   if (badCFG)
>     return 0;
> 
> @@ -1210,33 +1219,15 @@
> 
>   // See if this is a known constant.
>   const TryResult& KnownVal = tryEvaluateBool(C->getCond());
> -  if (LHSBlock) {
> +  if (LHSBlock)
>     addSuccessor(Block, KnownVal.isFalse() ? NULL : LHSBlock);
> -  } else {
> -    if (KnownVal.isFalse()) {
> -      // If we know the condition is false, add NULL as the successor for
> -      // the block containing the condition.  In this case, the confluence
> -      // block will have just one predecessor.
> -      addSuccessor(Block, 0);
> -      assert(ConfluenceBlock->pred_size() == 1);
> -    } else {
> -      // If we have no LHS expression, add the ConfluenceBlock as a direct
> -      // successor for the block containing the condition.  Moreover, we need to
> -      // reverse the order of the predecessors in the ConfluenceBlock because
> -      // the RHSBlock will have been added to the succcessors already, and we
> -      // want the first predecessor to the the block containing the expression
> -      // for the case when the ternary expression evaluates to true.
> -      addSuccessor(Block, ConfluenceBlock);
> -      // Note that there can possibly only be one predecessor if one of the
> -      // subexpressions resulted in calling a noreturn function.
> -      std::reverse(ConfluenceBlock->pred_begin(),
> -                   ConfluenceBlock->pred_end());
> -    }
> -  }
> -
>   addSuccessor(Block, KnownVal.isTrue() ? NULL : RHSBlock);
>   Block->setTerminator(C);
> -  return addStmt(C->getCond());
> +  CFGBlock *result;
> +  Expr *condExpr = C->getCond();
> +  if (condExpr != opaqueValue) result = addStmt(condExpr);
> +  if (BCO) result = addStmt(BCO->getCommon());
> +  return result;
> }

Hi John,

gcc says: CFG.cpp:1226: warning: 'result' may be used uninitialized in this function