Implement support for comparing pointers with <, >, <=, >=, ==, and !=

in C++, taking into account conversions to the "composite pointer
type" so that we can compare, e.g., a pointer to a derived class to a
pointer to a base class. 

Also, upgrade the "comparing distinct pointer types" from a warning to
an error for C++, since this is clearly an error. Turns out that we
hadn't gone through and audited this code for C++, ever. 

Fixes <rdar://problem/6816420>.

llvm-svn: 70829

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -1039,6 +1039,8 @@ def ext_typecheck_comparison_of_pointer_integer : Warning<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : Warning<
   "comparison of distinct pointer types (%0 and %1)">;
+def err_typecheck_comparison_of_distinct_pointers : Error<
+  "comparison of distinct pointer types (%0 and %1)">;
 def err_typecheck_vector_comparison : Error<
   "comparison of vector types (%0 and %1) not supported yet">;
 def err_typecheck_assign_const : Error<"read-only variable is not assignable">;

clang/lib/Sema/Sema.h

@@ -2424,6 +2424,8 @@ public:
                                                         QualType rhsType);
 
   bool IsStringLiteralToNonConstPointerConversion(Expr *From, QualType ToType);
+  QualType CompositePointerType(Expr *LHS, Expr *RHS,
+                                bool LHSIsNull, bool RHSIsNull);
 
   bool PerformImplicitConversion(Expr *&From, QualType ToType, 
                                  const char *Flavor,

clang/lib/Sema/SemaExpr.cpp

@@ -3653,7 +3653,7 @@ QualType Sema::CheckShiftOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
   return LHSTy;
 }
 
-// C99 6.5.8
+// C99 6.5.8, C++ [expr.rel]
 QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
                                     unsigned OpaqueOpc, bool isRelational) {
   BinaryOperator::Opcode Opc = (BinaryOperator::Opcode)OpaqueOpc;
@@ -3758,6 +3758,31 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
     QualType RCanPointeeTy =
       Context.getCanonicalType(rType->getAsPointerType()->getPointeeType());
 
+    // Simple check: if the pointee types are identical, we're done.
+    if (LCanPointeeTy == RCanPointeeTy)
+      return ResultTy;
+
+    if (getLangOptions().CPlusPlus) {
+      // C++ [expr.rel]p2:
+      //   [...] Pointer conversions (4.10) and qualification
+      //   conversions (4.4) are performed on pointer operands (or on
+      //   a pointer operand and a null pointer constant) to bring
+      //   them to their composite pointer type. [...]
+      //
+      // C++ [expr.eq]p2 uses the same notion for (in)equality
+      // comparisons of pointers.
+      QualType T = CompositePointerType(lex, rex, LHSIsNull, RHSIsNull);
+      if (T.isNull()) {
+        Diag(Loc, diag::err_typecheck_comparison_of_distinct_pointers)
+          << lType << rType << lex->getSourceRange() << rex->getSourceRange();
+        return QualType();
+      }
+
+      ImpCastExprToType(lex, T);
+      ImpCastExprToType(rex, T);
+      return ResultTy;
+    }
+
     if (!LHSIsNull && !RHSIsNull &&                       // C99 6.5.9p2
         !LCanPointeeTy->isVoidType() && !RCanPointeeTy->isVoidType() &&
         !Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),

clang/lib/Sema/SemaExprCXX.cpp

@@ -768,6 +768,77 @@ Sema::IsStringLiteralToNonConstPointerConversion(Expr *From, QualType ToType) {
   return false;
 }
 
+/// \brief Determine the composite pointer type (C++ [expr.rel]p2)
+/// given the left- and right-hand expressions in a relational
+/// operation.
+///
+/// While the notion of a composite pointer type is only described for
+/// relational operators (<, >, <=, >=), the same computation is used
+/// to determine the "common type" used for the equality operators
+/// (==, !=) when comparing pointers.
+///
+/// \param LHS the left-hand operand.
+/// \param RHS the right-hand operand.
+/// \param LHSIsNull whether \p LHS is the NULL pointer constant
+/// \param RHSIsNull whether \p RHS is the NULL pointer constant
+///
+/// \returns the composite pointer type, if any, or the null type if
+/// no such type exists. It is the caller's responsibility to emit
+/// diagnostic.
+QualType Sema::CompositePointerType(Expr *LHS, Expr *RHS,
+                                    bool LHSIsNull, bool RHSIsNull) {
+  // First, determine whether LHS and RHS have pointer types, and what
+  // types they point to.
+  QualType LHSPointee;
+  QualType RHSPointee;
+  if (const PointerType *LHSPtr = LHS->getType()->getAsPointerType())
+    LHSPointee = LHSPtr->getPointeeType();
+  if (const PointerType *RHSPtr = RHS->getType()->getAsPointerType())
+    RHSPointee = RHSPtr->getPointeeType();
+
+  // C++ [expr.rel]p2:
+  //   [...] If one operand is a null pointer constant, the composite
+  //   pointer type is the type of the other operand.
+  if (LHSIsNull && !RHSPointee.isNull())
+    return RHS->getType();
+  if (RHSIsNull && !LHSPointee.isNull())
+    return LHS->getType();
+
+  // If neither LHS nor RHS has pointer type, we're done.
+  if (LHSPointee.isNull() && RHSPointee.isNull())
+    return QualType();
+
+  //  [...] Otherwise, if one of the operands has type "pointer to cv1
+  //  void", then the other has type "pointer to cv2 T" and the
+  //  composite pointer type is "pointer to cv12 void", where cv12 is
+  //  the union of cv1 and cv2.
+  QualType LHSPointeeCanon = Context.getCanonicalType(LHSPointee);
+  QualType RHSPointeeCanon = Context.getCanonicalType(RHSPointee);
+  unsigned CVQuals = 
+    (LHSPointeeCanon.getCVRQualifiers() | RHSPointeeCanon.getCVRQualifiers());
+  if (LHSPointeeCanon->isVoidType() || RHSPointeeCanon->isVoidType())
+    return Context.getPointerType(Context.VoidTy.getQualifiedType(CVQuals));
+
+  // [...] Otherwise, the composite pointer type is a pointer type
+  // similar (4.4) to the type of one of the operands, with a
+  // cv-qualification signature (4.4) that is the union of the
+  // cv-qualification signatures of the operand types.
+  QualType FullyQualifiedLHSType
+    = Context.getPointerType(LHSPointee.getQualifiedType(CVQuals));
+  QualType CompositePointerType;
+  bool IncompatibleObjC = false;
+  if (IsPointerConversion(RHS, RHS->getType(), FullyQualifiedLHSType,
+                          CompositePointerType, IncompatibleObjC))
+    return CompositePointerType;
+  QualType FullyQualifiedRHSType
+    = Context.getPointerType(RHSPointee.getQualifiedType(CVQuals));
+  if (IsPointerConversion(LHS, LHS->getType(), FullyQualifiedRHSType,
+                          CompositePointerType, IncompatibleObjC))
+    return CompositePointerType;
+  
+  return QualType();
+}
+
 /// PerformImplicitConversion - Perform an implicit conversion of the
 /// expression From to the type ToType. Returns true if there was an
 /// error, false otherwise. The expression From is replaced with the

clang/test/SemaCXX/composite-pointer-type.cpp

@@ -0,0 +1,27 @@
+// RUN: clang-cc -fsyntax-only -verify %s
+
+class Base { };
+class Derived1 : public Base { };
+class Derived2 : public Base { };
+
+void f0(volatile Base *b, Derived1 *d1, const Derived2 *d2) {
+  if (b > d1)
+    return;
+  if (d1 <= b)
+    return;
+  if (b > d2)
+    return;
+  if (d1 >= d2) // expected-error{{comparison of distinct}}
+    return;
+}
+
+void f1(volatile Base *b, Derived1 *d1, const Derived2 *d2) {
+  if (b == d1)
+    return;
+  if (d1 == b)
+    return;
+  if (b != d2)
+    return;
+  if (d1 == d2) // expected-error{{comparison of distinct}}
+    return;
+}

clang/test/SemaCXX/elaborated-type-specifier.cpp

@@ -38,10 +38,9 @@ void test_S5_scope() {
   S4 *s4; // expected-error{{use of undeclared identifier 'S4'}}
 }
 
-// FIXME: the warning below should be an error!
 int test_funcparam_scope(struct S5 * s5) {
   struct S5 { int y; } *s5_2 = 0;
-  if (s5 == s5_2) return 1; // expected-warning {{comparison of distinct pointer types ('struct S5 *' and 'struct S5 *')}}
+  if (s5 == s5_2) return 1; // expected-error {{comparison of distinct pointer types ('struct S5 *' and 'struct S5 *')}}
   return 0;
 }