Old work on P0388. For reference in D102645. Not for review / commit.

[dblaikie]

FWIW, there's some way to post reviews in draft form (I think via arc --draft or something like that) - which would ensure that emails for the review aren't sent to the -commits mailing list for something you don't want folks on the -commits list to review just yet, etc.

[urnathan]

dblaikie, do you know if there's a way to do it outside of arc? (for reasons, I've not got arc to work from inside work). I tried not setting a reviewer, but then that got autofilled.

[dblaikie]

In D102650#2779193, @urnathan wrote:

dblaikie, do you know if there's a way to do it outside of arc? (for reasons, I've not got arc to work from inside work). I tried not setting a reviewer, but then that got autofilled.

I'm not sure - @MaskRay & @mehdi_amini might know more there.

[urnathan]

Taking control to close it

[urnathan]

Implemented as part of D102645, thanks!

clang/include/clang/AST/ASTContext.h

if (SubTnullability.hasValue() == SuperTnullability.hasValue()) {
       }
     }
     return true;
   }
 
   bool ObjCMethodsAreEqual(const ObjCMethodDecl *MethodDecl,
                            const ObjCMethodDecl *MethodImp);
 
-  bool UnwrapSimilarTypes(QualType &T1, QualType &T2);
-  bool UnwrapSimilarArrayTypes(QualType &T1, QualType &T2);
+  bool UnwrapSimilarTypes(QualType &T1, QualType &T2,
+                          bool AllowPiMismatch = true);
+  bool UnwrapSimilarArrayTypes(QualType &T1, QualType &T2,
+                               bool AllowPiMismatch = true);
 
   /// Determine if two types are similar, according to the C++ rules. That is,
   /// determine if they are the same other than qualifiers on the initial
   /// sequence of pointer / pointer-to-member / array (and in Clang, object
   /// pointer) types and their element types.
   ///
   /// Clang offers a number of qualifiers in addition to the C++ qualifiers;
   /// those qualifiers are also ignored in the 'similarity' check.

clang/lib/AST/ASTContext.cpp

QualType ASTContext::getUnqualifiedArrayType(QualType type,
   return getDependentSizedArrayType(unqualElementType, DSAT->getSizeExpr(),
                                     DSAT->getSizeModifier(), 0,
                                     SourceRange());
 }
 
 /// Attempt to unwrap two types that may both be array types with the same bound
 /// (or both be array types of unknown bound) for the purpose of comparing the
 /// cv-decomposition of two types per C++ [conv.qual].
-bool ASTContext::UnwrapSimilarArrayTypes(QualType &T1, QualType &T2) {
+///
+/// \param AllowPiMismatch Allow the Pi1 and Pi2 to differ as described in
+///        C++20 [conv.qual], if permitted by the current language mode.
+bool ASTContext::UnwrapSimilarArrayTypes(QualType &T1, QualType &T2,
+                                         bool AllowPiMismatch) {
   bool UnwrappedAny = false;
   while (true) {
     auto *AT1 = getAsArrayType(T1);
     if (!AT1) return UnwrappedAny;
 
     auto *AT2 = getAsArrayType(T2);
     if (!AT2) return UnwrappedAny;
 
     // If we don't have two array types with the same constant bound nor two
     // incomplete array types, we've unwrapped everything we can.
+    // C++20 also permits one type to be a constant array type and the other
+    // to be an incomplete array type.
+    // FIXME: Consider also unwrapping array of unknown bound and VLA.
     if (auto *CAT1 = dyn_cast<ConstantArrayType>(AT1)) {
       auto *CAT2 = dyn_cast<ConstantArrayType>(AT2);
-      if (!CAT2 || CAT1->getSize() != CAT2->getSize())
+      if (!(CAT2 && CAT1->getSize() == CAT2->getSize()) &&
+          !(getLangOpts().CPlusPlus2a && AllowPiMismatch &&
+            isa<IncompleteArrayType>(AT2)))
+        return UnwrappedAny;
+    } else if (isa<IncompleteArrayType>(AT1)) {
+      if (!isa<IncompleteArrayType>(AT2) &&
+          !(getLangOpts().CPlusPlus2a && AllowPiMismatch &&
+            isa<ConstantArrayType>(AT2)))
         return UnwrappedAny;
-    } else if (!isa<IncompleteArrayType>(AT1) ||
-               !isa<IncompleteArrayType>(AT2)) {
+    } else {
       return UnwrappedAny;
     }
 
     T1 = AT1->getElementType();
     T2 = AT2->getElementType();
     UnwrappedAny = true;
   }
 }
 
 /// Attempt to unwrap two types that may be similar (C++ [conv.qual]).
 ///
 /// If T1 and T2 are both pointer types of the same kind, or both array types
 /// with the same bound, unwraps layers from T1 and T2 until a pointer type is
 /// unwrapped. Top-level qualifiers on T1 and T2 are ignored.
 ///
 /// This function will typically be called in a loop that successively
 /// "unwraps" pointer and pointer-to-member types to compare them at each
 /// level.
 ///
+/// \param AllowPiMismatch Allow the Pi1 and Pi2 to differ as described in
+///        C++20 [conv.qual], if permitted by the current language mode.
+///
 /// \return \c true if a pointer type was unwrapped, \c false if we reached a
 /// pair of types that can't be unwrapped further.
-bool ASTContext::UnwrapSimilarTypes(QualType &T1, QualType &T2) {
-  UnwrapSimilarArrayTypes(T1, T2);
+bool ASTContext::UnwrapSimilarTypes(QualType &T1, QualType &T2,
+                                    bool AllowPiMismatch) {
+  UnwrapSimilarArrayTypes(T1, T2, AllowPiMismatch);
 
   const auto *T1PtrType = T1->getAs<PointerType>();
   const auto *T2PtrType = T2->getAs<PointerType>();
   if (T1PtrType && T2PtrType) {
     T1 = T1PtrType->getPointeeType();
     T2 = T2PtrType->getPointeeType();
     return true;
   }
while (true) {
     Quals1.removeCVRQualifiers();
     Quals2.removeCVRQualifiers();
     if (Quals1 != Quals2)
       return false;
 
     if (hasSameType(T1, T2))
       return true;
 
-    if (!UnwrapSimilarTypes(T1, T2))
+    if (!UnwrapSimilarTypes(T1, T2, /*AllowPiMismatch*/ false))
       return false;
   }
 }
 
 DeclarationNameInfo
 ASTContext::getNameForTemplate(TemplateName Name,
                                SourceLocation NameLoc) const {
   switch (Name.getKind()) {

clang/lib/CodeGen/CGExpr.cpp

case CK_Dynamic: {
     const auto *DCE = cast<CXXDynamicCastExpr>(E);
     return MakeNaturalAlignAddrLValue(EmitDynamicCast(V, DCE), E->getType());
   }
 
   case CK_ConstructorConversion:
   case CK_UserDefinedConversion:
   case CK_CPointerToObjCPointerCast:
   case CK_BlockPointerToObjCPointerCast:
-  case CK_NoOp:
   case CK_LValueToRValue:
     return EmitLValue(E->getSubExpr());
 
+  case CK_NoOp: {
+    // CK_NoOp can model a qualification conversion, which can remove an array
+    // bound and change the IR type.
+    // FIXME: Once pointee types are removed from IR, remove this.
+    LValue LV = EmitLValue(E->getSubExpr());
+    if (LV.isSimple()) {
+      Address V = LV.getAddress(*this);
+      if (V.isValid()) {
+        llvm::Type *T =
+            ConvertTypeForMem(E->getType())
+                ->getPointerTo(
+                    cast<llvm::PointerType>(V.getType())->getAddressSpace());
+        if (V.getType() != T)
+          LV.setAddress(Builder.CreateBitCast(V, T));
+      }
+    }
+    return LV;
+  }
+
   case CK_UncheckedDerivedToBase:
   case CK_DerivedToBase: {
     const auto *DerivedClassTy =
         E->getSubExpr()->getType()->castAs<RecordType>();
     auto *DerivedClassDecl = cast<CXXRecordDecl>(DerivedClassTy->getDecl());
 
     LValue LV = EmitLValue(E->getSubExpr());
     Address This = LV.getAddress(*this);

clang/lib/CodeGen/CGExprScalar.cpp

case CK_AddressSpaceConversion: {
     // Since target may map different address spaces in AST to the same address
     // space, an address space conversion may end up as a bitcast.
     return CGF.CGM.getTargetCodeGenInfo().performAddrSpaceCast(
         CGF, Visit(E), E->getType()->getPointeeType().getAddressSpace(),
         DestTy->getPointeeType().getAddressSpace(), ConvertType(DestTy));
   }
   case CK_AtomicToNonAtomic:
   case CK_NonAtomicToAtomic:
-  case CK_NoOp:
   case CK_UserDefinedConversion:
     return Visit(const_cast<Expr*>(E));
 
+  case CK_NoOp: {
+    // CK_NoOp can model a pointer qualification conversion, which can remove
+    // an array bound and change the IR type.
+    // FIXME: Once pointee types are removed from IR, remove this.
+    llvm::Value *V = Visit(const_cast<Expr *>(E));
+    return V ? Builder.CreateBitCast(V, ConvertType(DestTy)) : nullptr;
+  }
+
   case CK_BaseToDerived: {
     const CXXRecordDecl *DerivedClassDecl = DestTy->getPointeeCXXRecordDecl();
     assert(DerivedClassDecl && "BaseToDerived arg isn't a C++ object pointer!");
 
     Address Base = CGF.EmitPointerWithAlignment(E);
     Address Derived =
       CGF.GetAddressOfDerivedClass(Base, DerivedClassDecl,
                                    CE->path_begin(), CE->path_end(),

clang/lib/CodeGen/CGObjC.cpp

else
       argCK = CK_CPointerToObjCPointerCast;
   } else if (ivarRef.getType()->isBlockPointerType()) {
      if (argLoad.getType()->isBlockPointerType())
       argCK = CK_BitCast;
     else
       argCK = CK_AnyPointerToBlockPointerCast;
   } else if (ivarRef.getType()->isPointerType()) {
     argCK = CK_BitCast;
+  } else if (argLoad.getType()->isAtomicType() &&
+             !ivarRef.getType()->isAtomicType()) {
+    argCK = CK_AtomicToNonAtomic;
   }
   ImplicitCastExpr argCast(ImplicitCastExpr::OnStack,
                            ivarRef.getType(), argCK, &argLoad,
                            VK_RValue);
   Expr *finalArg = &argLoad;
   if (!getContext().hasSameUnqualifiedType(ivarRef.getType(),
                                            argLoad.getType()))
     finalArg = &argCast;

clang/lib/Sema/SemaCast.cpp

if (SrcExpr.isInvalid())
     return TC_Failed;
   if (tcr != TC_NotApplicable)
     return tcr;
 
   // C++ 5.2.9p6: May apply the reverse of any standard conversion, except
   // lvalue-to-rvalue, array-to-pointer, function-to-pointer, and boolean
   // conversions, subject to further restrictions.
   // Also, C++ 5.2.9p1 forbids casting away constness, which makes reversal
-  // of qualification conversions impossible.
+  // of qualification conversions impossible. (In C++20, adding an array bound
+  // would be the reverse of a qualification conversion, but adding permission
+  // to add an array bound in a static_cast is a wording oversight.)
   // In the CStyle case, the earlier attempt to const_cast should have taken
   // care of reverse qualification conversions.
 
   QualType SrcType = Self.Context.getCanonicalType(SrcExpr.get()->getType());
 
   // C++0x 5.2.9p9: A value of a scoped enumeration type can be explicitly
   // converted to an integral type. [...] A value of a scoped enumeration type
   // can also be explicitly converted to a floating-point type [...].

clang/lib/Sema/SemaExprCXX.cpp

if (!Steps.empty()) {
         return QualType();
 
       Steps.back().Quals = Quals;
       if (Q1 != Quals || Q2 != Quals)
         NeedConstBefore = Steps.size() - 1;
     }
 
     // FIXME: Can we unify the following with UnwrapSimilarTypes?
+
+    const ArrayType *Arr1, *Arr2;
+    if ((Arr1 = Context.getAsArrayType(Composite1)) &&
+        (Arr2 = Context.getAsArrayType(Composite2))) {
+      auto *CAT1 = dyn_cast<ConstantArrayType>(Arr1);
+      auto *CAT2 = dyn_cast<ConstantArrayType>(Arr2);
+      if (CAT1 && CAT2 && CAT1->getSize() == CAT2->getSize()) {
+        Composite1 = Arr1->getElementType();
+        Composite2 = Arr2->getElementType();
+        Steps.emplace_back(Step::Array, CAT1);
+        continue;
+      }
+      bool IAT1 = isa<IncompleteArrayType>(Arr1);
+      bool IAT2 = isa<IncompleteArrayType>(Arr2);
+      if ((IAT1 && IAT2) ||
+          (getLangOpts().CPlusPlus2a && (IAT1 != IAT2) &&
+           ((bool)CAT1 != (bool)CAT2) &&
+           (Steps.empty() || Steps.back().K != Step::Array))) {
+        // In C++2a onwards, we can unify an array of N T with an array of
+        // a different or unknown bound. But we can't form an array whose
+        // element type is an array of unknown bound by doing so.
+        Composite1 = Arr1->getElementType();
+        Composite2 = Arr2->getElementType();
+        Steps.emplace_back(Step::Array);
+        if (CAT1 || CAT2)
+          NeedConstBefore = Steps.size();
+        continue;
+      }
+    }
+
     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 (Steps.empty() && ((Composite1->isVoidPointerType() &&
                           (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;
   }
 
   //  - 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,

clang/lib/Sema/SemaInit.cpp

for (step_iterator Step = step_begin(), StepEnd = step_end();
     case SK_BindReferenceToTemporary: {
       // Make sure the "temporary" is actually an rvalue.
       assert(CurInit.get()->isRValue() && "not a temporary");
 
       // Check exception specifications
       if (S.CheckExceptionSpecCompatibility(CurInit.get(), DestType))
         return ExprError();
 
+      // C++2a [conv.init.list]p3:
+      //   The type of the temporary is the type referenced by T, unless T is
+      //   "reference to array of unknown bound of U" [...]
+      QualType TempType = Step->Type;
+      if (auto *IAT = S.Context.getAsIncompleteArrayType(Step->Type)) {
+        auto *CAT = S.Context.getAsConstantArrayType(CurInit.get()->getType());
+        assert(CAT && "array temporary has unknown bound?");
+        TempType = S.Context.getQualifiedType(
+            S.Context.getConstantArrayType(IAT->getElementType(),
+                                           CAT->getSize(), nullptr,
+                                           ArrayType::Normal, 0),
+            Step->Type.getQualifiers());
+      }
+
       // Materialize the temporary into memory.
       MaterializeTemporaryExpr *MTE = S.CreateMaterializeTemporaryExpr(
-          Step->Type, CurInit.get(), Entity.getType()->isLValueReferenceType());
+          TempType, CurInit.get(), Entity.getType()->isLValueReferenceType());
       CurInit = MTE;
 
       // If we're extending this temporary to automatic storage duration -- we
       // need to register its cleanup during the full-expression's cleanups.
       if (MTE->getStorageDuration() == SD_Automatic &&
           MTE->getType().isDestructedType())
         S.Cleanup.setExprNeedsCleanups(true);
       break;

clang/lib/Sema/SemaOverload.cpp

if (ToQuals.getAddressSpace() != FromQuals.getAddressSpace() &&
     return false;
 
   //   -- if the cv 1,j and cv 2,j are different, then const is in
   //      every cv for 0 < k < j.
   if (!CStyle && FromQuals.getCVRQualifiers() != ToQuals.getCVRQualifiers() &&
       !PreviousToQualsIncludeConst)
     return false;
 
+  // The following wording is from C++20, where the result of the conversion
+  // is T3, not T2.
+  //   -- if [...] P1,i [...] is "array of unknown bound of", P3,i is
+  //      "array of unknown bound of"
+  if (FromType->isIncompleteArrayType() && !ToType->isIncompleteArrayType())
+    return false;
+
+  //   -- if the resulting P3,i is different from P1,i [...], then const is
+  //      added to every cv 3_k for 0 < k < i.
+  if (!CStyle && FromType->isConstantArrayType() &&
+      ToType->isIncompleteArrayType() && !PreviousToQualsIncludeConst)
+    return false;
+
   // Keep track of whether all prior cv-qualifiers in the "to" type
   // include const.
   PreviousToQualsIncludeConst =
       PreviousToQualsIncludeConst && ToQuals.hasConst();
   return true;
 }
 
 /// IsQualificationConversion - Determines whether the conversion from

 
 /// CompareQualificationConversions - Compares two standard conversion
 /// sequences to determine whether they can be ranked based on their
 /// qualification conversions (C++ 13.3.3.2p3 bullet 3).
 static ImplicitConversionSequence::CompareKind
 CompareQualificationConversions(Sema &S,
                                 const StandardConversionSequence& SCS1,
                                 const StandardConversionSequence& SCS2) {
-  // C++ 13.3.3.2p3:
+  // C++ [over.ics.rank]p3:
   //  -- S1 and S2 differ only in their qualification conversion and
-  //     yield similar types T1 and T2 (C++ 4.4), respectively, and the
-  //     cv-qualification signature of type T1 is a proper subset of
-  //     the cv-qualification signature of type T2, and S1 is not the
+  //     yield similar types T1 and T2 (C++ 4.4), respectively, [...]
+  // [C++98]
+  //     [...] and the cv-qualification signature of type T1 is a proper subset
+  //     of the cv-qualification signature of type T2, and S1 is not the
   //     deprecated string literal array-to-pointer conversion (4.2).
+  // [C++2a]
+  //     [...] where T1 can be converted to T2 by a qualification conversion.
   if (SCS1.First != SCS2.First || SCS1.Second != SCS2.Second ||
       SCS1.Third != SCS2.Third || SCS1.Third != ICK_Qualification)
     return ImplicitConversionSequence::Indistinguishable;
 
   // FIXME: the example in the standard doesn't use a qualification
   // conversion (!)
   QualType T1 = SCS1.getToType(2);
   QualType T2 = SCS2.getToType(2);
   T1 = S.Context.getCanonicalType(T1);
   T2 = S.Context.getCanonicalType(T2);
   assert(!T1->isReferenceType() && !T2->isReferenceType());
   Qualifiers T1Quals, T2Quals;
   QualType UnqualT1 = S.Context.getUnqualifiedArrayType(T1, T1Quals);
   QualType UnqualT2 = S.Context.getUnqualifiedArrayType(T2, T2Quals);
 
   // If the types are the same, we won't learn anything by unwrapping
   // them.
   if (UnqualT1 == UnqualT2)
     return ImplicitConversionSequence::Indistinguishable;
 
-  ImplicitConversionSequence::CompareKind Result
-    = ImplicitConversionSequence::Indistinguishable;
+  // Don't ever prefer a standard conversion sequence that uses the deprecated
+  // string literal array to pointer conversion.
+  bool CanPick1 = !SCS1.DeprecatedStringLiteralToCharPtr;
+  bool CanPick2 = !SCS2.DeprecatedStringLiteralToCharPtr;
 
   // Objective-C++ ARC:
   //   Prefer qualification conversions not involving a change in lifetime
-  //   to qualification conversions that do not change lifetime.
-  if (SCS1.QualificationIncludesObjCLifetime !=
-                                      SCS2.QualificationIncludesObjCLifetime) {
-    Result = SCS1.QualificationIncludesObjCLifetime
-               ? ImplicitConversionSequence::Worse
-               : ImplicitConversionSequence::Better;
-  }
+  //   to qualification conversions that do change lifetime.
+  if (SCS1.QualificationIncludesObjCLifetime &&
+      !SCS2.QualificationIncludesObjCLifetime)
+    CanPick1 = false;
+  if (SCS2.QualificationIncludesObjCLifetime &&
+      !SCS1.QualificationIncludesObjCLifetime)
+    CanPick2 = false;
 
-  while (S.Context.UnwrapSimilarTypes(T1, T2)) {
-    // Within each iteration of the loop, we check the qualifiers to
-    // determine if this still looks like a qualification
-    // conversion. Then, if all is well, we unwrap one more level of
-    // pointers or pointers-to-members and do it all again
-    // until there are no more pointers or pointers-to-members left
-    // to unwrap. This essentially mimics what
-    // IsQualificationConversion does, but here we're checking for a
-    // strict subset of qualifiers.
-    if (T1.getQualifiers().withoutObjCLifetime() ==
-        T2.getQualifiers().withoutObjCLifetime())
-      // The qualifiers are the same, so this doesn't tell us anything
-      // about how the sequences rank.
-      // ObjC ownership quals are omitted above as they interfere with
-      // the ARC overload rule.
-      ;
-    else if (T2.isMoreQualifiedThan(T1)) {
-      // T1 has fewer qualifiers, so it could be the better sequence.
-      if (Result == ImplicitConversionSequence::Worse)
-        // Neither has qualifiers that are a subset of the other's
-        // qualifiers.
-        return ImplicitConversionSequence::Indistinguishable;
-
-      Result = ImplicitConversionSequence::Better;
-    } else if (T1.isMoreQualifiedThan(T2)) {
-      // T2 has fewer qualifiers, so it could be the better sequence.
-      if (Result == ImplicitConversionSequence::Better)
-        // Neither has qualifiers that are a subset of the other's
-        // qualifiers.
-        return ImplicitConversionSequence::Indistinguishable;
+  bool ObjCLifetimeConversion;
+  if (CanPick1 &&
+      !S.IsQualificationConversion(T1, T2, false, ObjCLifetimeConversion))
+    CanPick1 = false;
+  // FIXME: In Objective-C ARC, we can have qualification conversions in both
+  // directions, so we can't short-cut this second check in general.
+  if (CanPick2 &&
+      !S.IsQualificationConversion(T2, T1, false, ObjCLifetimeConversion))
+    CanPick2 = false;
 
-      Result = ImplicitConversionSequence::Worse;
-    } else {
-      // Qualifiers are disjoint.
-      return ImplicitConversionSequence::Indistinguishable;
-    }
+  if (CanPick1 != CanPick2)
+    return CanPick1 ? ImplicitConversionSequence::Better
+                    : ImplicitConversionSequence::Worse;
 
-    // If the types after this point are equivalent, we're done.
-    if (S.Context.hasSameUnqualifiedType(T1, T2))
-      break;
-  }
-
-  // Check that the winning standard conversion sequence isn't using
-  // the deprecated string literal array to pointer conversion.
-  switch (Result) {
-  case ImplicitConversionSequence::Better:
-    if (SCS1.DeprecatedStringLiteralToCharPtr)
-      Result = ImplicitConversionSequence::Indistinguishable;
-    break;
-
-  case ImplicitConversionSequence::Indistinguishable:
-    break;
-
-  case ImplicitConversionSequence::Worse:
-    if (SCS2.DeprecatedStringLiteralToCharPtr)
-      Result = ImplicitConversionSequence::Indistinguishable;
-    break;
-  }
-
-  return Result;
-}
-
 /// CompareDerivedToBaseConversions - Compares two standard conversion
 /// sequences to determine whether they can be ranked based on their
 /// various kinds of derived-to-base conversions (C++
 /// [over.ics.rank]p4b3).  As part of these checks, we also look at
 /// conversions between Objective-C interface types.
 static ImplicitConversionSequence::CompareKind
 CompareDerivedToBaseConversions(Sema &S, SourceLocation Loc,
                                 const StandardConversionSequence& SCS1,