Bug #:

Submitted by:
Reviewed by:
Implemented type checking for compound assignments (*=, /=, etc.).

This encouraged me to do a fairly dramatic refactoring of the Check* functions.
(since I wanted to reuse the existing work, rather than duplicate the logic).

For example, I changed all the Check* functions to return a QualType (instead
of returning an Expr). This had a very nice side benefit...there is now
only one instantiation point for BinaryOperator()! (A property I've always
wanted...separating type checking from AST building is *much* nicer). Another
change is to remove "code" from all the Check* functions (this allowed
me to remove the weird comment about enums/unsigned:-). Removing the
code forced me to add a few functions, however. For example,

<   ExprResult CheckAdditiveOperands( // C99 6.5.6
<     Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);

>   inline QualType CheckAdditionOperands( // C99 6.5.6
>     Expr *lex, Expr *rex, SourceLocation OpLoc);
>   inline QualType CheckSubtractionOperands( // C99 6.5.6
>     Expr *lex, Expr *rex, SourceLocation OpLoc);

While this isn't as terse, it more closely reflects the differences in
the typechecking logic. For example, I disliked having to check the code again
in CheckMultiplicativeOperands/CheckAdditiveOperands.

Created the following helper functions:
- Expr::isNullPointerConstant().
- SemaExpr.cpp: static inline BinaryOperator::Opcode ConvertTokenKindToBinaryOpcode().
This was purely asethetic, since ParseBinOp() is now larger. I didn't feel
like looking at 2 huge switch statements. ParseBinOp() now avoids using
any of the BinaryOperator predicates (since I switched to a switch statement:-)

Only one regret (minor). I couldn't figure out how to avoid having two assign functions,
CheckCompoundAssignmentOperands, CheckSimpleAssignmentOperands. Conceptually,
the two functions make sense. Unfortunately, their implementation contains a lot of
duplication (thought they aren't that be in the first place).

llvm-svn: 39433

6 files changed, 680 insertions, 387 deletions

diff --git a/clang/AST/Expr.cpp b/clang/AST/Expr.cpp
index fc1b0c56696..15b2b4b4e6e 100644
--- a/clang/AST/Expr.cpp
+++ b/clang/AST/Expr.cpp
@@ -153,3 +153,10 @@ bool Expr::isModifiableLvalue() {
     return false;
   }
 }
+
+bool Expr::isNullPointerConstant() const {
+  const IntegerLiteral *constant = dyn_cast<IntegerLiteral>(this);
+  if (!constant || constant->getValue() != 0)
+    return false;
+  return true;
+}
diff --git a/clang/AST/Sema.h b/clang/AST/Sema.h
index 0313cd47f1a..df1bde49ba0 100644
--- a/clang/AST/Sema.h
+++ b/clang/AST/Sema.h
@@ -233,32 +233,39 @@ private:
     IntFromPointer,
     IncompatiblePointer
   };
-  // Conversions for assignment, argument passing, initialization, or return
-  QualType UsualAssignmentConversions(QualType lhs, Expr *rex, // C99 6.5.16
+  // Conversions for assignment, argument passing, initialization, and
+  // function return values. UsualAssignmentConversions is currently used by 
+  // CheckSimpleAssignment, CheckCompoundAssignment and ParseCallExpr. 
+  QualType UsualAssignmentConversions(QualType lhs, QualType rhs, // C99 6.5.16
                                       AssignmentConversionResult &r); 
   
   /// the following "Check" methods will either return a well formed AST node
   /// or will return true if the expressions didn't type check properly.
   
   /// type checking binary operators (subroutines of ParseBinOp).
-  /// The unsigned arguments are really enums (BinaryOperator::Opcode)
-  ExprResult CheckMultiplicativeOperands( // C99 6.5.5
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode); 
-  ExprResult CheckAdditiveOperands( // C99 6.5.6
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckShiftOperands( // C99 6.5.7
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckRelationalOperands( // C99 6.5.8
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckEqualityOperands( // C99 6.5.9
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode); 
-  ExprResult CheckBitwiseOperands( // C99 6.5.[10...12]
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode); 
-  ExprResult CheckLogicalOperands( // C99 6.5.[13,14]
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckAssignmentOperands( // C99 6.5.16
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckCommaOperands( // C99 6.5.17
+  inline QualType CheckMultiplyDivideOperands( // C99 6.5.5
+    Expr *lex, Expr *rex, SourceLocation OpLoc); 
+  inline QualType CheckRemainderOperands( // C99 6.5.5
+    Expr *lex, Expr *rex, SourceLocation OpLoc); 
+  inline QualType CheckAdditionOperands( // C99 6.5.6
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckSubtractionOperands( // C99 6.5.6
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckShiftOperands( // C99 6.5.7
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckRelationalOperands( // C99 6.5.8
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckEqualityOperands( // C99 6.5.9
+    Expr *lex, Expr *rex, SourceLocation OpLoc); 
+  inline QualType CheckBitwiseOperands( // C99 6.5.[10...12]
+    Expr *lex, Expr *rex, SourceLocation OpLoc); 
+  inline QualType CheckLogicalOperands( // C99 6.5.[13,14]
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckSimpleAssignmentOperands( // C99 6.5.16.1
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckCompoundAssignmentOperands( // C99 6.5.16.2
+    Expr *lex, QualType, SourceLocation OpLoc);
+  inline QualType CheckCommaOperands( // C99 6.5.17
     Expr *lex, Expr *rex, SourceLocation OpLoc);
   
   /// type checking unary operators (subroutines of ParseUnaryOp).
diff --git a/clang/AST/SemaExpr.cpp b/clang/AST/SemaExpr.cpp
index 2f16c4a580d..3a7e3317264 100644
--- a/clang/AST/SemaExpr.cpp
+++ b/clang/AST/SemaExpr.cpp
@@ -365,7 +365,7 @@ ParseCallExpr(ExprTy *Fn, SourceLocation LParenLoc,
       if (lhsType == rhsType) // common case, fast path...
         continue;
       AssignmentConversionResult result;
-      UsualAssignmentConversions(lhsType, ((Expr **)Args)[i], result);
+      UsualAssignmentConversions(lhsType, rhsType, result);
 
       SourceLocation l = (i == 0) ? LParenLoc : CommaLocs[i-1];
 
@@ -377,7 +377,9 @@ ParseCallExpr(ExprTy *Fn, SourceLocation LParenLoc,
       case Compatible:
         break;
       case PointerFromInt:
-        Diag(l, diag::ext_typecheck_passing_pointer_from_int, utostr(i+1));
+        // check for null pointer constant (C99 6.3.2.3p3)
+        if (!((Expr **)Args)[i]->isNullPointerConstant())
+          Diag(l, diag::ext_typecheck_passing_pointer_from_int, utostr(i+1));
         break;
       case IntFromPointer:
         Diag(l, diag::ext_typecheck_passing_int_from_pointer, utostr(i+1));
@@ -401,73 +403,6 @@ ParseCastExpr(SourceLocation LParenLoc, TypeTy *Ty,
   return new CastExpr(QualType::getFromOpaquePtr(Ty), (Expr*)Op);
 }
 
-
-
-// Binary Operators.  'Tok' is the token for the operator.
-Action::ExprResult Sema::ParseBinOp(SourceLocation TokLoc, tok::TokenKind Kind,
-                                    ExprTy *LHS, ExprTy *RHS) {
-  BinaryOperator::Opcode Opc;
-  switch (Kind) {
-  default: assert(0 && "Unknown binop!");
-  case tok::star:                 Opc = BinaryOperator::Mul; break;
-  case tok::slash:                Opc = BinaryOperator::Div; break;
-  case tok::percent:              Opc = BinaryOperator::Rem; break;
-  case tok::plus:                 Opc = BinaryOperator::Add; break;
-  case tok::minus:                Opc = BinaryOperator::Sub; break;
-  case tok::lessless:             Opc = BinaryOperator::Shl; break;
-  case tok::greatergreater:       Opc = BinaryOperator::Shr; break;
-  case tok::lessequal:            Opc = BinaryOperator::LE; break;
-  case tok::less:                 Opc = BinaryOperator::LT; break;
-  case tok::greaterequal:         Opc = BinaryOperator::GE; break;
-  case tok::greater:              Opc = BinaryOperator::GT; break;
-  case tok::exclaimequal:         Opc = BinaryOperator::NE; break;
-  case tok::equalequal:           Opc = BinaryOperator::EQ; break;
-  case tok::amp:                  Opc = BinaryOperator::And; break;
-  case tok::caret:                Opc = BinaryOperator::Xor; break;
-  case tok::pipe:                 Opc = BinaryOperator::Or; break;
-  case tok::ampamp:               Opc = BinaryOperator::LAnd; break;
-  case tok::pipepipe:             Opc = BinaryOperator::LOr; break;
-  case tok::equal:                Opc = BinaryOperator::Assign; break;
-  case tok::starequal:            Opc = BinaryOperator::MulAssign; break;
-  case tok::slashequal:           Opc = BinaryOperator::DivAssign; break;
-  case tok::percentequal:         Opc = BinaryOperator::RemAssign; break;
-  case tok::plusequal:            Opc = BinaryOperator::AddAssign; break;
-  case tok::minusequal:           Opc = BinaryOperator::SubAssign; break;
-  case tok::lesslessequal:        Opc = BinaryOperator::ShlAssign; break;
-  case tok::greatergreaterequal:  Opc = BinaryOperator::ShrAssign; break;
-  case tok::ampequal:             Opc = BinaryOperator::AndAssign; break;
-  case tok::caretequal:           Opc = BinaryOperator::XorAssign; break;
-  case tok::pipeequal:            Opc = BinaryOperator::OrAssign; break;
-  case tok::comma:                Opc = BinaryOperator::Comma; break;
-  }
-
-  Expr *lhs = (Expr *)LHS, *rhs = (Expr*)RHS;
-
-  assert((lhs != 0) && "ParseBinOp(): missing left expression");
-  assert((rhs != 0) && "ParseBinOp(): missing right expression");
-
-  if (BinaryOperator::isMultiplicativeOp(Opc)) 
-    return CheckMultiplicativeOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isAdditiveOp(Opc))
-    return CheckAdditiveOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isShiftOp(Opc))
-    return CheckShiftOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isRelationalOp(Opc))
-    return CheckRelationalOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isEqualityOp(Opc))
-    return CheckEqualityOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isBitwiseOp(Opc))
-    return CheckBitwiseOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isLogicalOp(Opc))
-    return CheckLogicalOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isAssignmentOp(Opc))
-    return CheckAssignmentOperands(lhs, rhs, TokLoc, Opc);
-  else if (Opc == BinaryOperator::Comma)
-    return CheckCommaOperands(lhs, rhs, TokLoc);
-
-  assert(0 && "ParseBinOp(): illegal binary op");
-}
-
 /// ParseConditionalOp - Parse a ?: operation.  Note that 'LHS' may be null
 /// in the case of a the GNU conditional expr extension.
 Action::ExprResult Sema::ParseConditionalOp(SourceLocation QuestionLoc, 
@@ -562,10 +497,8 @@ QualType Sema::UsualArithmeticConversions(QualType t1, QualType t2) {
 /// C99 spec dictates. 
 /// Note: the warning above turn into errors when -pedantic-errors is enabled. 
 ///
-QualType Sema::UsualAssignmentConversions(QualType lhsType, Expr *rex,
+QualType Sema::UsualAssignmentConversions(QualType lhsType, QualType rhsType,
                                           AssignmentConversionResult &r) {
-  QualType rhsType = rex->getType();
-  
   // this check seems unnatural, however it necessary to insure the proper
   // conversion of functions/arrays. If the conversion where done for all
   // DeclExpr's (created by ParseIdentifierExpr), it would mess up the
@@ -578,10 +511,7 @@ QualType Sema::UsualAssignmentConversions(QualType lhsType, Expr *rex,
     return lhsType;
   else if (lhsType->isPointerType()) {
     if (rhsType->isIntegerType()) {
-      // check for null pointer constant (C99 6.3.2.3p3)
-      const IntegerLiteral *constant = dyn_cast<IntegerLiteral>(rex);
-      if (!constant || constant->getValue() != 0)
-        r = PointerFromInt;
+      r = PointerFromInt;
       return rhsType;
     }
     // FIXME: make sure the qualifier are matching
@@ -616,160 +546,223 @@ QualType Sema::UsualAssignmentConversions(QualType lhsType, Expr *rex,
   return QualType();
 }
 
-Action::ExprResult Sema::CheckMultiplicativeOperands(
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckMultiplyDivideOperands(
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
   
-  if ((BOP)code == BinaryOperator::Rem) {
-    if (!resType->isIntegerType())
-      return Diag(loc, diag::err_typecheck_invalid_operands);
-  } else { // *, /
-    if (!resType->isArithmeticType())
-      return Diag(loc, diag::err_typecheck_invalid_operands);
-  }
-  return new BinaryOperator(lex, rex, (BOP)code, resType);
+  if (resType->isArithmeticType())
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckAdditiveOperands( // C99 6.5.6
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckRemainderOperands(
+  Expr *lex, Expr *rex, SourceLocation loc) 
+{
+  QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
+  
+  if (resType->isIntegerType())
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
+}
+
+inline QualType Sema::CheckAdditionOperands( // C99 6.5.6
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType lhsType = lex->getType(), rhsType = rex->getType();
   QualType resType = UsualArithmeticConversions(lhsType, rhsType);
   
   // handle the common case first (both operands are arithmetic).
   if (resType->isArithmeticType())
-    return new BinaryOperator(lex, rex, (BOP)code, resType);
-  else {
-    if ((BOP)code == BinaryOperator::Add) {
-      if ((lhsType->isPointerType() && rhsType->isIntegerType()) ||
-          (lhsType->isIntegerType() && rhsType->isPointerType()))
-        return new BinaryOperator(lex, rex, (BOP)code, resType);
-    } else { // -
-      if ((lhsType->isPointerType() && rhsType->isIntegerType()) ||
-          (lhsType->isPointerType() && rhsType->isPointerType()))
-        return new BinaryOperator(lex, rex, (BOP)code, resType);
-    }
-  }
-  return Diag(loc, diag::err_typecheck_invalid_operands);
+    return resType;
+
+  if ((lhsType->isPointerType() && rhsType->isIntegerType()) ||
+      (lhsType->isIntegerType() && rhsType->isPointerType()))
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckShiftOperands( // C99 6.5.7
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code)
+inline QualType Sema::CheckSubtractionOperands( // C99 6.5.6
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
-  QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
+  QualType lhsType = lex->getType(), rhsType = rex->getType();
+  QualType resType = UsualArithmeticConversions(lhsType, rhsType);
   
-  if (!resType->isIntegerType())
-    return Diag(loc, diag::err_typecheck_invalid_operands);
+  // handle the common case first (both operands are arithmetic).
+  if (resType->isArithmeticType())
+    return resType;
+  if ((lhsType->isPointerType() && rhsType->isIntegerType()) ||
+      (lhsType->isPointerType() && rhsType->isPointerType()))
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
+}
 
-  return new BinaryOperator(lex, rex, (BOP)code, resType);
+inline QualType Sema::CheckShiftOperands( // C99 6.5.7
+  Expr *lex, Expr *rex, SourceLocation loc)
+{
+  QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
+  
+  if (resType->isIntegerType())
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckRelationalOperands( // C99 6.5.8
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code)
+inline QualType Sema::CheckRelationalOperands( // C99 6.5.8
+  Expr *lex, Expr *rex, SourceLocation loc)
 {
   QualType lType = lex->getType(), rType = rex->getType();
   
   if (lType->isRealType() && rType->isRealType())
-    return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
+    return Context.IntTy;
   
   if (lType->isPointerType() &&  rType->isPointerType())
-    return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
+    return Context.IntTy;
 
   if (lType->isIntegerType() || rType->isIntegerType()) // GCC extension.
-    return Diag(loc, diag::ext_typecheck_comparison_of_pointer_integer);
-  return Diag(loc, diag::err_typecheck_invalid_operands);
+    Diag(loc, diag::ext_typecheck_comparison_of_pointer_integer);
+  else
+    Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckEqualityOperands( // C99 6.5.9
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code)
+inline QualType Sema::CheckEqualityOperands( // C99 6.5.9
+  Expr *lex, Expr *rex, SourceLocation loc)
 {
   QualType lType = lex->getType(), rType = rex->getType();
   
   if (lType->isArithmeticType() && rType->isArithmeticType())
-    return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
-  
+    return Context.IntTy;
   if (lType->isPointerType() &&  rType->isPointerType())
-    return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
-
+    return Context.IntTy;
+    
   if (lType->isIntegerType() || rType->isIntegerType()) // GCC extension.
-    return Diag(loc, diag::ext_typecheck_comparison_of_pointer_integer);
-  return Diag(loc, diag::err_typecheck_invalid_operands);
+    Diag(loc, diag::ext_typecheck_comparison_of_pointer_integer);
+  else
+    Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckBitwiseOperands(
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckBitwiseOperands(
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
   
-  if (!resType->isIntegerType())
-    return Diag(loc, diag::err_typecheck_invalid_operands);
-
-  return new BinaryOperator(lex, rex, (BOP)code, resType);
+  if (resType->isIntegerType())
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckLogicalOperands( // C99 6.5.[13,14]
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckLogicalOperands( // C99 6.5.[13,14]
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType lhsType = UsualUnaryConversion(lex->getType());
   QualType rhsType = UsualUnaryConversion(rex->getType());
   
-  if (!lhsType->isScalarType() || !rhsType->isScalarType())
-    return Diag(loc, diag::err_typecheck_invalid_operands);
-    
-  return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
+  if (lhsType->isScalarType() || rhsType->isScalarType())
+    return Context.IntTy;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckAssignmentOperands( 
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckSimpleAssignmentOperands( // C99 6.5.16.1
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType lhsType = lex->getType();
   QualType rhsType = rex->getType();
   
-  if ((BOP)code == BinaryOperator::Assign) { // C99 6.5.16.1
-    // FIXME: consider hacking isModifiableLvalue to return an enum that
-    // communicates why the expression/type wasn't a modifiableLvalue.
-    
-    // this check is done first to give a more precise diagnostic.
-    if (lhsType.isConstQualified()) 
-      return Diag(loc, diag::err_typecheck_assign_const);
-
-    if (!lex->isModifiableLvalue()) // this includes checking for "const"
-      return Diag(loc, diag::ext_typecheck_assign_non_lvalue);
-      
-    if (lhsType == rhsType) // common case, fast path...
-      return new BinaryOperator(lex, rex, (BOP)code, lhsType);
-    
-    AssignmentConversionResult result;
-    QualType resType = UsualAssignmentConversions(lhsType, rex, result);
-
-    // decode the result (notice that AST's are still created for extensions).
-    switch (result) {
-    case Compatible:
-      break;
-    case PointerFromInt:
+  // FIXME: consider hacking isModifiableLvalue to return an enum that
+  // communicates why the expression/type wasn't a modifiableLvalue.
+  
+  // this check is done first to give a more precise diagnostic.
+  if (lhsType.isConstQualified()) {
+    Diag(loc, diag::err_typecheck_assign_const);
+    return QualType();
+  }
+  if (!lex->isModifiableLvalue()) { // this includes checking for "const"
+    Diag(loc, diag::ext_typecheck_assign_non_lvalue);
+    return QualType();
+  }
+  if (lhsType == rhsType) // common case, fast path...
+    return lhsType;
+  
+  AssignmentConversionResult result;
+  QualType resType = UsualAssignmentConversions(lhsType, rhsType, result);
+
+  // decode the result (notice that extensions still return a type).
+  switch (result) {
+  case Compatible:
+    return resType;
+  case Incompatible:
+    Diag(loc, diag::err_typecheck_assign_incompatible);
+    return QualType();
+  case PointerFromInt:
+    // check for null pointer constant (C99 6.3.2.3p3)
+    if (!rex->isNullPointerConstant())
       Diag(loc, diag::ext_typecheck_assign_pointer_from_int);
-      break;
-    case IntFromPointer:
-      Diag(loc, diag::ext_typecheck_assign_int_from_pointer);
-      break;
-    case IncompatiblePointer:
-      Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
-      break;
-    case Incompatible:
-      return Diag(loc, diag::err_typecheck_assign_incompatible);
-    }
-    return new BinaryOperator(lex, rex, (BOP)code, resType);
+    return resType;
+  case IntFromPointer:
+    Diag(loc, diag::ext_typecheck_assign_int_from_pointer);
+    return resType;
+  case IncompatiblePointer:
+    Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
+    return resType;
+  }
+  assert(0 && "should never get here");
+}
+
+inline QualType Sema::CheckCompoundAssignmentOperands( // C99 6.5.16.2
+  Expr *lex, QualType rhsType, SourceLocation loc) 
+{
+  QualType lhsType = lex->getType();
+  
+  // FIXME: consider hacking isModifiableLvalue to return an enum that
+  // communicates why the expression/type wasn't a modifiableLvalue.
+  
+  // this check is done first to give a more precise diagnostic.
+  if (lhsType.isConstQualified()) {
+    Diag(loc, diag::err_typecheck_assign_const);
+    return QualType();
+  }
+  if (!lex->isModifiableLvalue()) { // this includes checking for "const"
+    Diag(loc, diag::ext_typecheck_assign_non_lvalue);
+    return QualType();
+  }
+  if (lhsType == rhsType) // common case, fast path...
+    return lhsType;
+  
+  AssignmentConversionResult result;
+  QualType resType = UsualAssignmentConversions(lhsType, rhsType, result);
+
+  // decode the result (notice that extensions still return a type).
+  switch (result) {
+  case Compatible:
+    return resType;
+  case Incompatible:
+    Diag(loc, diag::err_typecheck_assign_incompatible);
+    return QualType();
+  case PointerFromInt:
+    Diag(loc, diag::ext_typecheck_assign_pointer_from_int);
+    return resType;
+  case IntFromPointer:
+    Diag(loc, diag::ext_typecheck_assign_int_from_pointer);
+    return resType;
+  case IncompatiblePointer:
+    Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
+    return resType;
   }
-  // FIXME: type check compound assignments...
-  return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
+  assert(0 && "should never get here");
 }
 
-Action::ExprResult Sema::CheckCommaOperands( // C99 6.5.17
+inline QualType Sema::CheckCommaOperands( // C99 6.5.17
   Expr *lex, Expr *rex, SourceLocation loc) 
 {
-  QualType rhsType = UsualUnaryConversion(rex->getType());
-  return new BinaryOperator(lex, rex, BinaryOperator::Comma, rhsType);
+  return UsualUnaryConversion(rex->getType());
 }
 
 Action::ExprResult
@@ -886,3 +879,145 @@ Sema::CheckArithmeticOperand(Expr *op, SourceLocation OpLoc, unsigned Opc) {
   }
   return new UnaryOperator(op, (UOP)Opc, resultType);
 }
+
+static inline BinaryOperator::Opcode ConvertTokenKindToBinaryOpcode(
+  tok::TokenKind Kind) {
+  BinaryOperator::Opcode Opc;
+  switch (Kind) {
+  default: assert(0 && "Unknown binop!");
+  case tok::star:                 Opc = BinaryOperator::Mul; break;
+  case tok::slash:                Opc = BinaryOperator::Div; break;
+  case tok::percent:              Opc = BinaryOperator::Rem; break;
+  case tok::plus:                 Opc = BinaryOperator::Add; break;
+  case tok::minus:                Opc = BinaryOperator::Sub; break;
+  case tok::lessless:             Opc = BinaryOperator::Shl; break;
+  case tok::greatergreater:       Opc = BinaryOperator::Shr; break;
+  case tok::lessequal:            Opc = BinaryOperator::LE; break;
+  case tok::less:                 Opc = BinaryOperator::LT; break;
+  case tok::greaterequal:         Opc = BinaryOperator::GE; break;
+  case tok::greater:              Opc = BinaryOperator::GT; break;
+  case tok::exclaimequal:         Opc = BinaryOperator::NE; break;
+  case tok::equalequal:           Opc = BinaryOperator::EQ; break;
+  case tok::amp:                  Opc = BinaryOperator::And; break;
+  case tok::caret:                Opc = BinaryOperator::Xor; break;
+  case tok::pipe:                 Opc = BinaryOperator::Or; break;
+  case tok::ampamp:               Opc = BinaryOperator::LAnd; break;
+  case tok::pipepipe:             Opc = BinaryOperator::LOr; break;
+  case tok::equal:                Opc = BinaryOperator::Assign; break;
+  case tok::starequal:            Opc = BinaryOperator::MulAssign; break;
+  case tok::slashequal:           Opc = BinaryOperator::DivAssign; break;
+  case tok::percentequal:         Opc = BinaryOperator::RemAssign; break;
+  case tok::plusequal:            Opc = BinaryOperator::AddAssign; break;
+  case tok::minusequal:           Opc = BinaryOperator::SubAssign; break;
+  case tok::lesslessequal:        Opc = BinaryOperator::ShlAssign; break;
+  case tok::greatergreaterequal:  Opc = BinaryOperator::ShrAssign; break;
+  case tok::ampequal:             Opc = BinaryOperator::AndAssign; break;
+  case tok::caretequal:           Opc = BinaryOperator::XorAssign; break;
+  case tok::pipeequal:            Opc = BinaryOperator::OrAssign; break;
+  case tok::comma:                Opc = BinaryOperator::Comma; break;
+  }
+  return Opc;
+}
+
+// Binary Operators.  'Tok' is the token for the operator.
+Action::ExprResult Sema::ParseBinOp(SourceLocation TokLoc, tok::TokenKind Kind,
+                                    ExprTy *LHS, ExprTy *RHS) {
+  BinaryOperator::Opcode Opc = ConvertTokenKindToBinaryOpcode(Kind);
+  Expr *lhs = (Expr *)LHS, *rhs = (Expr*)RHS;
+
+  assert((lhs != 0) && "ParseBinOp(): missing left expression");
+  assert((rhs != 0) && "ParseBinOp(): missing right expression");
+
+  QualType result;
+  
+  switch (Opc) {
+  default:
+    assert(0 && "Unknown binary expr!");
+  case BinaryOperator::Assign:
+    result = CheckSimpleAssignmentOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Mul: 
+  case BinaryOperator::Div:
+    result = CheckMultiplyDivideOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Rem:
+    result = CheckRemainderOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Add:
+    result = CheckAdditionOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Sub:
+    result = CheckSubtractionOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Shl: 
+  case BinaryOperator::Shr:
+    result = CheckShiftOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::LE:
+  case BinaryOperator::LT:
+  case BinaryOperator::GE:
+  case BinaryOperator::GT:
+    result = CheckRelationalOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::EQ:
+  case BinaryOperator::NE:
+    result = CheckEqualityOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::And:
+  case BinaryOperator::Xor:
+  case BinaryOperator::Or:
+    result = CheckBitwiseOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::LAnd:
+  case BinaryOperator::LOr:
+    result = CheckLogicalOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::MulAssign:
+  case BinaryOperator::DivAssign:
+    result = CheckMultiplyDivideOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::RemAssign:
+    result = CheckRemainderOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::AddAssign:
+    result = CheckAdditionOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::SubAssign:
+    result = CheckSubtractionOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::ShlAssign:
+  case BinaryOperator::ShrAssign:
+    result = CheckShiftOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::AndAssign:
+  case BinaryOperator::XorAssign:
+  case BinaryOperator::OrAssign:
+    result = CheckBitwiseOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::Comma:
+    result = CheckCommaOperands(lhs, rhs, TokLoc);
+    break;
+  }
+  if (result.isNull())
+    return true;
+  return new BinaryOperator(lhs, rhs, Opc, result);
+}
+
diff --git a/clang/Sema/Sema.h b/clang/Sema/Sema.h
index 0313cd47f1a..df1bde49ba0 100644
--- a/clang/Sema/Sema.h
+++ b/clang/Sema/Sema.h
@@ -233,32 +233,39 @@ private:
     IntFromPointer,
     IncompatiblePointer
   };
-  // Conversions for assignment, argument passing, initialization, or return
-  QualType UsualAssignmentConversions(QualType lhs, Expr *rex, // C99 6.5.16
+  // Conversions for assignment, argument passing, initialization, and
+  // function return values. UsualAssignmentConversions is currently used by 
+  // CheckSimpleAssignment, CheckCompoundAssignment and ParseCallExpr. 
+  QualType UsualAssignmentConversions(QualType lhs, QualType rhs, // C99 6.5.16
                                       AssignmentConversionResult &r); 
   
   /// the following "Check" methods will either return a well formed AST node
   /// or will return true if the expressions didn't type check properly.
   
   /// type checking binary operators (subroutines of ParseBinOp).
-  /// The unsigned arguments are really enums (BinaryOperator::Opcode)
-  ExprResult CheckMultiplicativeOperands( // C99 6.5.5
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode); 
-  ExprResult CheckAdditiveOperands( // C99 6.5.6
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckShiftOperands( // C99 6.5.7
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckRelationalOperands( // C99 6.5.8
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckEqualityOperands( // C99 6.5.9
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode); 
-  ExprResult CheckBitwiseOperands( // C99 6.5.[10...12]
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode); 
-  ExprResult CheckLogicalOperands( // C99 6.5.[13,14]
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckAssignmentOperands( // C99 6.5.16
-    Expr *lex, Expr *rex, SourceLocation OpLoc, unsigned OpCode);
-  ExprResult CheckCommaOperands( // C99 6.5.17
+  inline QualType CheckMultiplyDivideOperands( // C99 6.5.5
+    Expr *lex, Expr *rex, SourceLocation OpLoc); 
+  inline QualType CheckRemainderOperands( // C99 6.5.5
+    Expr *lex, Expr *rex, SourceLocation OpLoc); 
+  inline QualType CheckAdditionOperands( // C99 6.5.6
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckSubtractionOperands( // C99 6.5.6
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckShiftOperands( // C99 6.5.7
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckRelationalOperands( // C99 6.5.8
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckEqualityOperands( // C99 6.5.9
+    Expr *lex, Expr *rex, SourceLocation OpLoc); 
+  inline QualType CheckBitwiseOperands( // C99 6.5.[10...12]
+    Expr *lex, Expr *rex, SourceLocation OpLoc); 
+  inline QualType CheckLogicalOperands( // C99 6.5.[13,14]
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckSimpleAssignmentOperands( // C99 6.5.16.1
+    Expr *lex, Expr *rex, SourceLocation OpLoc);
+  inline QualType CheckCompoundAssignmentOperands( // C99 6.5.16.2
+    Expr *lex, QualType, SourceLocation OpLoc);
+  inline QualType CheckCommaOperands( // C99 6.5.17
     Expr *lex, Expr *rex, SourceLocation OpLoc);
   
   /// type checking unary operators (subroutines of ParseUnaryOp).
diff --git a/clang/Sema/SemaExpr.cpp b/clang/Sema/SemaExpr.cpp
index 2f16c4a580d..3a7e3317264 100644
--- a/clang/Sema/SemaExpr.cpp
+++ b/clang/Sema/SemaExpr.cpp
@@ -365,7 +365,7 @@ ParseCallExpr(ExprTy *Fn, SourceLocation LParenLoc,
       if (lhsType == rhsType) // common case, fast path...
         continue;
       AssignmentConversionResult result;
-      UsualAssignmentConversions(lhsType, ((Expr **)Args)[i], result);
+      UsualAssignmentConversions(lhsType, rhsType, result);
 
       SourceLocation l = (i == 0) ? LParenLoc : CommaLocs[i-1];
 
@@ -377,7 +377,9 @@ ParseCallExpr(ExprTy *Fn, SourceLocation LParenLoc,
       case Compatible:
         break;
       case PointerFromInt:
-        Diag(l, diag::ext_typecheck_passing_pointer_from_int, utostr(i+1));
+        // check for null pointer constant (C99 6.3.2.3p3)
+        if (!((Expr **)Args)[i]->isNullPointerConstant())
+          Diag(l, diag::ext_typecheck_passing_pointer_from_int, utostr(i+1));
         break;
       case IntFromPointer:
         Diag(l, diag::ext_typecheck_passing_int_from_pointer, utostr(i+1));
@@ -401,73 +403,6 @@ ParseCastExpr(SourceLocation LParenLoc, TypeTy *Ty,
   return new CastExpr(QualType::getFromOpaquePtr(Ty), (Expr*)Op);
 }
 
-
-
-// Binary Operators.  'Tok' is the token for the operator.
-Action::ExprResult Sema::ParseBinOp(SourceLocation TokLoc, tok::TokenKind Kind,
-                                    ExprTy *LHS, ExprTy *RHS) {
-  BinaryOperator::Opcode Opc;
-  switch (Kind) {
-  default: assert(0 && "Unknown binop!");
-  case tok::star:                 Opc = BinaryOperator::Mul; break;
-  case tok::slash:                Opc = BinaryOperator::Div; break;
-  case tok::percent:              Opc = BinaryOperator::Rem; break;
-  case tok::plus:                 Opc = BinaryOperator::Add; break;
-  case tok::minus:                Opc = BinaryOperator::Sub; break;
-  case tok::lessless:             Opc = BinaryOperator::Shl; break;
-  case tok::greatergreater:       Opc = BinaryOperator::Shr; break;
-  case tok::lessequal:            Opc = BinaryOperator::LE; break;
-  case tok::less:                 Opc = BinaryOperator::LT; break;
-  case tok::greaterequal:         Opc = BinaryOperator::GE; break;
-  case tok::greater:              Opc = BinaryOperator::GT; break;
-  case tok::exclaimequal:         Opc = BinaryOperator::NE; break;
-  case tok::equalequal:           Opc = BinaryOperator::EQ; break;
-  case tok::amp:                  Opc = BinaryOperator::And; break;
-  case tok::caret:                Opc = BinaryOperator::Xor; break;
-  case tok::pipe:                 Opc = BinaryOperator::Or; break;
-  case tok::ampamp:               Opc = BinaryOperator::LAnd; break;
-  case tok::pipepipe:             Opc = BinaryOperator::LOr; break;
-  case tok::equal:                Opc = BinaryOperator::Assign; break;
-  case tok::starequal:            Opc = BinaryOperator::MulAssign; break;
-  case tok::slashequal:           Opc = BinaryOperator::DivAssign; break;
-  case tok::percentequal:         Opc = BinaryOperator::RemAssign; break;
-  case tok::plusequal:            Opc = BinaryOperator::AddAssign; break;
-  case tok::minusequal:           Opc = BinaryOperator::SubAssign; break;
-  case tok::lesslessequal:        Opc = BinaryOperator::ShlAssign; break;
-  case tok::greatergreaterequal:  Opc = BinaryOperator::ShrAssign; break;
-  case tok::ampequal:             Opc = BinaryOperator::AndAssign; break;
-  case tok::caretequal:           Opc = BinaryOperator::XorAssign; break;
-  case tok::pipeequal:            Opc = BinaryOperator::OrAssign; break;
-  case tok::comma:                Opc = BinaryOperator::Comma; break;
-  }
-
-  Expr *lhs = (Expr *)LHS, *rhs = (Expr*)RHS;
-
-  assert((lhs != 0) && "ParseBinOp(): missing left expression");
-  assert((rhs != 0) && "ParseBinOp(): missing right expression");
-
-  if (BinaryOperator::isMultiplicativeOp(Opc)) 
-    return CheckMultiplicativeOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isAdditiveOp(Opc))
-    return CheckAdditiveOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isShiftOp(Opc))
-    return CheckShiftOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isRelationalOp(Opc))
-    return CheckRelationalOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isEqualityOp(Opc))
-    return CheckEqualityOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isBitwiseOp(Opc))
-    return CheckBitwiseOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isLogicalOp(Opc))
-    return CheckLogicalOperands(lhs, rhs, TokLoc, Opc);
-  else if (BinaryOperator::isAssignmentOp(Opc))
-    return CheckAssignmentOperands(lhs, rhs, TokLoc, Opc);
-  else if (Opc == BinaryOperator::Comma)
-    return CheckCommaOperands(lhs, rhs, TokLoc);
-
-  assert(0 && "ParseBinOp(): illegal binary op");
-}
-
 /// ParseConditionalOp - Parse a ?: operation.  Note that 'LHS' may be null
 /// in the case of a the GNU conditional expr extension.
 Action::ExprResult Sema::ParseConditionalOp(SourceLocation QuestionLoc, 
@@ -562,10 +497,8 @@ QualType Sema::UsualArithmeticConversions(QualType t1, QualType t2) {
 /// C99 spec dictates. 
 /// Note: the warning above turn into errors when -pedantic-errors is enabled. 
 ///
-QualType Sema::UsualAssignmentConversions(QualType lhsType, Expr *rex,
+QualType Sema::UsualAssignmentConversions(QualType lhsType, QualType rhsType,
                                           AssignmentConversionResult &r) {
-  QualType rhsType = rex->getType();
-  
   // this check seems unnatural, however it necessary to insure the proper
   // conversion of functions/arrays. If the conversion where done for all
   // DeclExpr's (created by ParseIdentifierExpr), it would mess up the
@@ -578,10 +511,7 @@ QualType Sema::UsualAssignmentConversions(QualType lhsType, Expr *rex,
     return lhsType;
   else if (lhsType->isPointerType()) {
     if (rhsType->isIntegerType()) {
-      // check for null pointer constant (C99 6.3.2.3p3)
-      const IntegerLiteral *constant = dyn_cast<IntegerLiteral>(rex);
-      if (!constant || constant->getValue() != 0)
-        r = PointerFromInt;
+      r = PointerFromInt;
       return rhsType;
     }
     // FIXME: make sure the qualifier are matching
@@ -616,160 +546,223 @@ QualType Sema::UsualAssignmentConversions(QualType lhsType, Expr *rex,
   return QualType();
 }
 
-Action::ExprResult Sema::CheckMultiplicativeOperands(
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckMultiplyDivideOperands(
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
   
-  if ((BOP)code == BinaryOperator::Rem) {
-    if (!resType->isIntegerType())
-      return Diag(loc, diag::err_typecheck_invalid_operands);
-  } else { // *, /
-    if (!resType->isArithmeticType())
-      return Diag(loc, diag::err_typecheck_invalid_operands);
-  }
-  return new BinaryOperator(lex, rex, (BOP)code, resType);
+  if (resType->isArithmeticType())
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckAdditiveOperands( // C99 6.5.6
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckRemainderOperands(
+  Expr *lex, Expr *rex, SourceLocation loc) 
+{
+  QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
+  
+  if (resType->isIntegerType())
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
+}
+
+inline QualType Sema::CheckAdditionOperands( // C99 6.5.6
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType lhsType = lex->getType(), rhsType = rex->getType();
   QualType resType = UsualArithmeticConversions(lhsType, rhsType);
   
   // handle the common case first (both operands are arithmetic).
   if (resType->isArithmeticType())
-    return new BinaryOperator(lex, rex, (BOP)code, resType);
-  else {
-    if ((BOP)code == BinaryOperator::Add) {
-      if ((lhsType->isPointerType() && rhsType->isIntegerType()) ||
-          (lhsType->isIntegerType() && rhsType->isPointerType()))
-        return new BinaryOperator(lex, rex, (BOP)code, resType);
-    } else { // -
-      if ((lhsType->isPointerType() && rhsType->isIntegerType()) ||
-          (lhsType->isPointerType() && rhsType->isPointerType()))
-        return new BinaryOperator(lex, rex, (BOP)code, resType);
-    }
-  }
-  return Diag(loc, diag::err_typecheck_invalid_operands);
+    return resType;
+
+  if ((lhsType->isPointerType() && rhsType->isIntegerType()) ||
+      (lhsType->isIntegerType() && rhsType->isPointerType()))
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckShiftOperands( // C99 6.5.7
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code)
+inline QualType Sema::CheckSubtractionOperands( // C99 6.5.6
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
-  QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
+  QualType lhsType = lex->getType(), rhsType = rex->getType();
+  QualType resType = UsualArithmeticConversions(lhsType, rhsType);
   
-  if (!resType->isIntegerType())
-    return Diag(loc, diag::err_typecheck_invalid_operands);
+  // handle the common case first (both operands are arithmetic).
+  if (resType->isArithmeticType())
+    return resType;
+  if ((lhsType->isPointerType() && rhsType->isIntegerType()) ||
+      (lhsType->isPointerType() && rhsType->isPointerType()))
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
+}
 
-  return new BinaryOperator(lex, rex, (BOP)code, resType);
+inline QualType Sema::CheckShiftOperands( // C99 6.5.7
+  Expr *lex, Expr *rex, SourceLocation loc)
+{
+  QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
+  
+  if (resType->isIntegerType())
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckRelationalOperands( // C99 6.5.8
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code)
+inline QualType Sema::CheckRelationalOperands( // C99 6.5.8
+  Expr *lex, Expr *rex, SourceLocation loc)
 {
   QualType lType = lex->getType(), rType = rex->getType();
   
   if (lType->isRealType() && rType->isRealType())
-    return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
+    return Context.IntTy;
   
   if (lType->isPointerType() &&  rType->isPointerType())
-    return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
+    return Context.IntTy;
 
   if (lType->isIntegerType() || rType->isIntegerType()) // GCC extension.
-    return Diag(loc, diag::ext_typecheck_comparison_of_pointer_integer);
-  return Diag(loc, diag::err_typecheck_invalid_operands);
+    Diag(loc, diag::ext_typecheck_comparison_of_pointer_integer);
+  else
+    Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckEqualityOperands( // C99 6.5.9
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code)
+inline QualType Sema::CheckEqualityOperands( // C99 6.5.9
+  Expr *lex, Expr *rex, SourceLocation loc)
 {
   QualType lType = lex->getType(), rType = rex->getType();
   
   if (lType->isArithmeticType() && rType->isArithmeticType())
-    return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
-  
+    return Context.IntTy;
   if (lType->isPointerType() &&  rType->isPointerType())
-    return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
-
+    return Context.IntTy;
+    
   if (lType->isIntegerType() || rType->isIntegerType()) // GCC extension.
-    return Diag(loc, diag::ext_typecheck_comparison_of_pointer_integer);
-  return Diag(loc, diag::err_typecheck_invalid_operands);
+    Diag(loc, diag::ext_typecheck_comparison_of_pointer_integer);
+  else
+    Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckBitwiseOperands(
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckBitwiseOperands(
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType resType = UsualArithmeticConversions(lex->getType(), rex->getType());
   
-  if (!resType->isIntegerType())
-    return Diag(loc, diag::err_typecheck_invalid_operands);
-
-  return new BinaryOperator(lex, rex, (BOP)code, resType);
+  if (resType->isIntegerType())
+    return resType;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckLogicalOperands( // C99 6.5.[13,14]
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckLogicalOperands( // C99 6.5.[13,14]
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType lhsType = UsualUnaryConversion(lex->getType());
   QualType rhsType = UsualUnaryConversion(rex->getType());
   
-  if (!lhsType->isScalarType() || !rhsType->isScalarType())
-    return Diag(loc, diag::err_typecheck_invalid_operands);
-    
-  return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
+  if (lhsType->isScalarType() || rhsType->isScalarType())
+    return Context.IntTy;
+  Diag(loc, diag::err_typecheck_invalid_operands);
+  return QualType();
 }
 
-Action::ExprResult Sema::CheckAssignmentOperands( 
-  Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
+inline QualType Sema::CheckSimpleAssignmentOperands( // C99 6.5.16.1
+  Expr *lex, Expr *rex, SourceLocation loc) 
 {
   QualType lhsType = lex->getType();
   QualType rhsType = rex->getType();
   
-  if ((BOP)code == BinaryOperator::Assign) { // C99 6.5.16.1
-    // FIXME: consider hacking isModifiableLvalue to return an enum that
-    // communicates why the expression/type wasn't a modifiableLvalue.
-    
-    // this check is done first to give a more precise diagnostic.
-    if (lhsType.isConstQualified()) 
-      return Diag(loc, diag::err_typecheck_assign_const);
-
-    if (!lex->isModifiableLvalue()) // this includes checking for "const"
-      return Diag(loc, diag::ext_typecheck_assign_non_lvalue);
-      
-    if (lhsType == rhsType) // common case, fast path...
-      return new BinaryOperator(lex, rex, (BOP)code, lhsType);
-    
-    AssignmentConversionResult result;
-    QualType resType = UsualAssignmentConversions(lhsType, rex, result);
-
-    // decode the result (notice that AST's are still created for extensions).
-    switch (result) {
-    case Compatible:
-      break;
-    case PointerFromInt:
+  // FIXME: consider hacking isModifiableLvalue to return an enum that
+  // communicates why the expression/type wasn't a modifiableLvalue.
+  
+  // this check is done first to give a more precise diagnostic.
+  if (lhsType.isConstQualified()) {
+    Diag(loc, diag::err_typecheck_assign_const);
+    return QualType();
+  }
+  if (!lex->isModifiableLvalue()) { // this includes checking for "const"
+    Diag(loc, diag::ext_typecheck_assign_non_lvalue);
+    return QualType();
+  }
+  if (lhsType == rhsType) // common case, fast path...
+    return lhsType;
+  
+  AssignmentConversionResult result;
+  QualType resType = UsualAssignmentConversions(lhsType, rhsType, result);
+
+  // decode the result (notice that extensions still return a type).
+  switch (result) {
+  case Compatible:
+    return resType;
+  case Incompatible:
+    Diag(loc, diag::err_typecheck_assign_incompatible);
+    return QualType();
+  case PointerFromInt:
+    // check for null pointer constant (C99 6.3.2.3p3)
+    if (!rex->isNullPointerConstant())
       Diag(loc, diag::ext_typecheck_assign_pointer_from_int);
-      break;
-    case IntFromPointer:
-      Diag(loc, diag::ext_typecheck_assign_int_from_pointer);
-      break;
-    case IncompatiblePointer:
-      Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
-      break;
-    case Incompatible:
-      return Diag(loc, diag::err_typecheck_assign_incompatible);
-    }
-    return new BinaryOperator(lex, rex, (BOP)code, resType);
+    return resType;
+  case IntFromPointer:
+    Diag(loc, diag::ext_typecheck_assign_int_from_pointer);
+    return resType;
+  case IncompatiblePointer:
+    Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
+    return resType;
+  }
+  assert(0 && "should never get here");
+}
+
+inline QualType Sema::CheckCompoundAssignmentOperands( // C99 6.5.16.2
+  Expr *lex, QualType rhsType, SourceLocation loc) 
+{
+  QualType lhsType = lex->getType();
+  
+  // FIXME: consider hacking isModifiableLvalue to return an enum that
+  // communicates why the expression/type wasn't a modifiableLvalue.
+  
+  // this check is done first to give a more precise diagnostic.
+  if (lhsType.isConstQualified()) {
+    Diag(loc, diag::err_typecheck_assign_const);
+    return QualType();
+  }
+  if (!lex->isModifiableLvalue()) { // this includes checking for "const"
+    Diag(loc, diag::ext_typecheck_assign_non_lvalue);
+    return QualType();
+  }
+  if (lhsType == rhsType) // common case, fast path...
+    return lhsType;
+  
+  AssignmentConversionResult result;
+  QualType resType = UsualAssignmentConversions(lhsType, rhsType, result);
+
+  // decode the result (notice that extensions still return a type).
+  switch (result) {
+  case Compatible:
+    return resType;
+  case Incompatible:
+    Diag(loc, diag::err_typecheck_assign_incompatible);
+    return QualType();
+  case PointerFromInt:
+    Diag(loc, diag::ext_typecheck_assign_pointer_from_int);
+    return resType;
+  case IntFromPointer:
+    Diag(loc, diag::ext_typecheck_assign_int_from_pointer);
+    return resType;
+  case IncompatiblePointer:
+    Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
+    return resType;
   }
-  // FIXME: type check compound assignments...
-  return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
+  assert(0 && "should never get here");
 }
 
-Action::ExprResult Sema::CheckCommaOperands( // C99 6.5.17
+inline QualType Sema::CheckCommaOperands( // C99 6.5.17
   Expr *lex, Expr *rex, SourceLocation loc) 
 {
-  QualType rhsType = UsualUnaryConversion(rex->getType());
-  return new BinaryOperator(lex, rex, BinaryOperator::Comma, rhsType);
+  return UsualUnaryConversion(rex->getType());
 }
 
 Action::ExprResult
@@ -886,3 +879,145 @@ Sema::CheckArithmeticOperand(Expr *op, SourceLocation OpLoc, unsigned Opc) {
   }
   return new UnaryOperator(op, (UOP)Opc, resultType);
 }
+
+static inline BinaryOperator::Opcode ConvertTokenKindToBinaryOpcode(
+  tok::TokenKind Kind) {
+  BinaryOperator::Opcode Opc;
+  switch (Kind) {
+  default: assert(0 && "Unknown binop!");
+  case tok::star:                 Opc = BinaryOperator::Mul; break;
+  case tok::slash:                Opc = BinaryOperator::Div; break;
+  case tok::percent:              Opc = BinaryOperator::Rem; break;
+  case tok::plus:                 Opc = BinaryOperator::Add; break;
+  case tok::minus:                Opc = BinaryOperator::Sub; break;
+  case tok::lessless:             Opc = BinaryOperator::Shl; break;
+  case tok::greatergreater:       Opc = BinaryOperator::Shr; break;
+  case tok::lessequal:            Opc = BinaryOperator::LE; break;
+  case tok::less:                 Opc = BinaryOperator::LT; break;
+  case tok::greaterequal:         Opc = BinaryOperator::GE; break;
+  case tok::greater:              Opc = BinaryOperator::GT; break;
+  case tok::exclaimequal:         Opc = BinaryOperator::NE; break;
+  case tok::equalequal:           Opc = BinaryOperator::EQ; break;
+  case tok::amp:                  Opc = BinaryOperator::And; break;
+  case tok::caret:                Opc = BinaryOperator::Xor; break;
+  case tok::pipe:                 Opc = BinaryOperator::Or; break;
+  case tok::ampamp:               Opc = BinaryOperator::LAnd; break;
+  case tok::pipepipe:             Opc = BinaryOperator::LOr; break;
+  case tok::equal:                Opc = BinaryOperator::Assign; break;
+  case tok::starequal:            Opc = BinaryOperator::MulAssign; break;
+  case tok::slashequal:           Opc = BinaryOperator::DivAssign; break;
+  case tok::percentequal:         Opc = BinaryOperator::RemAssign; break;
+  case tok::plusequal:            Opc = BinaryOperator::AddAssign; break;
+  case tok::minusequal:           Opc = BinaryOperator::SubAssign; break;
+  case tok::lesslessequal:        Opc = BinaryOperator::ShlAssign; break;
+  case tok::greatergreaterequal:  Opc = BinaryOperator::ShrAssign; break;
+  case tok::ampequal:             Opc = BinaryOperator::AndAssign; break;
+  case tok::caretequal:           Opc = BinaryOperator::XorAssign; break;
+  case tok::pipeequal:            Opc = BinaryOperator::OrAssign; break;
+  case tok::comma:                Opc = BinaryOperator::Comma; break;
+  }
+  return Opc;
+}
+
+// Binary Operators.  'Tok' is the token for the operator.
+Action::ExprResult Sema::ParseBinOp(SourceLocation TokLoc, tok::TokenKind Kind,
+                                    ExprTy *LHS, ExprTy *RHS) {
+  BinaryOperator::Opcode Opc = ConvertTokenKindToBinaryOpcode(Kind);
+  Expr *lhs = (Expr *)LHS, *rhs = (Expr*)RHS;
+
+  assert((lhs != 0) && "ParseBinOp(): missing left expression");
+  assert((rhs != 0) && "ParseBinOp(): missing right expression");
+
+  QualType result;
+  
+  switch (Opc) {
+  default:
+    assert(0 && "Unknown binary expr!");
+  case BinaryOperator::Assign:
+    result = CheckSimpleAssignmentOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Mul: 
+  case BinaryOperator::Div:
+    result = CheckMultiplyDivideOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Rem:
+    result = CheckRemainderOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Add:
+    result = CheckAdditionOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Sub:
+    result = CheckSubtractionOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::Shl: 
+  case BinaryOperator::Shr:
+    result = CheckShiftOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::LE:
+  case BinaryOperator::LT:
+  case BinaryOperator::GE:
+  case BinaryOperator::GT:
+    result = CheckRelationalOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::EQ:
+  case BinaryOperator::NE:
+    result = CheckEqualityOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::And:
+  case BinaryOperator::Xor:
+  case BinaryOperator::Or:
+    result = CheckBitwiseOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::LAnd:
+  case BinaryOperator::LOr:
+    result = CheckLogicalOperands(lhs, rhs, TokLoc);
+    break;
+  case BinaryOperator::MulAssign:
+  case BinaryOperator::DivAssign:
+    result = CheckMultiplyDivideOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::RemAssign:
+    result = CheckRemainderOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::AddAssign:
+    result = CheckAdditionOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::SubAssign:
+    result = CheckSubtractionOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::ShlAssign:
+  case BinaryOperator::ShrAssign:
+    result = CheckShiftOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::AndAssign:
+  case BinaryOperator::XorAssign:
+  case BinaryOperator::OrAssign:
+    result = CheckBitwiseOperands(lhs, rhs, TokLoc);
+    if (result.isNull())
+      return true;
+    result = CheckCompoundAssignmentOperands(lhs, result, TokLoc);
+    break;
+  case BinaryOperator::Comma:
+    result = CheckCommaOperands(lhs, rhs, TokLoc);
+    break;
+  }
+  if (result.isNull())
+    return true;
+  return new BinaryOperator(lhs, rhs, Opc, result);
+}
+
diff --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 580c5199d48..3643d468249 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -46,6 +46,8 @@ public:
   ///
   bool isModifiableLvalue();
   
+  bool isNullPointerConstant() const;
+
   virtual void visit(StmtVisitor &Visitor);
   static bool classof(const Stmt *T) { 
     return T->getStmtClass() >= firstExprConstant &&
@@ -365,9 +367,9 @@ public:
   static bool isMultiplicativeOp(Opcode Op) { return Op >= Mul && Op <= Rem; }
   static bool isAdditiveOp(Opcode Op) { return Op == Add || Op == Sub; }
   static bool isShiftOp(Opcode Op) { return Op == Shl || Op == Shr; }
+  static bool isBitwiseOp(Opcode Op) { return Op >= And && Op <= Or; }
   static bool isRelationalOp(Opcode Op) { return Op >= LT && Op <= GE; }
   static bool isEqualityOp(Opcode Op) { return Op == EQ || Op == NE; }
-  static bool isBitwiseOp(Opcode Op) { return Op >= And && Op <= Or; }
   static bool isLogicalOp(Opcode Op) { return Op == LAnd || Op == LOr; }
   static bool isAssignmentOp(Opcode Op) { return Op >= Assign && Op<=OrAssign; }