Clean up and slightly generalize implementation of composite pointer
authorRichard Smith <richard@metafoo.co.uk>
Fri, 10 Jan 2020 03:24:44 +0000 (19:24 -0800)
committerRichard Smith <richard@metafoo.co.uk>
Sat, 11 Jan 2020 00:12:00 +0000 (16:12 -0800)
type computation, in preparation for P0388R4, which adds another few
cases here.

We now properly handle forming multi-level composite pointer types
involving nested Objective-C pointer types (as is consistent with
including them as part of the notion of 'similar types' on which this
rule is based). We no longer lose non-CVR qualifiers on nested pointer
types.

clang/lib/Sema/SemaExprCXX.cpp
clang/test/SemaObjCXX/composite-objc-pointertype.mm
clang/test/SemaOpenCLCXX/address-space-cond.cl [new file with mode: 0644]

index 17f2028..e9d075c 100644 (file)
@@ -6120,10 +6120,10 @@ mergeExceptionSpecs(Sema &S, FunctionProtoType::ExceptionSpecInfo ESI1,
 
 /// Find a merged pointer type and convert the two expressions to it.
 ///
-/// This finds the composite pointer type (or member pointer type) for @p E1
-/// and @p E2 according to C++1z 5p14. It converts both expressions to this
-/// type and returns it.
-/// It does not emit diagnostics.
+/// This finds the composite pointer type for \p E1 and \p E2 according to
+/// C++2a [expr.type]p3. It converts both expressions to this type and returns
+/// it.  It does not emit diagnostics (FIXME: that's not true if \p ConvertArgs
+/// is \c true).
 ///
 /// \param Loc The location of the operator requiring these two expressions to
 /// be converted to the composite pointer type.
@@ -6176,60 +6176,117 @@ QualType Sema::FindCompositePointerType(SourceLocation Loc,
   assert(!T1->isNullPtrType() && !T2->isNullPtrType() &&
          "nullptr_t should be a null pointer constant");
 
-  //  - if T1 or T2 is "pointer to cv1 void" and the other type is
-  //    "pointer to cv2 T", "pointer to cv12 void", where cv12 is
-  //    the union of cv1 and cv2;
-  //  - if T1 or T2 is "pointer to noexcept function" and the other type is
-  //    "pointer to function", where the function types are otherwise the same,
-  //    "pointer to function";
-  //     FIXME: This rule is defective: it should also permit removing noexcept
-  //     from a pointer to member function.  As a Clang extension, we also
-  //     permit removing 'noreturn', so we generalize this rule to;
-  //     - [Clang] If T1 and T2 are both of type "pointer to function" or
-  //       "pointer to member function" and the pointee types can be unified
-  //       by a function pointer conversion, that conversion is applied
-  //       before checking the following rules.
+  struct Step {
+    enum Kind { Pointer, ObjCPointer, MemberPointer, Array } K;
+    // Qualifiers to apply under the step kind.
+    Qualifiers Quals;
+    /// The class for a pointer-to-member; a constant array type with a bound
+    /// (if any) for an array.
+    const Type *ClassOrBound;
+
+    Step(Kind K, const Type *ClassOrBound = nullptr)
+        : K(K), Quals(), ClassOrBound(ClassOrBound) {}
+    QualType rebuild(ASTContext &Ctx, QualType T) const {
+      T = Ctx.getQualifiedType(T, Quals);
+      switch (K) {
+      case Pointer:
+        return Ctx.getPointerType(T);
+      case MemberPointer:
+        return Ctx.getMemberPointerType(T, ClassOrBound);
+      case ObjCPointer:
+        return Ctx.getObjCObjectPointerType(T);
+      case Array:
+        if (auto *CAT = cast_or_null<ConstantArrayType>(ClassOrBound))
+          return Ctx.getConstantArrayType(T, CAT->getSize(), nullptr,
+                                          ArrayType::Normal, 0);
+        else
+          return Ctx.getIncompleteArrayType(T, ArrayType::Normal, 0);
+      }
+      llvm_unreachable("unknown step kind");
+    }
+  };
+
+  SmallVector<Step, 8> Steps;
+
   //  - if T1 is "pointer to cv1 C1" and T2 is "pointer to cv2 C2", where C1
   //    is reference-related to C2 or C2 is reference-related to C1 (8.6.3),
   //    the cv-combined type of T1 and T2 or the cv-combined type of T2 and T1,
   //    respectively;
   //  - if T1 is "pointer to member of C1 of type cv1 U1" and T2 is "pointer
-  //    to member of C2 of type cv2 U2" where C1 is reference-related to C2 or
-  //    C2 is reference-related to C1 (8.6.3), the cv-combined type of T2 and
-  //    T1 or the cv-combined type of T1 and T2, respectively;
+  //    to member of C2 of type cv2 U2" for some non-function type U, where
+  //    C1 is reference-related to C2 or C2 is reference-related to C1, the
+  //    cv-combined type of T2 and T1 or the cv-combined type of T1 and T2,
+  //    respectively;
   //  - if T1 and T2 are similar types (4.5), the cv-combined type of T1 and
   //    T2;
   //
-  // If looked at in the right way, these bullets all do the same thing.
-  // What we do here is, we build the two possible cv-combined types, and try
-  // the conversions in both directions. If only one works, or if the two
-  // composite types are the same, we have succeeded.
-  // FIXME: extended qualifiers?
-  //
-  // Note that this will fail to find a composite pointer type for "pointer
-  // to void" and "pointer to function". We can't actually perform the final
-  // conversion in this case, even though a composite pointer type formally
-  // exists.
-  SmallVector<unsigned, 4> QualifierUnion;
-  SmallVector<std::pair<const Type *, const Type *>, 4> MemberOfClass;
+  // Dismantle T1 and T2 to simultaneously determine whether they are similar
+  // and to prepare to form the cv-combined type if so.
   QualType Composite1 = T1;
   QualType Composite2 = T2;
   unsigned NeedConstBefore = 0;
   while (true) {
+    assert(!Composite1.isNull() && !Composite2.isNull());
+
+    Qualifiers Q1, Q2;
+    Composite1 = Context.getUnqualifiedArrayType(Composite1, Q1);
+    Composite2 = Context.getUnqualifiedArrayType(Composite2, Q2);
+
+    // Top-level qualifiers are ignored. Merge at all lower levels.
+    if (!Steps.empty()) {
+      // Find the qualifier union: (approximately) the unique minimal set of
+      // qualifiers that is compatible with both types.
+      Qualifiers Quals = Qualifiers::fromCVRUMask(Q1.getCVRUQualifiers() |
+                                                  Q2.getCVRUQualifiers());
+
+      // Under one level of pointer or pointer-to-member, we can change to an
+      // unambiguous compatible address space.
+      if (Q1.getAddressSpace() == Q2.getAddressSpace()) {
+        Quals.setAddressSpace(Q1.getAddressSpace());
+      } else if (Steps.size() == 1) {
+        bool MaybeQ1 = Q1.isAddressSpaceSupersetOf(Q2);
+        bool MaybeQ2 = Q2.isAddressSpaceSupersetOf(Q1);
+        if (MaybeQ1 == MaybeQ2)
+          return QualType(); // No unique best address space.
+        Quals.setAddressSpace(MaybeQ1 ? Q1.getAddressSpace()
+                                      : Q2.getAddressSpace());
+      } else {
+        return QualType();
+      }
+
+      // FIXME: In C, we merge __strong and none to __strong at the top level.
+      if (Q1.getObjCGCAttr() == Q2.getObjCGCAttr())
+        Quals.setObjCGCAttr(Q1.getObjCGCAttr());
+      else
+        return QualType();
+
+      // Mismatched lifetime qualifiers never compatibly include each other.
+      if (Q1.getObjCLifetime() == Q2.getObjCLifetime())
+        Quals.setObjCLifetime(Q1.getObjCLifetime());
+      else
+        return QualType();
+
+      Steps.back().Quals = Quals;
+      if (Q1 != Quals || Q2 != Quals)
+        NeedConstBefore = Steps.size() - 1;
+    }
+
+    // FIXME: Can we unify the following with UnwrapSimilarTypes?
     const PointerType *Ptr1, *Ptr2;
     if ((Ptr1 = Composite1->getAs<PointerType>()) &&
         (Ptr2 = Composite2->getAs<PointerType>())) {
       Composite1 = Ptr1->getPointeeType();
       Composite2 = Ptr2->getPointeeType();
+      Steps.emplace_back(Step::Pointer);
+      continue;
+    }
 
-      // 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())
-        NeedConstBefore = QualifierUnion.size();
-
-      QualifierUnion.push_back(
-                 Composite1.getCVRQualifiers() | Composite2.getCVRQualifiers());
-      MemberOfClass.push_back(std::make_pair(nullptr, nullptr));
+    const ObjCObjectPointerType *ObjPtr1, *ObjPtr2;
+    if ((ObjPtr1 = Composite1->getAs<ObjCObjectPointerType>()) &&
+        (ObjPtr2 = Composite2->getAs<ObjCObjectPointerType>())) {
+      Composite1 = ObjPtr1->getPointeeType();
+      Composite2 = ObjPtr2->getPointeeType();
+      Steps.emplace_back(Step::ObjCPointer);
       continue;
     }
 
@@ -6239,34 +6296,79 @@ QualType Sema::FindCompositePointerType(SourceLocation Loc,
       Composite1 = MemPtr1->getPointeeType();
       Composite2 = MemPtr2->getPointeeType();
 
-      // 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())
-        NeedConstBefore = QualifierUnion.size();
+      // At the top level, we can perform a base-to-derived pointer-to-member
+      // conversion:
+      //
+      //  - [...] where C1 is reference-related to C2 or C2 is
+      //    reference-related to C1
+      //
+      // (Note that the only kinds of reference-relatedness in scope here are
+      // "same type or derived from".) At any other level, the class must
+      // exactly match.
+      const Type *Class = nullptr;
+      QualType Cls1(MemPtr1->getClass(), 0);
+      QualType Cls2(MemPtr2->getClass(), 0);
+      if (Context.hasSameType(Cls1, Cls2))
+        Class = MemPtr1->getClass();
+      else if (Steps.empty())
+        Class = IsDerivedFrom(Loc, Cls1, Cls2) ? MemPtr1->getClass() :
+                IsDerivedFrom(Loc, Cls2, Cls1) ? MemPtr2->getClass() : nullptr;
+      if (!Class)
+        return QualType();
+
+      Steps.emplace_back(Step::MemberPointer, Class);
+      continue;
+    }
 
-      QualifierUnion.push_back(
-                 Composite1.getCVRQualifiers() | Composite2.getCVRQualifiers());
-      MemberOfClass.push_back(std::make_pair(MemPtr1->getClass(),
-                                             MemPtr2->getClass()));
+    // Special case: at the top level, we can decompose an Objective-C pointer
+    // and a 'cv void *'. Unify the qualifiers.
+    if (Steps.empty() && ((Composite1->isVoidPointerType() &&
+                           Composite2->isObjCObjectPointerType()) ||
+                          (Composite1->isObjCObjectPointerType() &&
+                           Composite2->isVoidPointerType()))) {
+      Composite1 = Composite1->getPointeeType();
+      Composite2 = Composite2->getPointeeType();
+      Steps.emplace_back(Step::Pointer);
       continue;
     }
 
+    // FIXME: arrays
+
     // FIXME: block pointer types?
 
     // Cannot unwrap any more types.
     break;
   }
 
-  // Apply the function pointer conversion to unify the types. We've already
-  // unwrapped down to the function types, and we want to merge rather than
-  // just convert, so do this ourselves rather than calling
+  //  - if T1 or T2 is "pointer to noexcept function" and the other type is
+  //    "pointer to function", where the function types are otherwise the same,
+  //    "pointer to function";
+  //  - if T1 or T2 is "pointer to member of C1 of type function", the other
+  //    type is "pointer to member of C2 of type noexcept function", and C1
+  //    is reference-related to C2 or C2 is reference-related to C1, where
+  //    the function types are otherwise the same, "pointer to member of C2 of
+  //    type function" or "pointer to member of C1 of type function",
+  //    respectively;
+  //
+  // We also support 'noreturn' here, so as a Clang extension we generalize the
+  // above to:
+  //
+  //  - [Clang] If T1 and T2 are both of type "pointer to function" or
+  //    "pointer to member function" and the pointee types can be unified
+  //    by a function pointer conversion, that conversion is applied
+  //    before checking the following rules.
+  //
+  // We've already unwrapped down to the function types, and we want to merge
+  // rather than just convert, so do this ourselves rather than calling
   // IsFunctionConversion.
   //
   // FIXME: In order to match the standard wording as closely as possible, we
   // currently only do this under a single level of pointers. Ideally, we would
   // allow this in general, and set NeedConstBefore to the relevant depth on
-  // the side(s) where we changed anything.
-  if (QualifierUnion.size() == 1) {
+  // the side(s) where we changed anything. If we permit that, we should also
+  // consider this conversion when determining type similarity and model it as
+  // a qualification conversion.
+  if (Steps.size() == 1) {
     if (auto *FPT1 = Composite1->getAs<FunctionProtoType>()) {
       if (auto *FPT2 = Composite2->getAs<FunctionProtoType>()) {
         FunctionProtoType::ExtProtoInfo EPI1 = FPT1->getExtProtoInfo();
@@ -6292,88 +6394,72 @@ QualType Sema::FindCompositePointerType(SourceLocation Loc,
     }
   }
 
-  if (NeedConstBefore) {
-    // 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)
-        QualifierUnion[I] = QualifierUnion[I] | Qualifiers::Const;
+  // There are some more conversions we can perform under exactly one pointer.
+  if (Steps.size() == 1 && Steps.front().K == Step::Pointer &&
+      !Context.hasSameType(Composite1, Composite2)) {
+    //  - if T1 or T2 is "pointer to cv1 void" and the other type is
+    //    "pointer to cv2 T", where T is an object type or void,
+    //    "pointer to cv12 void", where cv12 is the union of cv1 and cv2;
+    if (Composite1->isVoidType() && Composite2->isObjectType())
+      Composite2 = Composite1;
+    else if (Composite2->isVoidType() && Composite1->isObjectType())
+      Composite1 = Composite2;
+    //  - if T1 is "pointer to cv1 C1" and T2 is "pointer to cv2 C2", where C1
+    //    is reference-related to C2 or C2 is reference-related to C1 (8.6.3),
+    //    the cv-combined type of T1 and T2 or the cv-combined type of T2 and
+    //    T1, respectively;
+    //
+    // The "similar type" handling covers all of this except for the "T1 is a
+    // base class of T2" case in the definition of reference-related.
+    else if (IsDerivedFrom(Loc, Composite1, Composite2))
+      Composite1 = Composite2;
+    else if (IsDerivedFrom(Loc, Composite2, Composite1))
+      Composite2 = Composite1;
   }
 
-  // Rewrap the composites as pointers or member pointers with the union CVRs.
-  auto MOC = MemberOfClass.rbegin();
-  for (unsigned CVR : llvm::reverse(QualifierUnion)) {
-    Qualifiers Quals = Qualifiers::fromCVRMask(CVR);
-    auto Classes = *MOC++;
-    if (Classes.first && Classes.second) {
-      // Rebuild member pointer type
-      Composite1 = Context.getMemberPointerType(
-          Context.getQualifiedType(Composite1, Quals), Classes.first);
-      Composite2 = Context.getMemberPointerType(
-          Context.getQualifiedType(Composite2, Quals), Classes.second);
-    } else {
-      // Rebuild pointer type
-      Composite1 =
-          Context.getPointerType(Context.getQualifiedType(Composite1, Quals));
-      Composite2 =
-          Context.getPointerType(Context.getQualifiedType(Composite2, Quals));
-    }
-  }
-
-  struct Conversion {
-    Sema &S;
-    Expr *&E1, *&E2;
-    QualType Composite;
-    InitializedEntity Entity;
-    InitializationKind Kind;
-    InitializationSequence E1ToC, E2ToC;
-    bool Viable;
-
-    Conversion(Sema &S, SourceLocation Loc, Expr *&E1, Expr *&E2,
-               QualType Composite)
-        : S(S), E1(E1), E2(E2), Composite(Composite),
-          Entity(InitializedEntity::InitializeTemporary(Composite)),
-          Kind(InitializationKind::CreateCopy(Loc, SourceLocation())),
-          E1ToC(S, Entity, Kind, E1), E2ToC(S, Entity, Kind, E2),
-          Viable(E1ToC && E2ToC) {}
-
-    bool perform() {
-      ExprResult E1Result = E1ToC.Perform(S, Entity, Kind, E1);
-      if (E1Result.isInvalid())
-        return true;
-      E1 = E1Result.getAs<Expr>();
+  // At this point, either the inner types are the same or we have failed to
+  // find a composite pointer type.
+  if (!Context.hasSameType(Composite1, Composite2))
+    return QualType();
 
-      ExprResult E2Result = E2ToC.Perform(S, Entity, Kind, E2);
-      if (E2Result.isInvalid())
-        return true;
-      E2 = E2Result.getAs<Expr>();
+  // Per C++ [conv.qual]p3, add 'const' to every level before the last
+  // differing qualifier.
+  for (unsigned I = 0; I != NeedConstBefore; ++I)
+    Steps[I].Quals.addConst();
 
-      return false;
-    }
-  };
+  // Rebuild the composite type.
+  QualType Composite = Composite1;
+  for (auto &S : llvm::reverse(Steps))
+    Composite = S.rebuild(Context, Composite);
 
-  // Try to convert to each composite pointer type.
-  Conversion C1(*this, Loc, E1, E2, Composite1);
-  if (C1.Viable && Context.hasSameType(Composite1, Composite2)) {
-    if (ConvertArgs && C1.perform())
+  if (ConvertArgs) {
+    // Convert the expressions to the composite pointer type.
+    InitializedEntity Entity =
+        InitializedEntity::InitializeTemporary(Composite);
+    InitializationKind Kind =
+        InitializationKind::CreateCopy(Loc, SourceLocation());
+
+    InitializationSequence E1ToC(*this, Entity, Kind, E1);
+    if (!E1ToC)
       return QualType();
-    return C1.Composite;
-  }
-  Conversion C2(*this, Loc, E1, E2, Composite2);
 
-  if (C1.Viable == C2.Viable) {
-    // Either Composite1 and Composite2 are viable and are different, or
-    // neither is viable.
-    // FIXME: How both be viable and different?
-    return QualType();
-  }
+    InitializationSequence E2ToC(*this, Entity, Kind, E2);
+    if (!E2ToC)
+      return QualType();
 
-  // Convert to the chosen type.
-  if (ConvertArgs && (C1.Viable ? C1 : C2).perform())
-    return QualType();
+    // FIXME: Let the caller know if these fail to avoid duplicate diagnostics.
+    ExprResult E1Result = E1ToC.Perform(*this, Entity, Kind, E1);
+    if (E1Result.isInvalid())
+      return QualType();
+    E1 = E1Result.get();
+
+    ExprResult E2Result = E2ToC.Perform(*this, Entity, Kind, E2);
+    if (E2Result.isInvalid())
+      return QualType();
+    E2 = E2Result.get();
+  }
 
-  return C1.Viable ? C1.Composite : C2.Composite;
+  return Composite;
 }
 
 ExprResult Sema::MaybeBindToTemporary(Expr *E) {
index 35739a8..64cdc2c 100644 (file)
 }
 @end
 
+@class A;
+
+void multilevel() {
+  A **p;
+  const A **q;
+
+  using T = decltype(true ? q : p);
+  using T = const A * const *;
+}
+
diff --git a/clang/test/SemaOpenCLCXX/address-space-cond.cl b/clang/test/SemaOpenCLCXX/address-space-cond.cl
new file mode 100644 (file)
index 0000000..8090598
--- /dev/null
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 %s -cl-std=clc++ -pedantic -verify
+
+namespace PointerRvalues {
+
+void f(__global int *__constant *a, const __global int *__constant *b) {
+  using T = decltype(true ? +a : +b);
+  using T = const __global int *const __constant *;
+}
+
+void g(const __global int *a, __generic int *b) {
+  using T = decltype(true ? +a : +b);
+  using T = const __generic int *;
+}
+
+void h(const __global int **a, __generic int **b) {
+  using T = decltype(true ? +a : +b); // expected-error {{incompatible operand types}}
+}
+
+void i(__global int **a, __generic int **b) {
+  using T = decltype(true ? +a : +b); // expected-error {{incompatible operand types}}
+}
+
+}