DR330: look through array types when forming the cv-decomposition of a type.
authorRichard Smith <richard-llvm@metafoo.co.uk>
Wed, 11 Jul 2018 00:19:19 +0000 (00:19 +0000)
committerRichard Smith <richard-llvm@metafoo.co.uk>
Wed, 11 Jul 2018 00:19:19 +0000 (00:19 +0000)
This allows more qualification conversions, eg. conversion from
   'int *(*)[]' -> 'const int *const (*)[]'
is now permitted, along with all the consequences of that: more types
are similar, more cases are permitted by const_cast, and conversely,
fewer "casting away constness" cases are permitted by reinterpret_cast.

llvm-svn: 336745

clang/include/clang/AST/ASTContext.h
clang/lib/AST/ASTContext.cpp
clang/lib/Sema/SemaCast.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
clang/test/CXX/drs/dr3xx.cpp
clang/test/SemaCXX/const-cast.cpp
clang/www/cxx_dr_status.html

index ac317a7..ddfbdc2 100644 (file)
@@ -2282,7 +2282,19 @@ public:
   bool ObjCMethodsAreEqual(const ObjCMethodDecl *MethodDecl,
                            const ObjCMethodDecl *MethodImp);
 
-  bool UnwrapSimilarPointerTypes(QualType &T1, QualType &T2);
+  bool UnwrapSimilarTypes(QualType &T1, QualType &T2);
+
+  /// 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.
+  bool hasSimilarType(QualType T1, QualType T2);
+
+  /// Determine if two types are similar, ignoring only CVR qualifiers.
+  bool hasCvrSimilarType(QualType T1, QualType T2);
 
   /// Retrieves the "canonical" nested name specifier for a
   /// given nested name specifier.
index ca54d8f..832f3a2 100644 (file)
@@ -4965,15 +4965,49 @@ QualType ASTContext::getUnqualifiedArrayType(QualType type,
                                     SourceRange());
 }
 
-/// UnwrapSimilarPointerTypes - If T1 and T2 are pointer types  that
-/// may be similar (C++ 4.4), replaces T1 and T2 with the type that
-/// they point to and return true. If T1 and T2 aren't pointer types
-/// or pointer-to-member types, or if they are not similar at this
-/// level, returns false and leaves T1 and T2 unchanged. 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.
-bool ASTContext::UnwrapSimilarPointerTypes(QualType &T1, QualType &T2) {
+/// 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].
+static void unwrapSimilarArrayTypes(ASTContext &Ctx, QualType &T1,
+                                    QualType &T2) {
+  while (true) {
+    auto *AT1 = Ctx.getAsArrayType(T1);
+    if (!AT1) return;
+
+    auto *AT2 = Ctx.getAsArrayType(T2);
+    if (!AT2) return;
+
+    // If we don't have two array types with the same constant bound nor two
+    // incomplete array types, we've unwrapped everything we can.
+    if (auto *CAT1 = dyn_cast<ConstantArrayType>(AT1)) {
+      auto *CAT2 = dyn_cast<ConstantArrayType>(AT2);
+      if (!CAT2 || CAT1->getSize() != CAT2->getSize())
+        return;
+    } else if (!isa<IncompleteArrayType>(AT1) ||
+               !isa<IncompleteArrayType>(AT2)) {
+      return;
+    }
+
+    T1 = AT1->getElementType();
+    T2 = AT2->getElementType();
+  }
+}
+
+/// 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.
+///
+/// \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(*this, T1, T2);
+
   const auto *T1PtrType = T1->getAs<PointerType>();
   const auto *T2PtrType = T2->getAs<PointerType>();
   if (T1PtrType && T2PtrType) {
@@ -5007,6 +5041,37 @@ bool ASTContext::UnwrapSimilarPointerTypes(QualType &T1, QualType &T2) {
   return false;
 }
 
+bool ASTContext::hasSimilarType(QualType T1, QualType T2) {
+  while (true) {
+    Qualifiers Quals;
+    T1 = getUnqualifiedArrayType(T1, Quals);
+    T2 = getUnqualifiedArrayType(T2, Quals);
+    if (hasSameType(T1, T2))
+      return true;
+    if (!UnwrapSimilarTypes(T1, T2))
+      return false;
+  }
+}
+
+bool ASTContext::hasCvrSimilarType(QualType T1, QualType T2) {
+  while (true) {
+    Qualifiers Quals1, Quals2;
+    T1 = getUnqualifiedArrayType(T1, Quals1);
+    T2 = getUnqualifiedArrayType(T2, Quals2);
+
+    Quals1.removeCVRQualifiers();
+    Quals2.removeCVRQualifiers();
+    if (Quals1 != Quals2)
+      return false;
+
+    if (hasSameType(T1, T2))
+      return true;
+
+    if (!UnwrapSimilarTypes(T1, T2))
+      return false;
+  }
+}
+
 DeclarationNameInfo
 ASTContext::getNameForTemplate(TemplateName Name,
                                SourceLocation NameLoc) const {
index a2e2950..ec95032 100644 (file)
@@ -456,12 +456,16 @@ enum CastAwayConstnessKind {
 
 /// Unwrap one level of types for CastsAwayConstness.
 ///
-/// Like Sema::UnwrapSimilarPointerTypes, this removes one level of
-/// indirection from both types, provided that they're both pointer-like.
-/// Unlike the Sema function, doesn't care if the unwrapped pieces are related.
+/// Like Sema::UnwrapSimilarTypes, this removes one level of indirection from
+/// both types, provided that they're both pointer-like or array-like. Unlike
+/// the Sema function, doesn't care if the unwrapped pieces are related.
 static CastAwayConstnessKind
 unwrapCastAwayConstnessLevel(ASTContext &Context, QualType &T1, QualType &T2) {
-  if (Context.UnwrapSimilarPointerTypes(T1, T2))
+  // Note, even if this returns false, it may have unwrapped some number of
+  // matching "array of" pieces. That's OK, we don't need to check their
+  // cv-qualifiers (that check is covered by checking the qualifiers on the
+  // array types themselves).
+  if (Context.UnwrapSimilarTypes(T1, T2))
     return CastAwayConstnessKind::CACK_Similar;
 
   // Special case: if the destination type is a reference type, unwrap it as
@@ -473,8 +477,11 @@ unwrapCastAwayConstnessLevel(ASTContext &Context, QualType &T1, QualType &T2) {
 
   auto Classify = [](QualType T) {
     if (T->isAnyPointerType()) return 1;
-    if (T->getAs<MemberPointerType>()) return 2;
-    if (T->getAs<BlockPointerType>()) return 3;
+    if (T->isMemberPointerType()) return 2;
+    if (T->isBlockPointerType()) return 3;
+    // We somewhat-arbitrarily don't look through VLA types here. This is at
+    // least consistent with the behavior of UnwrapSimilarTypes.
+    if (T->isConstantArrayType() || T->isIncompleteArrayType()) return 4;
     return 0;
   };
 
@@ -486,8 +493,14 @@ unwrapCastAwayConstnessLevel(ASTContext &Context, QualType &T1, QualType &T2) {
   if (!T2Class)
     return CastAwayConstnessKind::CACK_None;
 
-  T1 = T1->getPointeeType();
-  T2 = T2->getPointeeType();
+  auto Unwrap = [&](QualType T) {
+    if (auto *AT = Context.getAsArrayType(T))
+      return AT->getElementType();
+    return T->getPointeeType();
+  };
+
+  T1 = Unwrap(T1);
+  T2 = Unwrap(T2);
   return T1Class == T2Class ? CastAwayConstnessKind::CACK_SimilarKind
                             : CastAwayConstnessKind::CACK_Incoherent;
 }
@@ -1674,29 +1687,14 @@ static TryCastResult TryConstCast(Sema &Self, ExprResult &SrcExpr,
       msg = diag::err_bad_const_cast_dest;
     return TC_NotApplicable;
   }
-  SrcType = Self.Context.getCanonicalType(SrcType);
 
-  // Unwrap the pointers. Ignore qualifiers. Terminate early if the types are
-  // completely equal.
-  // C++ 5.2.11p3 describes the core semantics of const_cast. All cv specifiers
-  // in multi-level pointers may change, but the level count must be the same,
-  // as must be the final pointee type.
-  while (SrcType != DestType &&
-         Self.Context.UnwrapSimilarPointerTypes(SrcType, DestType)) {
-    Qualifiers SrcQuals, DestQuals;
-    SrcType = Self.Context.getUnqualifiedArrayType(SrcType, SrcQuals);
-    DestType = Self.Context.getUnqualifiedArrayType(DestType, DestQuals);
-    
-    // const_cast is permitted to strip cvr-qualifiers, only. Make sure that
-    // the other qualifiers (e.g., address spaces) are identical.
-    SrcQuals.removeCVRQualifiers();
-    DestQuals.removeCVRQualifiers();
-    if (SrcQuals != DestQuals)
-      return TC_NotApplicable;
-  }
-
-  // Since we're dealing in canonical types, the remainder must be the same.
-  if (SrcType != DestType)
+  // C++ [expr.const.cast]p3:
+  //   "For two similar types T1 and T2, [...]"
+  //
+  // We only allow a const_cast to change cvr-qualifiers, not other kinds of
+  // type qualifiers. (Likewise, we ignore other changes when determining
+  // whether a cast casts away constness.)
+  if (!Self.Context.hasCvrSimilarType(SrcType, DestType))
     return TC_NotApplicable;
 
   if (NeedToMaterializeTemporary)
index 6f01f19..35c3612 100644 (file)
@@ -3087,7 +3087,7 @@ Sema::IsQualificationConversion(QualType FromType, QualType ToType,
   //   in multi-level pointers, subject to the following rules: [...]
   bool PreviousToQualsIncludeConst = true;
   bool UnwrappedAnyPointer = false;
-  while (Context.UnwrapSimilarPointerTypes(FromType, ToType)) {
+  while (Context.UnwrapSimilarTypes(FromType, ToType)) {
     // 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
@@ -3642,16 +3642,6 @@ CompareImplicitConversionSequences(Sema &S, SourceLocation Loc,
   return Result;
 }
 
-static bool hasSimilarType(ASTContext &Context, QualType T1, QualType T2) {
-  while (Context.UnwrapSimilarPointerTypes(T1, T2)) {
-    Qualifiers Quals;
-    T1 = Context.getUnqualifiedArrayType(T1, Quals);
-    T2 = Context.getUnqualifiedArrayType(T2, Quals);
-  }
-
-  return Context.hasSameUnqualifiedType(T1, T2);
-}
-
 // Per 13.3.3.2p3, compare the given standard conversion sequences to
 // determine if one is a proper subset of the other.
 static ImplicitConversionSequence::CompareKind
@@ -3675,7 +3665,7 @@ compareStandardConversionSubsets(ASTContext &Context,
       Result = ImplicitConversionSequence::Worse;
     else
       return ImplicitConversionSequence::Indistinguishable;
-  } else if (!hasSimilarType(Context, SCS1.getToType(1), SCS2.getToType(1)))
+  } else if (!Context.hasSimilarType(SCS1.getToType(1), SCS2.getToType(1)))
     return ImplicitConversionSequence::Indistinguishable;
 
   if (SCS1.Third == SCS2.Third) {
@@ -3949,7 +3939,7 @@ CompareQualificationConversions(Sema &S,
                : ImplicitConversionSequence::Better;
   }
 
-  while (S.Context.UnwrapSimilarPointerTypes(T1, T2)) {
+  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
index 59bf9f8..febe7cd 100644 (file)
@@ -459,7 +459,7 @@ DefinedOrUnknownSVal SValBuilder::evalEQ(ProgramStateRef state,
 /// Assumes the input types are canonical.
 static bool shouldBeModeledWithNoOp(ASTContext &Context, QualType ToTy,
                                                          QualType FromTy) {
-  while (Context.UnwrapSimilarPointerTypes(ToTy, FromTy)) {
+  while (Context.UnwrapSimilarTypes(ToTy, FromTy)) {
     Qualifiers Quals1, Quals2;
     ToTy = Context.getUnqualifiedArrayType(ToTy, Quals1);
     FromTy = Context.getUnqualifiedArrayType(FromTy, Quals2);
@@ -474,6 +474,10 @@ static bool shouldBeModeledWithNoOp(ASTContext &Context, QualType ToTy,
 
   // If we are casting to void, the 'From' value can be used to represent the
   // 'To' value.
+  //
+  // FIXME: Doing this after unwrapping the types doesn't make any sense. A
+  // cast from 'int**' to 'void**' is not special in the way that a cast from
+  // 'int*' to 'void*' is.
   if (ToTy->isVoidType())
     return true;
 
index 1526100..04fd258 100644 (file)
@@ -354,6 +354,57 @@ namespace dr329 { // dr329: 3.5
   }
 }
 
+namespace dr330 { // dr330: 7
+  // Conversions between P and Q will be allowed by P0388.
+  typedef int *(*P)[3];
+  typedef const int *const (*Q)[3];
+  typedef const int *Qinner[3];
+  typedef Qinner const *Q2; // same as Q, but 'const' written outside the array type
+  typedef const int *const (*R)[4];
+  typedef const int *const (*S)[];
+  typedef const int *(*T)[];
+  void f(P p, Q q, Q2 q2, R r, S s, T t) {
+    q = p; // ok
+    q2 = p; // ok
+    r = p; // expected-error {{incompatible}}
+    s = p; // expected-error {{incompatible}} (for now)
+    t = p; // expected-error {{incompatible}}
+    s = q; // expected-error {{incompatible}}
+    s = q2; // expected-error {{incompatible}}
+    s = t; // ok, adding const
+    t = s; // expected-error {{incompatible}}
+    (void) const_cast<P>(q);
+    (void) const_cast<P>(q2);
+    (void) const_cast<Q>(p);
+    (void) const_cast<Q2>(p);
+    (void) const_cast<S>(p); // expected-error {{not allowed}} (for now)
+    (void) const_cast<P>(s); // expected-error {{not allowed}} (for now)
+    (void) const_cast<S>(q); // expected-error {{not allowed}}
+    (void) const_cast<S>(q2); // expected-error {{not allowed}}
+    (void) const_cast<Q>(s); // expected-error {{not allowed}}
+    (void) const_cast<Q2>(s); // expected-error {{not allowed}}
+    (void) const_cast<T>(s);
+    (void) const_cast<S>(t);
+    (void) const_cast<T>(q); // expected-error {{not allowed}}
+    (void) const_cast<Q>(t); // expected-error {{not allowed}}
+
+    (void) reinterpret_cast<P>(q); // expected-error {{casts away qualifiers}}
+    (void) reinterpret_cast<P>(q2); // expected-error {{casts away qualifiers}}
+    (void) reinterpret_cast<Q>(p);
+    (void) reinterpret_cast<Q2>(p);
+    (void) reinterpret_cast<S>(p);
+    (void) reinterpret_cast<P>(s); // expected-error {{casts away qualifiers}}
+    (void) reinterpret_cast<S>(q);
+    (void) reinterpret_cast<S>(q2);
+    (void) reinterpret_cast<Q>(s);
+    (void) reinterpret_cast<Q2>(s);
+    (void) reinterpret_cast<T>(s); // expected-error {{casts away qualifiers}}
+    (void) reinterpret_cast<S>(t);
+    (void) reinterpret_cast<T>(q); // expected-error {{casts away qualifiers}}
+    (void) reinterpret_cast<Q>(t);
+  }
+}
+
 namespace dr331 { // dr331: yes
   struct A {
     A(volatile A&); // expected-note {{candidate}}
index 87df61c..a926866 100644 (file)
@@ -58,8 +58,14 @@ short *bad_const_cast_test(char const *volatile *const volatile *var)
   // Non-pointer.
   char v = const_cast<char>(**var2); // expected-error {{const_cast to 'char', which is not a reference, pointer-to-object, or pointer-to-data-member}}
   const int *ar[100] = {0};
-  // Not even lenient g++ accepts this.
-  int *(*rar)[100] = const_cast<int *(*)[100]>(&ar); // expected-error {{const_cast from 'const int *(*)[100]' to 'int *(*)[100]' is not allowed}}
+  extern const int *aub[];
+  // const_cast looks through arrays as of DR330.
+  (void) const_cast<int *(*)[100]>(&ar); // ok
+  (void) const_cast<int *(*)[]>(&aub); // ok
+  // ... but the array bound must exactly match.
+  (void) const_cast<int *(*)[]>(&ar); // expected-error {{const_cast from 'const int *(*)[100]' to 'int *(*)[]' is not allowed}}
+  (void) const_cast<int *(*)[99]>(&ar); // expected-error {{const_cast from 'const int *(*)[100]' to 'int *(*)[99]' is not allowed}}
+  (void) const_cast<int *(*)[100]>(&aub); // expected-error {{const_cast from 'const int *(*)[]' to 'int *(*)[100]' is not allowed}}
   f fp1 = 0;
   // Function pointers.
   f fp2 = const_cast<f>(fp1); // expected-error {{const_cast to 'f' (aka 'int (*)(int)'), which is not a reference, pointer-to-object, or pointer-to-data-member}}
index e47dbd6..93d4ee9 100644 (file)
@@ -2021,7 +2021,7 @@ of class templates</td>
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#330">330</a></td>
     <td>CD4</td>
     <td>Qualification conversions and pointers to arrays of pointers</td>
-    <td class="none" align="center">Unknown</td>
+    <td class="svn" align="center">SVN</td>
   </tr>
   <tr id="331">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#331">331</a></td>