[clang] 65f5887 - [ObjC generics] Fix not inheriting type bounds in categories/extensions.

Volodymyr Sapsai via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 24 16:35:07 PDT 2020


Author: Volodymyr Sapsai
Date: 2020-04-24T16:32:28-07:00
New Revision: 65f58878e72a40d68ef3899c766846ee9ec7cf29

URL: https://github.com/llvm/llvm-project/commit/65f58878e72a40d68ef3899c766846ee9ec7cf29
DIFF: https://github.com/llvm/llvm-project/commit/65f58878e72a40d68ef3899c766846ee9ec7cf29.diff

LOG: [ObjC generics] Fix not inheriting type bounds in categories/extensions.

When a category/extension doesn't repeat a type bound, corresponding
type parameter is substituted with `id` when used as a type argument. As
a result, in the added test case it was causing errors like

> type argument 'T' (aka 'id') does not satisfy the bound ('id<NSCopying>') of type parameter 'T'

We are already checking that type parameters should be consistent
everywhere (see `checkTypeParamListConsistency`) and update
`ObjCTypeParamDecl` to have correct underlying type. And when we use the
type parameter as a method return type or a method parameter type, it is
substituted to the bounded type. But when we use the type parameter as a
type argument, we check `ObjCTypeParamType` that wasn't updated and
remains `id`.

Fix by updating not only `ObjCTypeParamDecl` UnderlyingType but also
TypeForDecl as we use the underlying type to create a canonical type for
`ObjCTypeParamType` (see `ASTContext::getObjCTypeParamType`).

This is a different approach to fixing the issue. The previous one was
02c2ab3d8872416589bd1a6ca3dfb96ba373a3b9 which was reverted in
4c539e8da1b3de38a53ef3f7497f5c45a3243b61. The problem with the previous
approach was that `ObjCTypeParamType::desugar` was returning underlying
type for `ObjCTypeParamDecl` without applying any protocols stored in
`ObjCTypeParamType`. It caused inconsistencies in comparing types before
and after desugaring.

Re-applying after fixing intermittent test failures.

rdar://problem/54329242

Reviewed By: erik.pilkington

Differential Revision: https://reviews.llvm.org/D72872

Added: 
    

Modified: 
    clang/include/clang/AST/ASTContext.h
    clang/include/clang/AST/Type.h
    clang/lib/AST/ASTContext.cpp
    clang/lib/AST/Type.cpp
    clang/lib/Sema/SemaDeclObjC.cpp
    clang/test/SemaObjC/parameterized_classes_collection_literal.m
    clang/test/SemaObjC/parameterized_classes_subst.m

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index dedbd857819d..82b8a51b9551 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1459,6 +1459,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
 
   QualType getObjCTypeParamType(const ObjCTypeParamDecl *Decl,
                                 ArrayRef<ObjCProtocolDecl *> protocols) const;
+  void adjustObjCTypeParamBoundType(const ObjCTypeParamDecl *Orig,
+                                    ObjCTypeParamDecl *New) const;
 
   bool ObjCObjectAdoptsQTypeProtocols(QualType QT, ObjCInterfaceDecl *Decl);
 

diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 322b14ce641a..cf746d9b947a 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -5605,6 +5605,7 @@ class ObjCTypeParamType : public Type,
   void Profile(llvm::FoldingSetNodeID &ID);
   static void Profile(llvm::FoldingSetNodeID &ID,
                       const ObjCTypeParamDecl *OTPDecl,
+                      QualType CanonicalType,
                       ArrayRef<ObjCProtocolDecl *> protocols);
 
   ObjCTypeParamDecl *getDecl() const { return OTPDecl; }

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index fffcfac60ca7..47834d41b919 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -4898,7 +4898,7 @@ ASTContext::getObjCTypeParamType(const ObjCTypeParamDecl *Decl,
                                  ArrayRef<ObjCProtocolDecl *> protocols) const {
   // Look in the folding set for an existing type.
   llvm::FoldingSetNodeID ID;
-  ObjCTypeParamType::Profile(ID, Decl, protocols);
+  ObjCTypeParamType::Profile(ID, Decl, Decl->getUnderlyingType(), protocols);
   void *InsertPos = nullptr;
   if (ObjCTypeParamType *TypeParam =
       ObjCTypeParamTypes.FindNodeOrInsertPos(ID, InsertPos))
@@ -4924,6 +4924,17 @@ ASTContext::getObjCTypeParamType(const ObjCTypeParamDecl *Decl,
   return QualType(newType, 0);
 }
 
+void ASTContext::adjustObjCTypeParamBoundType(const ObjCTypeParamDecl *Orig,
+                                              ObjCTypeParamDecl *New) const {
+  New->setTypeSourceInfo(getTrivialTypeSourceInfo(Orig->getUnderlyingType()));
+  // Update TypeForDecl after updating TypeSourceInfo.
+  auto NewTypeParamTy = cast<ObjCTypeParamType>(New->getTypeForDecl());
+  SmallVector<ObjCProtocolDecl *, 8> protocols;
+  protocols.append(NewTypeParamTy->qual_begin(), NewTypeParamTy->qual_end());
+  QualType UpdatedTy = getObjCTypeParamType(New, protocols);
+  New->setTypeForDecl(UpdatedTy.getTypePtr());
+}
+
 /// ObjCObjectAdoptsQTypeProtocols - Checks that protocols in IC's
 /// protocol list adopt all protocols in QT's qualified-id protocol
 /// list.

diff  --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
index 982aa8962f03..3408149bd760 100644
--- a/clang/lib/AST/Type.cpp
+++ b/clang/lib/AST/Type.cpp
@@ -3585,15 +3585,17 @@ void ObjCObjectTypeImpl::Profile(llvm::FoldingSetNodeID &ID) {
 
 void ObjCTypeParamType::Profile(llvm::FoldingSetNodeID &ID,
                                 const ObjCTypeParamDecl *OTPDecl,
+                                QualType CanonicalType,
                                 ArrayRef<ObjCProtocolDecl *> protocols) {
   ID.AddPointer(OTPDecl);
+  ID.AddPointer(CanonicalType.getAsOpaquePtr());
   ID.AddInteger(protocols.size());
   for (auto proto : protocols)
     ID.AddPointer(proto);
 }
 
 void ObjCTypeParamType::Profile(llvm::FoldingSetNodeID &ID) {
-  Profile(ID, getDecl(),
+  Profile(ID, getDecl(), getCanonicalTypeInternal(),
           llvm::makeArrayRef(qual_begin(), getNumProtocols()));
 }
 

diff  --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index 934e1a23141c..6db57898e378 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -938,8 +938,7 @@ static bool checkTypeParamListConsistency(Sema &S,
 
       // Override the new type parameter's bound type with the previous type,
       // so that it's consistent.
-      newTypeParam->setTypeSourceInfo(
-        S.Context.getTrivialTypeSourceInfo(prevTypeParam->getUnderlyingType()));
+      S.Context.adjustObjCTypeParamBoundType(prevTypeParam, newTypeParam);
       continue;
     }
 
@@ -966,8 +965,7 @@ static bool checkTypeParamListConsistency(Sema &S,
     }
 
     // Update the new type parameter's bound to match the previous one.
-    newTypeParam->setTypeSourceInfo(
-      S.Context.getTrivialTypeSourceInfo(prevTypeParam->getUnderlyingType()));
+    S.Context.adjustObjCTypeParamBoundType(prevTypeParam, newTypeParam);
   }
 
   return false;

diff  --git a/clang/test/SemaObjC/parameterized_classes_collection_literal.m b/clang/test/SemaObjC/parameterized_classes_collection_literal.m
index 472746e09db9..034d2e8da217 100644
--- a/clang/test/SemaObjC/parameterized_classes_collection_literal.m
+++ b/clang/test/SemaObjC/parameterized_classes_collection_literal.m
@@ -29,7 +29,9 @@ + (instancetype)arrayWithObjects:(const T [])objects count:(NSUInteger)cnt;
 @end
 
 @interface NSDictionary<K, V> : NSObject <NSCopying>
-+ (instancetype)dictionaryWithObjects:(const V [])objects forKeys:(const K [])keys count:(NSUInteger)cnt;
++ (instancetype)dictionaryWithObjects:(const V [])objects
+                              forKeys:(const K <NSCopying> [])keys
+                                count:(NSUInteger)cnt;
 @end
 
 void testArrayLiteral(void) {
@@ -50,3 +52,9 @@ void testDictionaryLiteral(void) {
     @"world" : @"blah" // expected-warning{{object of type 'NSString *' is not compatible with dictionary value type 'NSNumber *'}}
   };
 }
+
+void testCastingInDictionaryLiteral(NSString *arg) {
+  NSDictionary *dict = @{
+    (id)arg : @"foo",
+  };
+}

diff  --git a/clang/test/SemaObjC/parameterized_classes_subst.m b/clang/test/SemaObjC/parameterized_classes_subst.m
index d14a6e9deb40..b6d884760d29 100644
--- a/clang/test/SemaObjC/parameterized_classes_subst.m
+++ b/clang/test/SemaObjC/parameterized_classes_subst.m
@@ -467,3 +467,17 @@ - (void)mapUsingBlock:(id (^)(id))block {
 - (void)mapUsingBlock2:(id)block { // expected-warning{{conflicting parameter types in implementation}}
 }
 @end
+
+// --------------------------------------------------------------------------
+// Use a type parameter as a type argument.
+// --------------------------------------------------------------------------
+// Type bounds in a category/extension are omitted. rdar://problem/54329242
+ at interface ParameterizedContainer<T : id<NSCopying>>
+- (ParameterizedContainer<T> *)inInterface;
+ at end
+ at interface ParameterizedContainer<T> (Cat)
+- (ParameterizedContainer<T> *)inCategory;
+ at end
+ at interface ParameterizedContainer<U> ()
+- (ParameterizedContainer<U> *)inExtension;
+ at end


        


More information about the cfe-commits mailing list