Although we currently have explicit lvalue-to-rvalue conversions, they're

not actually frequently used, because ImpCastExprToType only creates a node
if the types differ.  So explicitly create an ICE in the lvalue-to-rvalue
conversion code in DefaultFunctionArrayLvalueConversion() as well as several
other new places, and consistently deal with the consequences throughout the
compiler.

In addition, introduce a new cast kind for loading an ObjCProperty l-value,
and make sure we emit those nodes whenever an ObjCProperty l-value appears
that's not on the LHS of an assignment operator.

This breaks a couple of rewriter tests, which I've x-failed until future
development occurs on the rewriter.

Ted Kremenek kindly contributed the analyzer workarounds in this patch.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@120890 91177308-0d34-0410-b5e6-96231b3b80d8

20 files changed, 346 insertions, 105 deletions

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index 265788a081..fbc5a67af1 100644
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -824,6 +824,8 @@ const char *CastExpr::getCastKindName() const {
     return "LValueBitCast";
   case CK_LValueToRValue:
     return "LValueToRValue";
+  case CK_GetObjCProperty:
+    return "GetObjCProperty";
   case CK_NoOp:
     return "NoOp";
   case CK_BaseToDerived:
@@ -1722,6 +1724,24 @@ Expr *Expr::IgnoreParenCasts() {
   }
 }
 
+Expr *Expr::IgnoreParenLValueCasts() {
+  Expr *E = this;
+  while (E) {
+    if (ParenExpr *P = dyn_cast<ParenExpr>(E)) {
+      E = P->getSubExpr();
+      continue;
+    }
+    if (CastExpr *P = dyn_cast<CastExpr>(E)) {
+      if (P->getCastKind() == CK_LValueToRValue) {
+        E = P->getSubExpr();
+        continue;
+      }
+    }
+    break;
+  }
+  return E;
+}
+  
 Expr *Expr::IgnoreParenImpCasts() {
   Expr *E = this;
   while (true) {
@@ -2043,12 +2063,34 @@ bool Expr::isNullPointerConstant(ASTContext &Ctx,
   return isIntegerConstantExpr(Result, Ctx) && Result == 0;
 }
 
+/// \brief If this expression is an l-value for an Objective C
+/// property, find the underlying property reference expression.
+const ObjCPropertyRefExpr *Expr::getObjCProperty() const {
+  const Expr *E = this;
+  while (true) {
+    assert((E->getValueKind() == VK_LValue &&
+            E->getObjectKind() == OK_ObjCProperty) &&
+           "expression is not a property reference");
+    E = E->IgnoreParenCasts();
+    if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
+      if (BO->getOpcode() == BO_Comma) {
+        E = BO->getRHS();
+        continue;
+      }
+    }
+
+    break;
+  }
+
+  return cast<ObjCPropertyRefExpr>(E);
+}
+
 FieldDecl *Expr::getBitField() {
   Expr *E = this->IgnoreParens();
 
   while (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {
-    if (ICE->getValueKind() != VK_RValue &&
-        ICE->getCastKind() == CK_NoOp)
+    if (ICE->getCastKind() == CK_LValueToRValue ||
+        (ICE->getValueKind() != VK_RValue && ICE->getCastKind() == CK_NoOp))
       E = ICE->getSubExpr()->IgnoreParens();
     else
       break;
diff --git a/lib/AST/ExprClassification.cpp b/lib/AST/ExprClassification.cpp
index e55545e693..1afc7602fb 100644
--- a/lib/AST/ExprClassification.cpp
+++ b/lib/AST/ExprClassification.cpp
@@ -413,8 +413,10 @@ static Cl::Kinds ClassifyBinaryOp(ASTContext &Ctx, const BinaryOperator *E) {
   assert(Ctx.getLangOptions().CPlusPlus &&
          "This is only relevant for C++.");
   // C++ [expr.ass]p1: All [...] return an lvalue referring to the left operand.
+  // Except we override this for writes to ObjC properties.
   if (E->isAssignmentOp())
-    return Cl::CL_LValue;
+    return (E->getLHS()->getObjectKind() == OK_ObjCProperty
+              ? Cl::CL_PRValue : Cl::CL_LValue);
 
   // C++ [expr.comma]p1: the result is of the same value category as its right
   //   operand, [...].
@@ -468,9 +470,10 @@ static Cl::ModifiableType IsModifiable(ASTContext &Ctx, const Expr *E,
   if (Kind == Cl::CL_PRValue) {
     // For the sake of better diagnostics, we want to specifically recognize
     // use of the GCC cast-as-lvalue extension.
-    if (const CStyleCastExpr *CE = dyn_cast<CStyleCastExpr>(E->IgnoreParens())){
-      if (CE->getSubExpr()->Classify(Ctx).isLValue()) {
-        Loc = CE->getLParenLoc();
+    if (const ExplicitCastExpr *CE =
+          dyn_cast<ExplicitCastExpr>(E->IgnoreParens())) {
+      if (CE->getSubExpr()->IgnoreParenImpCasts()->isLValue()) {
+        Loc = CE->getExprLoc();
         return Cl::CM_LValueCast;
       }
     }
diff --git a/lib/Analysis/CFG.cpp b/lib/Analysis/CFG.cpp
index 57e67e4268..b382d9be58 100644
--- a/lib/Analysis/CFG.cpp
+++ b/lib/Analysis/CFG.cpp
@@ -825,6 +825,16 @@ tryAgain:
 
     case Stmt::ContinueStmtClass:
       return VisitContinueStmt(cast<ContinueStmt>(S));
+    
+    case Stmt::CStyleCastExprClass: {
+      CastExpr *castExpr = cast<CastExpr>(S);
+      if (castExpr->getCastKind() == CK_LValueToRValue) {
+        // temporary workaround
+        S = castExpr->getSubExpr();
+        goto tryAgain;
+      }
+      return VisitStmt(S, asc);
+    }
 
     case Stmt::CXXCatchStmtClass:
       return VisitCXXCatchStmt(cast<CXXCatchStmt>(S));
@@ -871,8 +881,15 @@ tryAgain:
     case Stmt::IfStmtClass:
       return VisitIfStmt(cast<IfStmt>(S));
 
-    case Stmt::ImplicitCastExprClass:
-      return VisitImplicitCastExpr(cast<ImplicitCastExpr>(S), asc);
+    case Stmt::ImplicitCastExprClass: {
+      ImplicitCastExpr *castExpr = cast<ImplicitCastExpr>(S);
+      if (castExpr->getCastKind() == CK_LValueToRValue) {
+        // temporary workaround
+        S = castExpr->getSubExpr();
+        goto tryAgain;
+      }
+      return VisitImplicitCastExpr(castExpr, asc);
+    }
 
     case Stmt::IndirectGotoStmtClass:
       return VisitIndirectGotoStmt(cast<IndirectGotoStmt>(S));
diff --git a/lib/Checker/CheckSecuritySyntaxOnly.cpp b/lib/Checker/CheckSecuritySyntaxOnly.cpp
index 9a2ac45fa2..0223240ce6 100644
--- a/lib/Checker/CheckSecuritySyntaxOnly.cpp
+++ b/lib/Checker/CheckSecuritySyntaxOnly.cpp
@@ -187,8 +187,10 @@ void WalkAST::CheckLoopConditionForFloat(const ForStmt *FS) {
     return;
 
   // Are we comparing variables?
-  const DeclRefExpr *drLHS = dyn_cast<DeclRefExpr>(B->getLHS()->IgnoreParens());
-  const DeclRefExpr *drRHS = dyn_cast<DeclRefExpr>(B->getRHS()->IgnoreParens());
+  const DeclRefExpr *drLHS =
+    dyn_cast<DeclRefExpr>(B->getLHS()->IgnoreParenLValueCasts());
+  const DeclRefExpr *drRHS =
+    dyn_cast<DeclRefExpr>(B->getRHS()->IgnoreParenLValueCasts());
 
   // Does at least one of the variables have a floating point type?
   drLHS = drLHS && drLHS->getType()->isRealFloatingType() ? drLHS : NULL;
diff --git a/lib/Checker/DereferenceChecker.cpp b/lib/Checker/DereferenceChecker.cpp
index 747fcbe311..af929a7af7 100644
--- a/lib/Checker/DereferenceChecker.cpp
+++ b/lib/Checker/DereferenceChecker.cpp
@@ -59,6 +59,7 @@ void DereferenceChecker::AddDerefSource(llvm::raw_ostream &os,
                                      llvm::SmallVectorImpl<SourceRange> &Ranges,
                                         const Expr *Ex,
                                         bool loadedFrom) {
+  Ex = Ex->IgnoreParenLValueCasts();
   switch (Ex->getStmtClass()) {
     default:
       return;
diff --git a/lib/Checker/Environment.cpp b/lib/Checker/Environment.cpp
index aac4afaafb..4b67bda913 100644
--- a/lib/Checker/Environment.cpp
+++ b/lib/Checker/Environment.cpp
@@ -53,7 +53,8 @@ SVal Environment::getSVal(const Stmt *E, SValBuilder& svalBuilder) const {
         QualType CT = C->getType();
         if (CT->isVoidType())
           return UnknownVal();
-        if (C->getCastKind() == CK_NoOp) {
+        if (C->getCastKind() == CK_NoOp ||
+            C->getCastKind() == CK_LValueToRValue) { // temporary workaround
           E = C->getSubExpr();
           continue;
         }
diff --git a/lib/Checker/GRExprEngine.cpp b/lib/Checker/GRExprEngine.cpp
index a43d36aaeb..e737eed8d1 100644
--- a/lib/Checker/GRExprEngine.cpp
+++ b/lib/Checker/GRExprEngine.cpp
@@ -778,6 +778,10 @@ void GRExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
                                 S->getLocStart(),
                                 "Error evaluating statement");
 
+  // Expressions to ignore.
+  if (const Expr *Ex = dyn_cast<Expr>(S))
+    S = Ex->IgnoreParenLValueCasts();
+  
   // FIXME: add metadata to the CFG so that we can disable
   //  this check when we KNOW that there is no block-level subexpression.
   //  The motivation is that this check requires a hashtable lookup.
@@ -814,7 +818,9 @@ void GRExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
       MakeNode(Dst, S, Pred, GetState(Pred));
       break;
     }
-
+      
+    case Stmt::ParenExprClass:
+      llvm_unreachable("ParenExprs already handled.");
     // Cases that should never be evaluated simply because they shouldn't
     // appear in the CFG.
     case Stmt::BreakStmtClass:
@@ -1039,10 +1045,6 @@ void GRExprEngine::Visit(const Stmt* S, ExplodedNode* Pred,
       break;
     }
 
-    case Stmt::ParenExprClass:
-      Visit(cast<ParenExpr>(S)->getSubExpr()->IgnoreParens(), Pred, Dst);
-      break;
-
     case Stmt::ReturnStmtClass:
       VisitReturnStmt(cast<ReturnStmt>(S), Pred, Dst);
       break;
@@ -1113,8 +1115,8 @@ void GRExprEngine::VisitLValue(const Expr* Ex, ExplodedNode* Pred,
                                 Ex->getLocStart(),
                                 "Error evaluating statement");
 
-
-  Ex = Ex->IgnoreParens();
+  // Expressions to ignore.
+  Ex = Ex->IgnoreParenLValueCasts();
 
   if (Ex != CurrentStmt && Pred->getLocationContext()->getCFG()->isBlkExpr(Ex)){
     Dst.Add(Pred);
@@ -2661,6 +2663,7 @@ void GRExprEngine::VisitCast(const CastExpr *CastE, const Expr *Ex,
     }
     return;
 
+  case CK_GetObjCProperty:
   case CK_Dependent:
   case CK_ArrayToPointerDecay:
   case CK_BitCast:
diff --git a/lib/Checker/IdempotentOperationChecker.cpp b/lib/Checker/IdempotentOperationChecker.cpp
index 0e23c2a031..245caef485 100644
--- a/lib/Checker/IdempotentOperationChecker.cpp
+++ b/lib/Checker/IdempotentOperationChecker.cpp
@@ -543,7 +543,7 @@ bool IdempotentOperationChecker::isTruncationExtensionAssignment(
   if (VD != RHS_DR->getDecl())
      return false;
 
-  return dyn_cast<DeclRefExpr>(RHS->IgnoreParens()) == NULL;
+  return dyn_cast<DeclRefExpr>(RHS->IgnoreParenLValueCasts()) == NULL;
 }
 
 // Returns false if a path to this block was not completely analyzed, or true
diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index c98ee17842..78605ba8b1 100644
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -1755,6 +1755,21 @@ LValue CodeGenFunction::EmitCastLValue(const CastExpr *E) {
   case CK_Dependent:
     llvm_unreachable("dependent cast kind in IR gen!");
 
+  case CK_GetObjCProperty: {
+    LValue LV = EmitLValue(E->getSubExpr());
+    assert(LV.isPropertyRef());
+    RValue RV = EmitLoadOfPropertyRefLValue(LV);
+
+    // Property is an aggregate r-value.
+    if (RV.isAggregate()) {
+      return MakeAddrLValue(RV.getAggregateAddr(), E->getType());
+    }
+
+    // Implicit property returns an l-value.
+    assert(RV.isScalar());
+    return MakeAddrLValue(RV.getScalarVal(), E->getSubExpr()->getType());
+  }
+
   case CK_NoOp:
     if (!E->getSubExpr()->isRValue() || E->getType()->isRecordType()) {
       LValue LV = EmitLValue(E->getSubExpr());
diff --git a/lib/CodeGen/CGExprAgg.cpp b/lib/CodeGen/CGExprAgg.cpp
index 9e4e7dcc53..b2679a3106 100644
--- a/lib/CodeGen/CGExprAgg.cpp
+++ b/lib/CodeGen/CGExprAgg.cpp
@@ -282,8 +282,16 @@ void AggExprEmitter::VisitCastExpr(CastExpr *E) {
     break;
   }
 
+  case CK_GetObjCProperty: {
+    LValue LV = CGF.EmitLValue(E->getSubExpr());
+    assert(LV.isPropertyRef());
+    RValue RV = CGF.EmitLoadOfPropertyRefLValue(LV, getReturnValueSlot());
+    EmitGCMove(E, RV);
+    break;
+  }
+
+  case CK_LValueToRValue: // hope for downstream optimization
   case CK_NoOp:
-  case CK_LValueToRValue:
   case CK_UserDefinedConversion:
   case CK_ConstructorConversion:
     assert(CGF.getContext().hasSameUnqualifiedType(E->getSubExpr()->getType(),
@@ -349,9 +357,8 @@ void AggExprEmitter::VisitObjCMessageExpr(ObjCMessageExpr *E) {
 }
 
 void AggExprEmitter::VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *E) {
-  RValue RV = CGF.EmitLoadOfPropertyRefLValue(CGF.EmitObjCPropertyRefLValue(E),
-                                              getReturnValueSlot());
-  EmitGCMove(E, RV);
+  llvm_unreachable("direct property access not surrounded by "
+                   "lvalue-to-rvalue cast");
 }
 
 void AggExprEmitter::VisitBinComma(const BinaryOperator *E) {
diff --git a/lib/CodeGen/CGExprComplex.cpp b/lib/CodeGen/CGExprComplex.cpp
index e1d042ba9a..fbb3d4860a 100644
--- a/lib/CodeGen/CGExprComplex.cpp
+++ b/lib/CodeGen/CGExprComplex.cpp
@@ -108,6 +108,7 @@ public:
     return EmitLoadOfLValue(E);
   }
   ComplexPairTy VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *E) {
+    assert(E->getObjectKind() == OK_Ordinary);
     return EmitLoadOfLValue(E);
   }
   ComplexPairTy VisitObjCMessageExpr(ObjCMessageExpr *E) {
@@ -333,7 +334,21 @@ ComplexPairTy ComplexExprEmitter::EmitComplexToComplexCast(ComplexPairTy Val,
 
 ComplexPairTy ComplexExprEmitter::EmitCast(CastExpr::CastKind CK, Expr *Op, 
                                            QualType DestTy) {
-  // FIXME: this should be based off of the CastKind.
+  switch (CK) {
+  case CK_GetObjCProperty: {
+    LValue LV = CGF.EmitLValue(Op);
+    assert(LV.isPropertyRef() && "Unknown LValue type!");
+    return CGF.EmitLoadOfPropertyRefLValue(LV).getComplexVal();
+  }
+
+  case CK_NoOp:
+  case CK_LValueToRValue:
+    return Visit(Op);
+
+  // TODO: do all of these
+  default:
+    break;
+  }
 
   // Two cases here: cast from (complex to complex) and (scalar to complex).
   if (Op->getType()->isAnyComplexType())
diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp
index 86d7216bbe..8ed8cb38db 100644
--- a/lib/CodeGen/CGExprScalar.cpp
+++ b/lib/CodeGen/CGExprScalar.cpp
@@ -237,6 +237,8 @@ public:
     return EmitLoadOfLValue(E);
   }
   Value *VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *E) {
+    assert(E->getObjectKind() == OK_Ordinary &&
+           "reached property reference without lvalue-to-rvalue");
     return EmitLoadOfLValue(E);
   }
   Value *VisitObjCMessageExpr(ObjCMessageExpr *E) {
@@ -1097,9 +1099,18 @@ Value *ScalarExprEmitter::EmitCastExpr(CastExpr *CE) {
   case CK_ToUnion:
     llvm_unreachable("scalar cast to non-scalar value");
     break;
+
+  case CK_GetObjCProperty: {
+    assert(CGF.getContext().hasSameUnqualifiedType(E->getType(), DestTy));
+    assert(E->isLValue() && E->getObjectKind() == OK_ObjCProperty &&
+           "CK_GetObjCProperty for non-lvalue or non-ObjCProperty");
+    RValue RV = CGF.EmitLoadOfLValue(CGF.EmitLValue(E), E->getType());
+    return RV.getScalarVal();
+  }
       
   case CK_LValueToRValue:
     assert(CGF.getContext().hasSameUnqualifiedType(E->getType(), DestTy));
+    assert(E->isGLValue() && "lvalue-to-rvalue applied to r-value!");
     return Visit(const_cast<Expr*>(E));
 
   case CK_IntegralToPointer: {
@@ -1124,11 +1135,8 @@ Value *ScalarExprEmitter::EmitCastExpr(CastExpr *CE) {
     return Builder.CreatePtrToInt(Src, ConvertType(DestTy));
   }
   case CK_ToVoid: {
-    if (E->Classify(CGF.getContext()).isGLValue()) {
-      LValue LV = CGF.EmitLValue(E);
-      if (LV.isPropertyRef())
-        CGF.EmitLoadOfPropertyRefLValue(LV);
-    }
+    if (!E->isRValue())
+      CGF.EmitLValue(E);
     else
       CGF.EmitAnyExpr(E, AggValueSlot::ignored(), true);
     return 0;
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index 4d737b7370..29779070be 100644
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -1032,13 +1032,19 @@ StmtResult Parser::ParseForStatement(AttributeList *Attr) {
   } else {
     Value = ParseExpression();
 
+    ForEach = isTokIdentifier_in();
+
     // Turn the expression into a stmt.
-    if (!Value.isInvalid())
-      FirstPart = Actions.ActOnExprStmt(Actions.MakeFullExpr(Value.get()));
+    if (!Value.isInvalid()) {
+      if (ForEach)
+        FirstPart = Actions.ActOnForEachLValueExpr(Value.get());
+      else
+        FirstPart = Actions.ActOnExprStmt(Actions.MakeFullExpr(Value.get()));
+    }
 
     if (Tok.is(tok::semi)) {
       ConsumeToken();
-    } else if ((ForEach = isTokIdentifier_in())) {
+    } else if (ForEach) {
       ConsumeToken(); // consume 'in'
       
       if (Tok.is(tok::code_completion)) {
diff --git a/lib/Sema/SemaCXXCast.cpp b/lib/Sema/SemaCXXCast.cpp
index 214d1f6175..ac679f7009 100644
--- a/lib/Sema/SemaCXXCast.cpp
+++ b/lib/Sema/SemaCXXCast.cpp
@@ -516,6 +516,7 @@ CheckStaticCast(Sema &Self, Expr *&SrcExpr, QualType DestType,
   // a non-lvalue-reference target type does not lead to decay.
   // C++ 5.2.9p4: Any expression can be explicitly converted to type "cv void".
   if (DestType->isVoidType()) {
+    Self.IgnoredValueConversions(SrcExpr);
     Kind = CK_ToVoid;
     return;
   }
@@ -1371,6 +1372,7 @@ Sema::CXXCheckCStyleCast(SourceRange R, QualType CastTy, ExprValueKind &VK,
   // a non-lvalue-reference target type does not lead to decay.
   // C++ 5.2.9p4: Any expression can be explicitly converted to type "cv void".
   if (CastTy->isVoidType()) {
+    IgnoredValueConversions(CastExpr);    
     Kind = CK_ToVoid;
     return false;
   }
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 46c5967e5b..d6aa4ce885 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -2104,8 +2104,8 @@ do {
 void Sema::CheckFloatComparison(SourceLocation loc, Expr* lex, Expr *rex) {
   bool EmitWarning = true;
 
-  Expr* LeftExprSansParen = lex->IgnoreParens();
-  Expr* RightExprSansParen = rex->IgnoreParens();
+  Expr* LeftExprSansParen = lex->IgnoreParenImpCasts();
+  Expr* RightExprSansParen = rex->IgnoreParenImpCasts();
 
   // Special case: check for x == x (which is OK).
   // Do not emit warnings for such cases.
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 177d61cc82..8b29667bae 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -251,6 +251,24 @@ void Sema::DefaultFunctionArrayLvalueConversion(Expr *&E) {
   //   A glvalue of a non-function, non-array type T can be
   //   converted to a prvalue.
   if (E->isGLValue()) {
+    QualType T = E->getType();
+    assert(!T.isNull() && "r-value conversion on typeless expression?");
+
+    // Create a load out of an ObjCProperty l-value, if necessary.
+    if (E->getObjectKind() == OK_ObjCProperty) {
+      ConvertPropertyForRValue(E);
+      if (!E->isGLValue())
+        return;
+    }
+
+    // We don't want to throw lvalue-to-rvalue casts on top of
+    // expressions of certain types in C++.
+    if (getLangOptions().CPlusPlus &&
+        (E->getType() == Context.OverloadTy ||
+         T->isDependentType() ||
+         T->isRecordType()))
+      return;
+
     // C++ [conv.lval]p1:
     //   [...] If T is a non-class type, the type of the prvalue is the
     //   cv-unqualified version of T. Otherwise, the type of the
@@ -259,15 +277,12 @@ void Sema::DefaultFunctionArrayLvalueConversion(Expr *&E) {
     // C99 6.3.2.1p2:
     //   If the lvalue has qualified type, the value has the unqualified
     //   version of the type of the lvalue; otherwise, the value has the
-    //   type of the lvalue.
-    QualType T = E->getType();
-    assert(!T.isNull() && "r-value conversion on typeless expression?");
-    
-    if (T.hasQualifiers() && !T->isDependentType() &&
-        (!getLangOptions().CPlusPlus || !T->isRecordType()))
+    //   type of the lvalue.    
+    if (T.hasQualifiers())
       T = T.getUnqualifiedType();
-    
-    ImpCastExprToType(E, T, CK_LValueToRValue);
+
+    E = ImplicitCastExpr::Create(Context, T, CK_LValueToRValue,
+                                 E, 0, VK_RValue);
   }
 }
 
@@ -2649,6 +2664,10 @@ static QualType CheckRealImagOperand(Sema &S, Expr *&V, SourceLocation Loc,
   if (V->isTypeDependent())
     return S.Context.DependentTy;
 
+  // _Real and _Imag are only l-values for normal l-values.
+  if (V->getObjectKind() != OK_Ordinary)
+    S.DefaultFunctionArrayLvalueConversion(V);
+
   // These operators return the element type of a complex type.
   if (const ComplexType *CT = V->getType()->getAs<ComplexType>())
     return CT->getElementType();
@@ -3351,6 +3370,11 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
     return ExprError();
   }
 
+  // Perform a property load on the base regardless of whether we
+  // actually need it for the declaration.
+  if (BaseExpr->getObjectKind() == OK_ObjCProperty)
+    ConvertPropertyForRValue(BaseExpr);
+
   if (FieldDecl *FD = dyn_cast<FieldDecl>(MemberDecl))
     return BuildFieldReferenceExpr(*this, BaseExpr, IsArrow,
                                    SS, FD, FoundDecl, MemberNameInfo);
@@ -3359,7 +3383,7 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
     // We may have found a field within an anonymous union or struct
     // (C++ [class.union]).
     return BuildAnonymousStructUnionMemberReference(MemberLoc, FD,
-                                                     BaseExpr, OpLoc);
+                                                    BaseExpr, OpLoc);
 
   if (VarDecl *Var = dyn_cast<VarDecl>(MemberDecl)) {
     MarkDeclarationReferenced(MemberLoc, Var);
@@ -3694,6 +3718,10 @@ Sema::LookupMemberExpr(LookupResult &R, Expr *&BaseExpr,
   // Handle properties on 'id' and qualified "id".
   if (!IsArrow && (BaseType->isObjCIdType() ||
                    BaseType->isObjCQualifiedIdType())) {
+    // This actually uses the base as an r-value.
+    DefaultFunctionArrayLvalueConversion(BaseExpr);
+    assert(Context.hasSameUnqualifiedType(BaseType, BaseExpr->getType()));
+
     const ObjCObjectPointerType *QIdTy = BaseType->getAs<ObjCObjectPointerType>();
     IdentifierInfo *Member = MemberName.getAsIdentifierInfo();
 
@@ -3743,9 +3771,12 @@ Sema::LookupMemberExpr(LookupResult &R, Expr *&BaseExpr,
   // pointer to a (potentially qualified) interface type.
   if (!IsArrow)
     if (const ObjCObjectPointerType *OPT =
-          BaseType->getAsObjCInterfacePointerType())
+          BaseType->getAsObjCInterfacePointerType()) {
+      // This actually uses the base as an r-value.
+      DefaultFunctionArrayLvalueConversion(BaseExpr);      
       return HandleExprPropertyRefExpr(OPT, BaseExpr, MemberName, MemberLoc,
                                        SourceLocation(), QualType(), false);
+    }
 
   // Handle the following exceptional case (*Obj).isa.
   if (!IsArrow &&
@@ -4024,8 +4055,8 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc,
         Param? InitializedEntity::InitializeParameter(Context, Param)
              : InitializedEntity::InitializeParameter(Context, ProtoArgType);
       ExprResult ArgE = PerformCopyInitialization(Entity,
-                                                        SourceLocation(),
-                                                        Owned(Arg));
+                                                  SourceLocation(),
+                                                  Owned(Arg));
       if (ArgE.isInvalid())
         return true;
 
@@ -4532,16 +4563,19 @@ bool Sema::CheckCastTypes(SourceRange TyR, QualType castType,
   // We only support r-value casts in C.
   VK = VK_RValue;
 
-  DefaultFunctionArrayLvalueConversion(castExpr);
-
   // C99 6.5.4p2: the cast type needs to be void or scalar and the expression
   // type needs to be scalar.
   if (castType->isVoidType()) {
+    // We don't necessarily do lvalue-to-rvalue conversions on this.
+    IgnoredValueConversions(castExpr);
+
     // Cast to void allows any expr type.
     Kind = CK_ToVoid;
     return false;
   }
 
+  DefaultFunctionArrayLvalueConversion(castExpr);
+
   if (RequireCompleteType(TyR.getBegin(), castType,
                           diag::err_typecheck_cast_to_incomplete))
     return true;
@@ -5713,7 +5747,7 @@ Sema::CheckSingleAssignmentConstraints(QualType lhsType, Expr *&rExpr) {
 
     // FIXME: Currently, we fall through and treat C++ classes like C
     // structures.
-  }
+  }  
 
   // C99 6.5.16.1p1: the left operand is a pointer and the right is
   // a null pointer constant.
@@ -6201,8 +6235,8 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
     // obvious cases in the definition of the template anyways. The idea is to
     // warn when the typed comparison operator will always evaluate to the same
     // result.
-    Expr *LHSStripped = lex->IgnoreParens();
-    Expr *RHSStripped = rex->IgnoreParens();
+    Expr *LHSStripped = lex->IgnoreParenImpCasts();
+    Expr *RHSStripped = rex->IgnoreParenImpCasts();
     if (DeclRefExpr* DRL = dyn_cast<DeclRefExpr>(LHSStripped)) {
       if (DeclRefExpr* DRR = dyn_cast<DeclRefExpr>(RHSStripped)) {
         if (DRL->getDecl() == DRR->getDecl() &&
@@ -6782,7 +6816,8 @@ QualType Sema::CheckAssignmentOperands(Expr *LHS, Expr *&RHS,
   if (CompoundType.isNull()) {
     QualType LHSTy(LHSType);
     // Simple assignment "x = y".
-    ConvertPropertyAssignment(LHS, RHS, LHSTy);
+    if (LHS->getObjectKind() == OK_ObjCProperty)
+      ConvertPropertyForLValue(LHS, RHS, LHSTy);
     ConvTy = CheckSingleAssignmentConstraints(LHSTy, RHS);
     // Special case of NSObject attributes on c-style pointer types.
     if (ConvTy == IncompatiblePointer &&
@@ -6857,7 +6892,7 @@ QualType Sema::CheckAssignmentOperands(Expr *LHS, Expr *&RHS,
 }
 
 // C99 6.5.17
-static QualType CheckCommaOperands(Sema &S, Expr *LHS, Expr *&RHS,
+static QualType CheckCommaOperands(Sema &S, Expr *&LHS, Expr *&RHS,
                                    SourceLocation Loc) {
   S.DiagnoseUnusedExprResult(LHS);
 
@@ -6873,11 +6908,12 @@ static QualType CheckCommaOperands(Sema &S, Expr *LHS, Expr *&RHS,
   // C's comma performs lvalue conversion (C99 6.3.2.1) on both its
   // operands, but not unary promotions.
   // C++'s comma does not do any conversions at all (C++ [expr.comma]p1).
-  if (!S.getLangOptions().CPlusPlus) {
-    S.DefaultFunctionArrayLvalueConversion(LHS);
-    if (!LHS->getType()->isVoidType())
-      S.RequireCompleteType(Loc, LHS->getType(), diag::err_incomplete_type);
 
+  // So we treat the LHS as a ignored value, and in C++ we allow the
+  // containing site to determine what should be done with the RHS.
+  S.IgnoredValueConversions(LHS);
+
+  if (!S.getLangOptions().CPlusPlus) {
     S.DefaultFunctionArrayLvalueConversion(RHS);
     if (!RHS->getType()->isVoidType())
       S.RequireCompleteType(Loc, RHS->getType(), diag::err_incomplete_type);
@@ -6971,21 +7007,47 @@ static QualType CheckIncrementDecrementOperand(Sema &S, Expr *Op,
   }
 }
 
-void Sema::ConvertPropertyAssignment(Expr *LHS, Expr *&RHS, QualType& LHSTy) {
-  bool copyInit = false;
-  if (const ObjCPropertyRefExpr *PRE = dyn_cast<ObjCPropertyRefExpr>(LHS)) {
-    if (PRE->isImplicitProperty()) {
-      // If using property-dot syntax notation for assignment, and there is a
-      // setter, RHS expression is being passed to the setter argument. So,
-      // type conversion (and comparison) is RHS to setter's argument type.
-      if (const ObjCMethodDecl *SetterMD = PRE->getImplicitPropertySetter()) {
-        ObjCMethodDecl::param_iterator P = SetterMD->param_begin();
-        LHSTy = (*P)->getType();
+void Sema::ConvertPropertyForRValue(Expr *&E) {
+  assert(E->getValueKind() == VK_LValue &&
+         E->getObjectKind() == OK_ObjCProperty);
+  const ObjCPropertyRefExpr *PRE = E->getObjCProperty();
+
+  ExprValueKind VK = VK_RValue;
+  if (PRE->isImplicitProperty()) {
+    QualType Result = PRE->getImplicitPropertyGetter()->getResultType();
+    VK = Expr::getValueKindForType(Result);
+  }
+
+  E = ImplicitCastExpr::Create(Context, E->getType(), CK_GetObjCProperty,
+                               E, 0, VK);
+}
+
+void Sema::ConvertPropertyForLValue(Expr *&LHS, Expr *&RHS, QualType &LHSTy) {
+  assert(LHS->getValueKind() == VK_LValue &&
+         LHS->getObjectKind() == OK_ObjCProperty);
+  const ObjCPropertyRefExpr *PRE = LHS->getObjCProperty();
+
+  if (PRE->isImplicitProperty()) {
+    // If using property-dot syntax notation for assignment, and there is a
+    // setter, RHS expression is being passed to the setter argument. So,
+    // type conversion (and comparison) is RHS to setter's argument type.
+    if (const ObjCMethodDecl *SetterMD = PRE->getImplicitPropertySetter()) {
+      ObjCMethodDecl::param_iterator P = SetterMD->param_begin();
+      LHSTy = (*P)->getType();
+
+    // Otherwise, if the getter returns an l-value, just call that.
+    } else {
+      QualType Result = PRE->getImplicitPropertyGetter()->getResultType();
+      ExprValueKind VK = Expr::getValueKindForType(Result);
+      if (VK == VK_LValue) {
+        LHS = ImplicitCastExpr::Create(Context, LHS->getType(),
+                                       CK_GetObjCProperty, LHS, 0, VK);
+        return;
       }
     }
-    copyInit = (getLangOptions().CPlusPlus && LHSTy->isRecordType());
-  } 
-  if (copyInit) {
+  }
+
+  if (getLangOptions().CPlusPlus && LHSTy->isRecordType()) {
     InitializedEntity Entity = 
     InitializedEntity::InitializeParameter(Context, LHSTy);
     Expr *Arg = RHS;
@@ -7318,7 +7380,8 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
   switch (Opc) {
   case BO_Assign:
     ResultTy = CheckAssignmentOperands(lhs, rhs, OpLoc, QualType());
-    if (getLangOptions().CPlusPlus) {
+    if (getLangOptions().CPlusPlus &&
+        lhs->getObjectKind() != OK_ObjCProperty) {
       VK = lhs->getValueKind();
       OK = lhs->getObjectKind();
     }
@@ -7418,7 +7481,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
     return Owned(new (Context) BinaryOperator(lhs, rhs, Opc, ResultTy,
                                               VK, OK, OpLoc));
 
-  if (getLangOptions().CPlusPlus) {
+  if (getLangOptions().CPlusPlus && lhs->getObjectKind() != OK_ObjCProperty) {
     VK = VK_LValue;
     OK = lhs->getObjectKind();
   }