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.

llvm-svn: 125744

4 files changed, 81 insertions, 40 deletions

diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 6a3ee12b170..9913edb4347 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -885,11 +885,12 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
     return false;
 
   switch (E->getStmtClass()) {
+  case Stmt::BinaryConditionalOperatorClass:
   case Stmt::ConditionalOperatorClass: {
-    const ConditionalOperator *C = cast<ConditionalOperator>(E);
+    const AbstractConditionalOperator *C = cast<AbstractConditionalOperator>(E);
     return SemaCheckStringLiteral(C->getTrueExpr(), TheCall, HasVAListArg,
                                   format_idx, firstDataArg, isPrintf)
-        && SemaCheckStringLiteral(C->getRHS(), TheCall, HasVAListArg,
+        && SemaCheckStringLiteral(C->getFalseExpr(), TheCall, HasVAListArg,
                                   format_idx, firstDataArg, isPrintf);
   }
 
@@ -910,6 +911,13 @@ bool Sema::SemaCheckStringLiteral(const Expr *E, const CallExpr *TheCall,
     goto tryAgain;
   }
 
+  case Stmt::OpaqueValueExprClass:
+    if (const Expr *src = cast<OpaqueValueExpr>(E)->getSourceExpr()) {
+      E = src;
+      goto tryAgain;
+    }
+    return false;
+
   case Stmt::DeclRefExprClass: {
     const DeclRefExpr *DR = cast<DeclRefExpr>(E);
 
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 052dd4b956f..4bba2402311 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5232,8 +5232,7 @@ ExprResult Sema::ActOnParenOrParenListExpr(SourceLocation L,
 /// In that case, lhs = cond.
 /// C99 6.5.15
 QualType Sema::CheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
-                                        Expr *&SAVE, ExprValueKind &VK,
-                                        ExprObjectKind &OK,
+                                        ExprValueKind &VK, ExprObjectKind &OK,
                                         SourceLocation QuestionLoc) {
   // If both LHS and RHS are overloaded functions, try to resolve them.
   if (Context.hasSameType(LHS->getType(), RHS->getType()) && 
@@ -5252,18 +5251,13 @@ QualType Sema::CheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
 
   // C++ is sufficiently different to merit its own checker.
   if (getLangOptions().CPlusPlus)
-    return CXXCheckConditionalOperands(Cond, LHS, RHS, SAVE,
-                                       VK, OK, QuestionLoc);
+    return CXXCheckConditionalOperands(Cond, LHS, RHS, VK, OK, QuestionLoc);
 
   VK = VK_RValue;
   OK = OK_Ordinary;
 
   UsualUnaryConversions(Cond);
-  if (SAVE) {
-    SAVE = LHS = Cond;
-  }
-  else
-    UsualUnaryConversions(LHS);
+  UsualUnaryConversions(LHS);
   UsualUnaryConversions(RHS);
   QualType CondTy = Cond->getType();
   QualType LHSTy = LHS->getType();
@@ -5610,28 +5604,50 @@ QualType Sema::FindCompositeObjCPointerType(Expr *&LHS, Expr *&RHS,
 /// ActOnConditionalOp - Parse a ?: operation.  Note that 'LHS' may be null
 /// in the case of a the GNU conditional expr extension.
 ExprResult Sema::ActOnConditionalOp(SourceLocation QuestionLoc,
-                                                  SourceLocation ColonLoc,
-                                                  Expr *CondExpr, Expr *LHSExpr,
-                                                  Expr *RHSExpr) {
+                                    SourceLocation ColonLoc,
+                                    Expr *CondExpr, Expr *LHSExpr,
+                                    Expr *RHSExpr) {
   // If this is the gnu "x ?: y" extension, analyze the types as though the LHS
   // was the condition.
-  bool isLHSNull = LHSExpr == 0;
-  Expr *SAVEExpr = 0;
-  if (isLHSNull) {
-    LHSExpr = SAVEExpr = CondExpr;
+  OpaqueValueExpr *opaqueValue = 0;
+  Expr *commonExpr = 0;
+  if (LHSExpr == 0) {
+    commonExpr = CondExpr;
+
+    // We usually want to apply unary conversions *before* saving, except
+    // in the special case of a C++ l-value conditional.
+    if (!(getLangOptions().CPlusPlus
+          && !commonExpr->isTypeDependent()
+          && commonExpr->getValueKind() == RHSExpr->getValueKind()
+          && commonExpr->isGLValue()
+          && commonExpr->isOrdinaryOrBitFieldObject()
+          && RHSExpr->isOrdinaryOrBitFieldObject()
+          && Context.hasSameType(commonExpr->getType(), RHSExpr->getType()))) {
+      UsualUnaryConversions(commonExpr);
+    }
+
+    opaqueValue = new (Context) OpaqueValueExpr(commonExpr->getExprLoc(),
+                                                commonExpr->getType(),
+                                                commonExpr->getValueKind(),
+                                                commonExpr->getObjectKind());
+    LHSExpr = CondExpr = opaqueValue;
   }
 
   ExprValueKind VK = VK_RValue;
   ExprObjectKind OK = OK_Ordinary;
   QualType result = CheckConditionalOperands(CondExpr, LHSExpr, RHSExpr, 
-                                             SAVEExpr, VK, OK, QuestionLoc);
+                                             VK, OK, QuestionLoc);
   if (result.isNull())
     return ExprError();
 
-  return Owned(new (Context) ConditionalOperator(CondExpr, QuestionLoc,
-                                                 LHSExpr, ColonLoc, 
-                                                 RHSExpr, SAVEExpr,
-                                                 result, VK, OK));
+  if (!commonExpr)
+    return Owned(new (Context) ConditionalOperator(CondExpr, QuestionLoc,
+                                                   LHSExpr, ColonLoc, 
+                                                   RHSExpr, result, VK, OK));
+
+  return Owned(new (Context)
+    BinaryConditionalOperator(commonExpr, opaqueValue, CondExpr, LHSExpr,
+                              RHSExpr, QuestionLoc, ColonLoc, result, VK, OK));
 }
 
 // checkPointerTypesForAssignment - This is a very tricky routine (despite
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index e9f595549f2..6fa22a97769 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2927,8 +2927,7 @@ static bool ConvertForConditional(Sema &Self, Expr *&E, QualType T) {
 /// See C++ [expr.cond]. Note that LHS is never null, even for the GNU x ?: y
 /// extension. In this case, LHS == Cond. (But they're not aliases.)
 QualType Sema::CXXCheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
-                                           Expr *&SAVE, ExprValueKind &VK,
-                                           ExprObjectKind &OK,
+                                           ExprValueKind &VK, ExprObjectKind &OK,
                                            SourceLocation QuestionLoc) {
   // FIXME: Handle C99's complex types, vector types, block pointers and Obj-C++
   // interface pointers.
@@ -2936,12 +2935,6 @@ QualType Sema::CXXCheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
   // C++0x 5.16p1
   //   The first expression is contextually converted to bool.
   if (!Cond->isTypeDependent()) {
-    if (SAVE && Cond->getType()->isArrayType()) {
-      QualType CondTy = Cond->getType();
-      CondTy = Context.getArrayDecayedType(CondTy);
-      ImpCastExprToType(Cond, CondTy, CK_ArrayToPointerDecay);
-      SAVE = LHS = Cond;
-    }
     if (CheckCXXBooleanCondition(Cond))
       return QualType();
   }
@@ -3037,12 +3030,10 @@ QualType Sema::CXXCheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
   // l-values.
   bool Same = Context.hasSameType(LTy, RTy);
   if (Same &&
-      LHS->getValueKind() != VK_RValue &&
+      LHS->isGLValue() &&
       LHS->getValueKind() == RHS->getValueKind() &&
-      (LHS->getObjectKind() == OK_Ordinary ||
-       LHS->getObjectKind() == OK_BitField) &&
-      (RHS->getObjectKind() == OK_Ordinary ||
-       RHS->getObjectKind() == OK_BitField)) {
+      LHS->isOrdinaryOrBitFieldObject() &&
+      RHS->isOrdinaryOrBitFieldObject()) {
     VK = LHS->getValueKind();
     if (LHS->getObjectKind() == OK_BitField ||
         RHS->getObjectKind() == OK_BitField)
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index cb0d4250fca..1fc36754dff 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -1415,10 +1415,10 @@ public:
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
   ExprResult RebuildConditionalOperator(Expr *Cond,
-                                              SourceLocation QuestionLoc,
-                                              Expr *LHS,
-                                              SourceLocation ColonLoc,
-                                              Expr *RHS) {
+                                        SourceLocation QuestionLoc,
+                                        Expr *LHS,
+                                        SourceLocation ColonLoc,
+                                        Expr *RHS) {
     return getSema().ActOnConditionalOp(QuestionLoc, ColonLoc, Cond,
                                         LHS, RHS);
   }
@@ -5523,6 +5523,32 @@ TreeTransform<Derived>::TransformCompoundAssignOperator(
 }
 
 template<typename Derived>
+ExprResult TreeTransform<Derived>::
+TransformBinaryConditionalOperator(BinaryConditionalOperator *e) {
+  // Just rebuild the common and RHS expressions and see whether we
+  // get any changes.
+
+  ExprResult commonExpr = getDerived().TransformExpr(e->getCommon());
+  if (commonExpr.isInvalid())
+    return ExprError();
+
+  ExprResult rhs = getDerived().TransformExpr(e->getFalseExpr());
+  if (rhs.isInvalid())
+    return ExprError();
+
+  if (!getDerived().AlwaysRebuild() &&
+      commonExpr.get() == e->getCommon() &&
+      rhs.get() == e->getFalseExpr())
+    return SemaRef.Owned(e);
+
+  return getDerived().RebuildConditionalOperator(commonExpr.take(),
+                                                 e->getQuestionLoc(),
+                                                 0,
+                                                 e->getColonLoc(),
+                                                 rhs.get());
+}
+
+template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformConditionalOperator(ConditionalOperator *E) {
   ExprResult Cond = getDerived().TransformExpr(E->getCond());