Bug #:

Submitted by:
Reviewed by:
Implement CheckAssignmentOperands(). This includes...

- Adding 6 static member predicates to Type.
- Adding 2 error diagnostics and 3 GCC extensions.
- Adding a "getValue" accessor to IntegerLiteral.

Still more work to do (including implement compound assignments).

llvm-svn: 39427

clang/AST/SemaExpr.cpp

@@ -601,13 +601,79 @@ Action::ExprResult Sema::CheckLogicalOperands( // C99 6.5.[13,14]
   return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
 }
 
-Action::ExprResult Sema::CheckAssignmentOperands( // C99 6.5.16
+/// CheckAssignmentOperands (C99 6.5.16) - This routine currently 
+/// has code to accommodate several GCC extensions when type checking 
+/// pointers. Here are some objectionable examples that GCC considers warnings:
+///
+///  int a, *pint;
+///  short *pshort;
+///  struct foo *pfoo;
+///
+///  pint = pshort; // warning: assignment from incompatible pointer type
+///  a = pint; // warning: assignment makes integer from pointer without a cast
+///  pint = a; // warning: assignment makes pointer from integer without a cast
+///  pint = pfoo; // warning: assignment from incompatible pointer type
+///
+/// As a result, the code for dealing with pointers is more complex than the
+/// C99 spec dictates. 
+/// Note: the warning above turn into errors when -pedantic-errors is enabled. 
+///
+Action::ExprResult Sema::CheckAssignmentOperands( 
   Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
 {
   QualType lhsType = lex->getType();
   QualType rhsType = rex->getType();
   
-  // FIXME: add type checking and fix result type
+  if ((BOP)code == BinaryOperator::Assign) { // C99 6.5.16.1
+    if (lhsType.isConstQualified())
+      return Diag(loc, diag::err_typecheck_assign_const);
+      
+    if (lhsType->isArithmeticType() && rhsType->isArithmeticType()) {
+      return new BinaryOperator(lex, rex, (BOP)code, 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)
+          Diag(loc, diag::ext_typecheck_assign_pointer_from_int);
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+      }
+      // FIXME: make sure the qualifier are matching
+      if (rhsType->isPointerType()) { 
+        if (!Type::pointerTypesAreCompatible(lhsType, rhsType))
+          Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+      }
+    } else if (rhsType->isPointerType()) {
+      if (lhsType->isIntegerType()) {
+        Diag(loc, diag::ext_typecheck_assign_int_from_pointer);
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+      }
+      // FIXME: make sure the qualifier are matching
+      if (lhsType->isPointerType()) {
+        if (!Type::pointerTypesAreCompatible(lhsType, rhsType))
+          Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+      }
+    } else if (lhsType->isArrayType() && rhsType->isArrayType()) {
+      ///  int aryInt[3], aryInt2[3];
+      ///  aryInt = aryInt2; // gcc considers this an error (FIXME?)
+      if (Type::arrayTypesAreCompatible(lhsType, rhsType))
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+    } else if (lhsType->isStructureType() && rhsType->isStructureType()) {
+      if (Type::structureTypesAreCompatible(lhsType, rhsType))
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+    } else if (lhsType->isUnionType() && rhsType->isUnionType()) {
+      if (Type::unionTypesAreCompatible(lhsType, rhsType))
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+    } else if (lhsType->isFunctionType() && rhsType->isFunctionType()) {
+      if (Type::functionTypesAreCompatible(lhsType, rhsType))
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+    }
+    return Diag(loc, diag::err_typecheck_assign_incompatible);
+  }
+  
+  // FIXME: type check compound assignments...
   return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
 }
 

clang/AST/Type.cpp

@@ -71,6 +71,76 @@ bool Type::isUnionType() const {
   return false;
 }
 
+// C99 6.2.7p1: If both are complete types, then the following additional
+// requirements apply...FIXME (handle compatibility across source files).
+bool Type::structureTypesAreCompatible(QualType lhs, QualType rhs) {
+  TagDecl *ldecl = cast<TagType>(lhs.getCanonicalType())->getDecl();
+  TagDecl *rdecl = cast<TagType>(rhs.getCanonicalType())->getDecl();
+  
+  if (ldecl->getKind() == Decl::Struct && rdecl->getKind() == Decl::Struct) {
+    if (ldecl->getIdentifier() == rdecl->getIdentifier())
+      return true;
+  }
+  return false;
+}
+
+bool Type::unionTypesAreCompatible(QualType lhs, QualType rhs) {
+  TagDecl *ldecl = cast<TagType>(lhs.getCanonicalType())->getDecl();
+  TagDecl *rdecl = cast<TagType>(rhs.getCanonicalType())->getDecl();
+  
+  if (ldecl->getKind() == Decl::Union && rdecl->getKind() == Decl::Union) {
+    if (ldecl->getIdentifier() == rdecl->getIdentifier())
+      return true;
+  }
+  return false;
+}
+
+bool Type::pointerTypesAreCompatible(QualType lhs, QualType rhs) {
+  QualType ltype = cast<PointerType>(lhs.getCanonicalType())->getPointeeType();
+  QualType rtype = cast<PointerType>(rhs.getCanonicalType())->getPointeeType();
+  
+  // handle void first (not sure this is the best place to check for this).
+  if (ltype->isVoidType() || rtype->isVoidType())
+    return true;
+  return typesAreCompatible(ltype, rtype);
+}
+
+bool Type::functionTypesAreCompatible(QualType lhs, QualType rhs) {
+  return true; // FIXME: add more checking
+}
+
+bool Type::arrayTypesAreCompatible(QualType lhs, QualType rhs) {
+  QualType ltype = cast<ArrayType>(lhs.getCanonicalType())->getElementType();
+  QualType rtype = cast<ArrayType>(rhs.getCanonicalType())->getElementType();
+  
+  if (!typesAreCompatible(ltype, rtype))
+    return false;
+    
+  // FIXME: If both types specify constant sizes, then the sizes must also be 
+  // the same. Even if the sizes are the same, GCC produces an error.
+  return true;
+}
+
+bool Type::typesAreCompatible(QualType lcanon, QualType rcanon) {
+  // If two types are identical, they are are compatible
+  if (lcanon == rcanon)
+    return true;
+    
+  if (lcanon->isStructureType() && rcanon->isStructureType())
+    return structureTypesAreCompatible(lcanon, rcanon);
+    
+  if (lcanon->isPointerType() && rcanon->isPointerType())
+    return pointerTypesAreCompatible(lcanon, rcanon);
+
+  if (lcanon->isArrayType() && rcanon->isArrayType())
+    return arrayTypesAreCompatible(lcanon, rcanon);
+    
+  if (lcanon->isFunctionType() && rcanon->isFunctionType())
+    return functionTypesAreCompatible(lcanon, rcanon);
+      
+  return false;
+}
+
 bool Type::isIntegerType() const {
   if (const BuiltinType *BT = dyn_cast<BuiltinType>(CanonicalType))
     return BT->getKind() >= BuiltinType::Bool &&

clang/Sema/SemaExpr.cpp

@@ -601,13 +601,79 @@ Action::ExprResult Sema::CheckLogicalOperands( // C99 6.5.[13,14]
   return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
 }
 
-Action::ExprResult Sema::CheckAssignmentOperands( // C99 6.5.16
+/// CheckAssignmentOperands (C99 6.5.16) - This routine currently 
+/// has code to accommodate several GCC extensions when type checking 
+/// pointers. Here are some objectionable examples that GCC considers warnings:
+///
+///  int a, *pint;
+///  short *pshort;
+///  struct foo *pfoo;
+///
+///  pint = pshort; // warning: assignment from incompatible pointer type
+///  a = pint; // warning: assignment makes integer from pointer without a cast
+///  pint = a; // warning: assignment makes pointer from integer without a cast
+///  pint = pfoo; // warning: assignment from incompatible pointer type
+///
+/// As a result, the code for dealing with pointers is more complex than the
+/// C99 spec dictates. 
+/// Note: the warning above turn into errors when -pedantic-errors is enabled. 
+///
+Action::ExprResult Sema::CheckAssignmentOperands( 
   Expr *lex, Expr *rex, SourceLocation loc, unsigned code) 
 {
   QualType lhsType = lex->getType();
   QualType rhsType = rex->getType();
   
-  // FIXME: add type checking and fix result type
+  if ((BOP)code == BinaryOperator::Assign) { // C99 6.5.16.1
+    if (lhsType.isConstQualified())
+      return Diag(loc, diag::err_typecheck_assign_const);
+      
+    if (lhsType->isArithmeticType() && rhsType->isArithmeticType()) {
+      return new BinaryOperator(lex, rex, (BOP)code, 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)
+          Diag(loc, diag::ext_typecheck_assign_pointer_from_int);
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+      }
+      // FIXME: make sure the qualifier are matching
+      if (rhsType->isPointerType()) { 
+        if (!Type::pointerTypesAreCompatible(lhsType, rhsType))
+          Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+      }
+    } else if (rhsType->isPointerType()) {
+      if (lhsType->isIntegerType()) {
+        Diag(loc, diag::ext_typecheck_assign_int_from_pointer);
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+      }
+      // FIXME: make sure the qualifier are matching
+      if (lhsType->isPointerType()) {
+        if (!Type::pointerTypesAreCompatible(lhsType, rhsType))
+          Diag(loc, diag::ext_typecheck_assign_incompatible_pointer);
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+      }
+    } else if (lhsType->isArrayType() && rhsType->isArrayType()) {
+      ///  int aryInt[3], aryInt2[3];
+      ///  aryInt = aryInt2; // gcc considers this an error (FIXME?)
+      if (Type::arrayTypesAreCompatible(lhsType, rhsType))
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+    } else if (lhsType->isStructureType() && rhsType->isStructureType()) {
+      if (Type::structureTypesAreCompatible(lhsType, rhsType))
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+    } else if (lhsType->isUnionType() && rhsType->isUnionType()) {
+      if (Type::unionTypesAreCompatible(lhsType, rhsType))
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+    } else if (lhsType->isFunctionType() && rhsType->isFunctionType()) {
+      if (Type::functionTypesAreCompatible(lhsType, rhsType))
+        return new BinaryOperator(lex, rex, (BOP)code, lhsType);
+    }
+    return Diag(loc, diag::err_typecheck_assign_incompatible);
+  }
+  
+  // FIXME: type check compound assignments...
   return new BinaryOperator(lex, rex, (BOP)code, Context.IntTy);
 }
 

clang/include/clang/AST/Expr.h

@@ -83,7 +83,8 @@ public:
     : Expr(IntegerLiteralClass, type), Value(value) {
     assert(type->isIntegerType() && "Illegal type in IntegerLiteral");
   }
-
+  intmax_t getValue() const { return Value; }
+  
   virtual void visit(StmtVisitor &Visitor);
   static bool classof(const Stmt *T) { 
     return T->getStmtClass() == IntegerLiteralClass; 

clang/include/clang/AST/Type.h

@@ -233,6 +233,14 @@ public:
   bool isPromotableIntegerType() const; // C99 6.3.1.1p2
   bool isSignedIntegerType() const;     // C99 6.2.5p4
   bool isUnsignedIntegerType() const;   // C99 6.2.5p6
+  
+  /// Compatibility predicates used to check assignment expressions.
+  static bool typesAreCompatible(QualType, QualType); // C99 6.2.7p1
+  static bool structureTypesAreCompatible(QualType, QualType); // C99 6.2.7p1
+  static bool unionTypesAreCompatible(QualType, QualType); // C99 6.2.7p1
+  static bool pointerTypesAreCompatible(QualType, QualType);  // C99 6.7.5.1p2
+  static bool functionTypesAreCompatible(QualType, QualType); // C99 6.7.5.3p15
+  static bool arrayTypesAreCompatible(QualType, QualType); // C99 6.7.5.2p6
 private:
   // this forces clients to use isModifiableLvalue on QualType, the class that 
   // knows if the type is const. This predicate is a helper to QualType. 

clang/include/clang/Basic/DiagnosticKinds.def

@@ -539,6 +539,16 @@ DIAG(err_typecheck_invalid_operands, ERROR,
      "invalid operands to binary expression")
 DIAG(ext_typecheck_comparison_of_pointer_integer, EXTENSION,
      "comparison between pointer and integer")
+DIAG(err_typecheck_assign_const, ERROR,
+     "assignment of read-only variable")
+DIAG(err_typecheck_assign_incompatible, ERROR,
+     "incompatible types in assignment")
+DIAG(ext_typecheck_assign_int_from_pointer, EXTENSION,
+     "assignment makes integer from pointer without a cast")
+DIAG(ext_typecheck_assign_pointer_from_int, EXTENSION,
+     "assignment makes pointer from integer without a cast")
+DIAG(ext_typecheck_assign_incompatible_pointer, EXTENSION,
+     "assignment from incompatible pointer type")
 
 // Statements.
 DIAG(err_continue_not_in_loop, ERROR,