Revert "DR583, DR1512: Implement a rewrite to C++'s 'composite pointer type' rules."

This reverts commit r284800, as it failed all ARM/AArch64 bots.

llvm-svn: 284811

3 files changed, 131 insertions, 134 deletions

diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index eddc4207dfc..15564918b37 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -8929,21 +8929,35 @@ static bool convertPointersToCompositeType(Sema &S, SourceLocation Loc,
   // C++ [expr.eq]p1 uses the same notion for (in)equality
   // comparisons of pointers.
 
+  // C++ [expr.eq]p2:
+  //   In addition, pointers to members can be compared, or a pointer to
+  //   member and a null pointer constant. Pointer to member conversions
+  //   (4.11) and qualification conversions (4.4) are performed to bring
+  //   them to a common type. If one operand is a null pointer constant,
+  //   the common type is the type of the other operand. Otherwise, the
+  //   common type is a pointer to member 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 LHSType = LHS.get()->getType();
   QualType RHSType = RHS.get()->getType();
-  assert(LHSType->isPointerType() || RHSType->isPointerType() ||
-         LHSType->isMemberPointerType() || RHSType->isMemberPointerType());
+  assert((LHSType->isPointerType() && RHSType->isPointerType()) ||
+         (LHSType->isMemberPointerType() && RHSType->isMemberPointerType()));
 
-  QualType T = S.FindCompositePointerType(Loc, LHS, RHS);
+  bool NonStandardCompositeType = false;
+  bool *BoolPtr = S.isSFINAEContext() ? nullptr : &NonStandardCompositeType;
+  QualType T = S.FindCompositePointerType(Loc, LHS, RHS, BoolPtr);
   if (T.isNull()) {
-    if ((LHSType->isPointerType() || LHSType->isMemberPointerType()) &&
-        (RHSType->isPointerType() || RHSType->isMemberPointerType()))
-      diagnoseDistinctPointerComparison(S, Loc, LHS, RHS, /*isError*/true);
-    else
-      S.InvalidOperands(Loc, LHS, RHS);
+    diagnoseDistinctPointerComparison(S, Loc, LHS, RHS, /*isError*/true);
     return true;
   }
 
+  if (NonStandardCompositeType)
+    S.Diag(Loc, diag::ext_typecheck_comparison_of_distinct_pointers_nonstandard)
+      << LHSType << RHSType << T << LHS.get()->getSourceRange()
+      << RHS.get()->getSourceRange();
+
   LHS = S.ImpCastExprToType(LHS.get(), T, CK_BitCast);
   RHS = S.ImpCastExprToType(RHS.get(), T, CK_BitCast);
   return false;
@@ -9300,53 +9314,41 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
                                    LHS.get()->getSourceRange());
   }
 
-  if ((LHSType->isIntegerType() && !LHSIsNull) ||
-      (RHSType->isIntegerType() && !RHSIsNull)) {
-    // Skip normal pointer conversion checks in this case; we have better
-    // diagnostics for this below.
-  } else if (getLangOpts().CPlusPlus) {
-    // Equality comparison of a function pointer to a void pointer is invalid,
-    // but we allow it as an extension.
-    // FIXME: If we really want to allow this, should it be part of composite
-    // pointer type computation so it works in conditionals too?
-    if (!IsRelational &&
-        ((LHSType->isFunctionPointerType() && RHSType->isVoidPointerType()) ||
-         (RHSType->isFunctionPointerType() && LHSType->isVoidPointerType()))) {
-      // This is a gcc extension compatibility comparison.
-      // In a SFINAE context, we treat this as a hard error to maintain
-      // conformance with the C++ standard.
-      diagnoseFunctionPointerToVoidComparison(
-          *this, Loc, LHS, RHS, /*isError*/ (bool)isSFINAEContext());
-      
-      if (isSFINAEContext())
-        return QualType();
-      
-      RHS = ImpCastExprToType(RHS.get(), LHSType, CK_BitCast);
-      return ResultTy;
-    }
+  // All of the following pointer-related warnings are GCC extensions, except
+  // when handling null pointer constants. 
+  if (LHSType->isPointerType() && RHSType->isPointerType()) { // C99 6.5.8p2
+    QualType LCanPointeeTy =
+      LHSType->castAs<PointerType>()->getPointeeType().getCanonicalType();
+    QualType RCanPointeeTy =
+      RHSType->castAs<PointerType>()->getPointeeType().getCanonicalType();
+
+    if (getLangOpts().CPlusPlus) {
+      if (LCanPointeeTy == RCanPointeeTy)
+        return ResultTy;
+      if (!IsRelational &&
+          (LCanPointeeTy->isVoidType() || RCanPointeeTy->isVoidType())) {
+        // Valid unless comparison between non-null pointer and function pointer
+        // This is a gcc extension compatibility comparison.
+        // In a SFINAE context, we treat this as a hard error to maintain
+        // conformance with the C++ standard.
+        if ((LCanPointeeTy->isFunctionType() || RCanPointeeTy->isFunctionType())
+            && !LHSIsNull && !RHSIsNull) {
+          diagnoseFunctionPointerToVoidComparison(
+              *this, Loc, LHS, RHS, /*isError*/ (bool)isSFINAEContext());
+          
+          if (isSFINAEContext())
+            return QualType();
+          
+          RHS = ImpCastExprToType(RHS.get(), LHSType, CK_BitCast);
+          return ResultTy;
+        }
+      }
 
-    // C++ [expr.eq]p2:
-    //   If at least one operand is a pointer [...] bring them to their
-    //   composite pointer type.
-    // C++ [expr.rel]p2:
-    //   If both operands are pointers, [...] bring them to their composite
-    //   pointer type.
-    if ((int)LHSType->isPointerType() + (int)RHSType->isPointerType() >=
-        (IsRelational ? 2 : 1)) {
       if (convertPointersToCompositeType(*this, Loc, LHS, RHS))
         return QualType();
       else
         return ResultTy;
     }
-  } else if (LHSType->isPointerType() &&
-             RHSType->isPointerType()) { // C99 6.5.8p2
-    // All of the following pointer-related warnings are GCC extensions, except
-    // when handling null pointer constants.
-    QualType LCanPointeeTy =
-      LHSType->castAs<PointerType>()->getPointeeType().getCanonicalType();
-    QualType RCanPointeeTy =
-      RHSType->castAs<PointerType>()->getPointeeType().getCanonicalType();
-
     // C99 6.5.9p2 and C99 6.5.8p2
     if (Context.typesAreCompatible(LCanPointeeTy.getUnqualifiedType(),
                                    RCanPointeeTy.getUnqualifiedType())) {
@@ -9391,63 +9393,36 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
   }
 
   if (getLangOpts().CPlusPlus) {
-    // C++ [expr.eq]p4:
-    //   Two operands of type std::nullptr_t or one operand of type
-    //   std::nullptr_t and the other a null pointer constant compare equal.
-    if (!IsRelational && LHSIsNull && RHSIsNull) {
-      if (LHSType->isNullPtrType()) {
-        RHS = ImpCastExprToType(RHS.get(), LHSType, CK_NullToPointer);
-        return ResultTy;
-      }
-      if (RHSType->isNullPtrType()) {
-        LHS = ImpCastExprToType(LHS.get(), RHSType, CK_NullToPointer);
-        return ResultTy;
-      }
-    }
-
-    // Comparison of Objective-C pointers and block pointers against nullptr_t.
-    // These aren't covered by the composite pointer type rules.
-    if (!IsRelational && RHSType->isNullPtrType() &&
-        (LHSType->isObjCObjectPointerType() || LHSType->isBlockPointerType())) {
-      RHS = ImpCastExprToType(RHS.get(), LHSType, CK_NullToPointer);
+    // Comparison of nullptr_t with itself.
+    if (LHSType->isNullPtrType() && RHSType->isNullPtrType())
       return ResultTy;
-    }
-    if (!IsRelational && LHSType->isNullPtrType() &&
-        (RHSType->isObjCObjectPointerType() || RHSType->isBlockPointerType())) {
-      LHS = ImpCastExprToType(LHS.get(), RHSType, CK_NullToPointer);
+    
+    // Comparison of pointers with null pointer constants and equality
+    // comparisons of member pointers to null pointer constants.
+    if (RHSIsNull &&
+        ((LHSType->isAnyPointerType() || LHSType->isNullPtrType()) ||
+         (!IsRelational && 
+          (LHSType->isMemberPointerType() || LHSType->isBlockPointerType())))) {
+      RHS = ImpCastExprToType(RHS.get(), LHSType, 
+                        LHSType->isMemberPointerType()
+                          ? CK_NullToMemberPointer
+                          : CK_NullToPointer);
       return ResultTy;
     }
-
-    if (IsRelational &&
-        ((LHSType->isNullPtrType() && RHSType->isPointerType()) ||
-         (RHSType->isNullPtrType() && LHSType->isPointerType()))) {
-      // HACK: Relational comparison of nullptr_t against a pointer type is
-      // invalid per DR583, but we allow it within std::less<> and friends,
-      // since otherwise common uses of it break.
-      // FIXME: Consider removing this hack once LWG fixes std::less<> and
-      // friends to have std::nullptr_t overload candidates.
-      DeclContext *DC = CurContext;
-      if (isa<FunctionDecl>(DC))
-        DC = DC->getParent();
-      if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(DC)) {
-        if (CTSD->isInStdNamespace() &&
-            llvm::StringSwitch<bool>(CTSD->getName())
-                .Cases("less", "less_equal", "greater", "greater_equal", true)
-                .Default(false)) {
-          if (RHSType->isNullPtrType())
-            RHS = ImpCastExprToType(RHS.get(), LHSType, CK_NullToPointer);
-          else
-            LHS = ImpCastExprToType(LHS.get(), RHSType, CK_NullToPointer);
-          return ResultTy;
-        }
-      }
+    if (LHSIsNull &&
+        ((RHSType->isAnyPointerType() || RHSType->isNullPtrType()) ||
+         (!IsRelational && 
+          (RHSType->isMemberPointerType() || RHSType->isBlockPointerType())))) {
+      LHS = ImpCastExprToType(LHS.get(), RHSType, 
+                        RHSType->isMemberPointerType()
+                          ? CK_NullToMemberPointer
+                          : CK_NullToPointer);
+      return ResultTy;
     }
 
-    // C++ [expr.eq]p2:
-    //   If at least one operand is a pointer to member, [...] bring them to
-    //   their composite pointer type.
+    // Comparison of member pointers.
     if (!IsRelational &&
-        (LHSType->isMemberPointerType() || RHSType->isMemberPointerType())) {
+        LHSType->isMemberPointerType() && RHSType->isMemberPointerType()) {
       if (convertPointersToCompositeType(*this, Loc, LHS, RHS))
         return QualType();
       else
@@ -9556,19 +9531,15 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
       // Under a debugger, allow the comparison of pointers to integers,
       // since users tend to want to compare addresses.
     } else if ((LHSIsNull && LHSType->isIntegerType()) ||
-               (RHSIsNull && RHSType->isIntegerType())) {
-      if (IsRelational) {
-        isError = getLangOpts().CPlusPlus;
-        DiagID =
-          isError ? diag::err_typecheck_ordered_comparison_of_pointer_and_zero
-                  : diag::ext_typecheck_ordered_comparison_of_pointer_and_zero;
-      }
-    } else if (getLangOpts().CPlusPlus) {
+        (RHSIsNull && RHSType->isIntegerType())) {
+      if (IsRelational && !getLangOpts().CPlusPlus)
+        DiagID = diag::ext_typecheck_ordered_comparison_of_pointer_and_zero;
+    } else if (IsRelational && !getLangOpts().CPlusPlus)
+      DiagID = diag::ext_typecheck_ordered_comparison_of_pointer_integer;
+    else if (getLangOpts().CPlusPlus) {
       DiagID = diag::err_typecheck_comparison_of_pointer_integer;
       isError = true;
-    } else if (IsRelational)
-      DiagID = diag::ext_typecheck_ordered_comparison_of_pointer_integer;
-    else
+    } else
       DiagID = diag::ext_typecheck_comparison_of_pointer_integer;
 
     if (DiagID) {
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 66705f88f9e..83474cb7f83 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -5399,7 +5399,7 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
     // exception specifications, if any.
     if (LTy->isFunctionPointerType() || LTy->isMemberFunctionPointerType()) {
       Qualifiers Qs = LTy.getQualifiers();
-      LTy = FindCompositePointerType(QuestionLoc, LHS, RHS,
+      LTy = FindCompositePointerType(QuestionLoc, LHS, RHS, nullptr,
                                      /*ConvertArgs*/false);
       LTy = Context.getQualifiedType(LTy, Qs);
 
@@ -5511,9 +5511,19 @@ QualType Sema::CXXCheckConditionalOperands(ExprResult &Cond, ExprResult &LHS,
   //      performed to bring them to a common type, whose cv-qualification
   //      shall match the cv-qualification of either the second or the third
   //      operand. The result is of the common type.
-  QualType Composite = FindCompositePointerType(QuestionLoc, LHS, RHS);
-  if (!Composite.isNull())
+  bool NonStandardCompositeType = false;
+  QualType Composite = FindCompositePointerType(QuestionLoc, LHS, RHS,
+                                 isSFINAEContext() ? nullptr
+                                                   : &NonStandardCompositeType);
+  if (!Composite.isNull()) {
+    if (NonStandardCompositeType)
+      Diag(QuestionLoc,
+           diag::ext_typecheck_cond_incompatible_operands_nonstandard)
+        << LTy << RTy << Composite
+        << LHS.get()->getSourceRange() << RHS.get()->getSourceRange();
+
     return Composite;
+  }
 
   // Similarly, attempt to find composite type of two objective-c pointers.
   Composite = FindCompositeObjCPointerType(LHS, RHS, QuestionLoc);
@@ -5612,10 +5622,19 @@ mergeExceptionSpecs(Sema &S, FunctionProtoType::ExceptionSpecInfo ESI1,
 /// \param Loc The location of the operator requiring these two expressions to
 /// be converted to the composite pointer type.
 ///
+/// If \p NonStandardCompositeType is non-NULL, then we are permitted to find
+/// a non-standard (but still sane) composite type to which both expressions
+/// can be converted. When such a type is chosen, \c *NonStandardCompositeType
+/// will be set true.
+///
 /// \param ConvertArgs If \c false, do not convert E1 and E2 to the target type.
 QualType Sema::FindCompositePointerType(SourceLocation Loc,
                                         Expr *&E1, Expr *&E2,
+                                        bool *NonStandardCompositeType,
                                         bool ConvertArgs) {
+  if (NonStandardCompositeType)
+    *NonStandardCompositeType = false;
+
   assert(getLangOpts().CPlusPlus && "This function assumes C++");
 
   // C++1z [expr]p14:
@@ -5708,7 +5727,8 @@ QualType Sema::FindCompositePointerType(SourceLocation Loc,
 
       // If we're allowed to create a non-standard composite type, keep track
       // of where we need to fill in additional 'const' qualifiers.
-      if (Composite1.getCVRQualifiers() != Composite2.getCVRQualifiers())
+      if (NonStandardCompositeType &&
+          Composite1.getCVRQualifiers() != Composite2.getCVRQualifiers())
         NeedConstBefore = QualifierUnion.size();
 
       QualifierUnion.push_back(
@@ -5725,7 +5745,8 @@ QualType Sema::FindCompositePointerType(SourceLocation Loc,
 
       // If we're allowed to create a non-standard composite type, keep track
       // of where we need to fill in additional 'const' qualifiers.
-      if (Composite1.getCVRQualifiers() != Composite2.getCVRQualifiers())
+      if (NonStandardCompositeType &&
+          Composite1.getCVRQualifiers() != Composite2.getCVRQualifiers())
         NeedConstBefore = QualifierUnion.size();
 
       QualifierUnion.push_back(
@@ -5776,13 +5797,16 @@ QualType Sema::FindCompositePointerType(SourceLocation Loc,
     }
   }
 
-  if (NeedConstBefore) {
+  if (NeedConstBefore && NonStandardCompositeType) {
     // Extension: Add 'const' to qualifiers that come before the first qualifier
     // mismatch, so that our (non-standard!) composite type meets the
     // requirements of C++ [conv.qual]p4 bullet 3.
-    for (unsigned I = 0; I != NeedConstBefore; ++I)
-      if ((QualifierUnion[I] & Qualifiers::Const) == 0)
+    for (unsigned I = 0; I != NeedConstBefore; ++I) {
+      if ((QualifierUnion[I] & Qualifiers::Const) == 0) {
         QualifierUnion[I] = QualifierUnion[I] | Qualifiers::Const;
+        *NonStandardCompositeType = true;
+      }
+    }
   }
 
   // Rewrap the composites as pointers or member pointers with the union CVRs.
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 8d6c562dd0c..8cc592e21e5 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -7624,12 +7624,12 @@ public:
   }
 
   // C++ [over.match.oper]p16:
-  //   For every pointer to member type T or type std::nullptr_t, there
-  //   exist candidate operator functions of the form
+  //   For every pointer to member type T, there exist candidate operator
+  //   functions of the form
   //
   //        bool operator==(T,T);
   //        bool operator!=(T,T);
-  void addEqualEqualOrNotEqualMemberPointerOrNullptrOverloads() {
+  void addEqualEqualOrNotEqualMemberPointerOverloads() {
     /// Set of (canonical) types that we've already handled.
     llvm::SmallPtrSet<QualType, 8> AddedTypes;
 
@@ -7646,22 +7646,13 @@ public:
         QualType ParamTypes[2] = { *MemPtr, *MemPtr };
         S.AddBuiltinCandidate(S.Context.BoolTy, ParamTypes, Args, CandidateSet);
       }
-
-      if (CandidateTypes[ArgIdx].hasNullPtrType()) {
-        CanQualType NullPtrTy = S.Context.getCanonicalType(S.Context.NullPtrTy);
-        if (AddedTypes.insert(NullPtrTy).second) {
-          QualType ParamTypes[2] = { NullPtrTy, NullPtrTy };
-          S.AddBuiltinCandidate(S.Context.BoolTy, ParamTypes, Args,
-                                CandidateSet);
-        }
-      }
     }
   }
 
   // C++ [over.built]p15:
   //
-  //   For every T, where T is an enumeration type or a pointer type,
-  //   there exist candidate operator functions of the form
+  //   For every T, where T is an enumeration type, a pointer type, or 
+  //   std::nullptr_t, there exist candidate operator functions of the form
   //
   //        bool       operator<(T, T);
   //        bool       operator>(T, T);
@@ -7746,6 +7737,17 @@ public:
         QualType ParamTypes[2] = { *Enum, *Enum };
         S.AddBuiltinCandidate(S.Context.BoolTy, ParamTypes, Args, CandidateSet);
       }
+      
+      if (CandidateTypes[ArgIdx].hasNullPtrType()) {
+        CanQualType NullPtrTy = S.Context.getCanonicalType(S.Context.NullPtrTy);
+        if (AddedTypes.insert(NullPtrTy).second &&
+            !UserDefinedBinaryOperators.count(std::make_pair(NullPtrTy,
+                                                             NullPtrTy))) {
+          QualType ParamTypes[2] = { NullPtrTy, NullPtrTy };
+          S.AddBuiltinCandidate(S.Context.BoolTy, ParamTypes, Args,
+                                CandidateSet);
+        }
+      }
     }
   }
 
@@ -8441,7 +8443,7 @@ void Sema::AddBuiltinOperatorCandidates(OverloadedOperatorKind Op,
 
   case OO_EqualEqual:
   case OO_ExclaimEqual:
-    OpBuilder.addEqualEqualOrNotEqualMemberPointerOrNullptrOverloads();
+    OpBuilder.addEqualEqualOrNotEqualMemberPointerOverloads();
     // Fall through.
 
   case OO_Less: