ObjC: Use a new type for ObjC type parameter (patch 3 out of 3)

[manmanren]

Summary

Depends on https://reviews.llvm.org/D23078 and https://reviews.llvm.org/D23079

We say ObjCTypeParamType is ObjCObjectPointerType, but we assert
fail when trying to call Type::getPointeeType.

We compare the types between declaration and definition for ObjCMethods that
use the type parameter, by using simpleTransform that transforms the
ObjCTypeParamType to the underlying type of the ObjCTypeParamDecl. This matches
our previous behavior.

rdar://24619481
rdar://25060179

[doug.gregor]

I think most of the complexity here will fold away when ObjCTypeParamType becomes sugar for the underlying ObjCObjectPointerType.

lib/AST/Type.cpp

1095 ↗

(On Diff #66529)

I think we should combine the protocol lists.

1290 ↗

(On Diff #66529)

I think you won't need this if you take the approach I suggested on the previous patch of making ObjCTypeParamType type sugar for, effectively, the type you're computing here.

lib/Sema/SemaExprObjC.cpp

388 ↗

(On Diff #66529)

Dropping protocol qualifiers here? Again, I think this would be solved by treating ObjCTypeParamType as sugar.

824 ↗

(On Diff #66529)

Also unnecessary w/ the desugaring approach?

lib/Sema/SemaType.cpp

901 ↗

(On Diff #66529)

... same comment about desugaring.

917 ↗

(On Diff #66529)

Here too :)

3397 ↗

(On Diff #66529)

... same comment about desugaring.

lib/StaticAnalyzer/Core/CallEvent.cpp

63 ↗

(On Diff #66529)

... same comment about desugaring.

133 ↗

(On Diff #66529)

... same comment about desugaring.

[manmanren]

I will update this patch once the 2nd patch is done.

Thanks for reviewing,
Manman

lib/AST/Type.cpp

1095 ↗

(On Diff #66529)

I did combine the protocol lists in the implementation, but forgot to remove the comments :]

1290 ↗

(On Diff #66529)

I will update this once we fix the previous patch.

[manmanren]

This patch is now much simpler with the updated version of D23079.

[doug.gregor]

Ahhh, much cleaner. Thanks!

cfe/trunk/lib/AST/ASTContext.cpp

 /// If this is for the canonical type of a type parameter, we can apply
 /// protocol qualifiers on the ObjCObjectPointerType.
 QualType
 ASTContext::applyObjCProtocolQualifiers(QualType type,
                   ArrayRef<ObjCProtocolDecl *> protocols, bool &hasError,
                   bool allowOnPointerType) const {
   hasError = false;
 
+  if (const ObjCTypeParamType *objT =
+      dyn_cast<ObjCTypeParamType>(type.getTypePtr())) {
+    return getObjCTypeParamType(objT->getDecl(), protocols);
+  }
+
   // Apply protocol qualifiers to ObjCObjectPointerType.
   if (allowOnPointerType) {
     if (const ObjCObjectPointerType *objPtr =
         dyn_cast<ObjCObjectPointerType>(type.getTypePtr())) {
       const ObjCObjectType *objT = objPtr->getObjectType();
       // Merge protocol lists and construct ObjCObjectType.
       SmallVector<ObjCProtocolDecl*, 8> protocolsVec;
       protocolsVec.append(objT->qual_begin(),

cfe/trunk/lib/AST/DeclObjC.cpp

 ObjCTypeParamDecl *ObjCTypeParamDecl::Create(ASTContext &ctx, DeclContext *dc,
                                              ObjCTypeParamVariance variance,
                                              SourceLocation varianceLoc,
                                              unsigned index,
                                              SourceLocation nameLoc,
                                              IdentifierInfo *name,
                                              SourceLocation colonLoc,
                                              TypeSourceInfo *boundInfo) {
-  return new (ctx, dc) ObjCTypeParamDecl(ctx, dc, variance, varianceLoc, index,
-                                         nameLoc, name, colonLoc, boundInfo);
+  auto *TPDecl =
+    new (ctx, dc) ObjCTypeParamDecl(ctx, dc, variance, varianceLoc, index,
+  QualType TPType = ctx.getObjCTypeParamType(TPDecl, {});
+  TPDecl->setTypeForDecl(TPType.getTypePtr());
+  return TPDecl;
 }
 
 ObjCTypeParamDecl *ObjCTypeParamDecl::CreateDeserialized(ASTContext &ctx,
                                                          unsigned ID) {
   return new (ctx, ID) ObjCTypeParamDecl(ctx, nullptr,
                                          ObjCTypeParamVariance::Invariant,
                                          SourceLocation(), 0, SourceLocation(),
                                          nullptr, SourceLocation(), nullptr);

cfe/trunk/lib/AST/Type.cpp

QualType QualType::substObjCTypeArgs(
            ArrayRef<QualType> typeArgs,
            ObjCSubstitutionContext context) const {
   return simpleTransform(ctx, *this,
                          [&](QualType type) -> QualType {
     SplitQualType splitType = type.split();
 
     // Replace an Objective-C type parameter reference with the corresponding
     // type argument.
-    if (const auto *typedefTy = dyn_cast<TypedefType>(splitType.Ty)) {
-      if (auto *typeParam = dyn_cast<ObjCTypeParamDecl>(typedefTy->getDecl())) {
+    if (const auto *OTPTy = dyn_cast<ObjCTypeParamType>(splitType.Ty)) {
+      if (auto *typeParam = dyn_cast<ObjCTypeParamDecl>(OTPTy->getDecl())) {
         // If we have type arguments, use them.
         if (!typeArgs.empty()) {
-          // FIXME: Introduce SubstObjCTypeParamType ?
           QualType argType = typeArgs[typeParam->getIndex()];
+          if (OTPTy->qual_empty())
             return ctx.getQualifiedType(argType, splitType.Quals);
+
+          // Apply protocol lists if exists.
+          bool hasError;
+          SmallVector<ObjCProtocolDecl*, 8> protocolsVec;
+          protocolsVec.append(OTPTy->qual_begin(),
+                              OTPTy->qual_end());
+          ArrayRef<ObjCProtocolDecl *> protocolsToApply = protocolsVec;
+          QualType resultTy = ctx.applyObjCProtocolQualifiers(argType,
+              protocolsToApply, hasError, true/*allowOnPointerType*/);
+
+          return ctx.getQualifiedType(resultTy, splitType.Quals);
         }
 
         switch (context) {
         case ObjCSubstitutionContext::Ordinary:
         case ObjCSubstitutionContext::Parameter:
         case ObjCSubstitutionContext::Superclass:
           // Substitute the bound.
           return ctx.getQualifiedType(typeParam->getUnderlyingType(),

cfe/trunk/lib/Sema/SemaDeclObjC.cpp

S.Diag(ImplVar->getLocation(),
       << DiagNullabilityKind(
            *IfaceTy->getNullability(S.Context),
            ((IfaceVar->getObjCDeclQualifier() & Decl::OBJC_TQ_CSNullability)
             != 0));
     S.Diag(IfaceVar->getLocation(), diag::note_previous_declaration);
   }
   if (S.Context.hasSameUnqualifiedType(ImplTy, IfaceTy))
     return true;
-  
   if (!Warn)
     return false;
   unsigned DiagID = 
     IsOverridingMode ? diag::warn_conflicting_overriding_param_types 
                      : diag::warn_conflicting_param_types;
 
   // Mismatches between ObjC pointers go into a different warning
   // category, and sometimes they're even completely whitelisted.

cfe/trunk/lib/Sema/SemaType.cpp

TypeResult Sema::actOnObjCTypeArgsAndProtocolQualifiers(
   // For id<Proto1, Proto2> or Class<Proto1, Proto2>, we'll have an
   // object pointer type. Fill in source information for it.
   if (auto ObjCObjectPointerTL = ResultTL.getAs<ObjCObjectPointerTypeLoc>()) {
     // The '*' is implicit.
     ObjCObjectPointerTL.setStarLoc(SourceLocation());
     ResultTL = ObjCObjectPointerTL.getPointeeLoc();
   }
 
+  if (auto OTPTL = ResultTL.getAs<ObjCTypeParamTypeLoc>()) {
+    // Protocol qualifier information.
+    if (OTPTL.getNumProtocols() > 0) {
+      assert(OTPTL.getNumProtocols() == Protocols.size());
+      OTPTL.setProtocolLAngleLoc(ProtocolLAngleLoc);
+      OTPTL.setProtocolRAngleLoc(ProtocolRAngleLoc);
+      for (unsigned i = 0, n = Protocols.size(); i != n; ++i)
+        OTPTL.setProtocolLoc(i, ProtocolLocs[i]);
+    }
+
+    // We're done. Return the completed type to the parser.
+    return CreateParsedType(Result, ResultTInfo);
+  }
+
   auto ObjCObjectTL = ResultTL.castAs<ObjCObjectTypeLoc>();
 
   // Type argument information.
   if (ObjCObjectTL.getNumTypeArgs() > 0) {
     assert(ObjCObjectTL.getNumTypeArgs() == ActualTypeArgInfos.size());
     ObjCObjectTL.setTypeArgsLAngleLoc(TypeArgsLAngleLoc);
     ObjCObjectTL.setTypeArgsRAngleLoc(TypeArgsRAngleLoc);
     for (unsigned i = 0, n = ActualTypeArgInfos.size(); i != n; ++i)
if (!desugared->canHaveNullability()) {
     Diag(nullabilityLoc, diag::err_nullability_nonpointer)
       << DiagNullabilityKind(nullability, isContextSensitive) << type;
     return true;
   }
   
   // For the context-sensitive keywords/Objective-C property
   // attributes, require that the type be a single-level pointer.
   if (isContextSensitive) {
-    // Make sure that the pointee isn't itself a pointer type.
     QualType pointeeType = desugared->getPointeeType();
     if (pointeeType->isAnyPointerType() ||
         pointeeType->isObjCObjectPointerType() ||
         pointeeType->isMemberPointerType()) {
       Diag(nullabilityLoc, diag::err_nullability_cs_multilevel)
         << DiagNullabilityKind(nullability, true)
         << type;
       Diag(nullabilityLoc, diag::note_nullability_type_specifier)
         << DiagNullabilityKind(nullability, false)
         << type
         << FixItHint::CreateReplacement(nullabilityLoc,
                                         getNullabilitySpelling(nullability));
       return true;
     }
   }
 
   // Form the attributed type.
   type = Context.getAttributedType(
            AttributedType::getNullabilityAttrKind(nullability), type, type);
   return false;
 }
 
 bool Sema::checkObjCKindOfType(QualType &type, SourceLocation loc) {
+  if (isa<ObjCTypeParamType>(type)) {
+    // Build the attributed type to record where __kindof occurred.
+    type = Context.getAttributedType(AttributedType::attr_objc_kindof,
+                                     type, type);
+    return false;
+  }
+
   // Find out if it's an Objective-C object or object pointer type;
   const ObjCObjectPointerType *ptrType = type->getAs<ObjCObjectPointerType>();
   const ObjCObjectType *objType = ptrType ? ptrType->getObjectType() 
                                           : type->getAs<ObjCObjectType>();
 
   // If not, we can't apply __kindof.
   if (!objType) {
     // FIXME: Handle dependent types that aren't yet object types.

cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp

 static bool isObjCTypeParamDependent(QualType Type) {
   // It is illegal to typedef parameterized types inside an interface. Therfore
   // an Objective-C type can only be dependent on a type parameter when the type
   // parameter structurally present in the type itself.
   class IsObjCTypeParamDependentTypeVisitor
       : public RecursiveASTVisitor<IsObjCTypeParamDependentTypeVisitor> {
   public:
     IsObjCTypeParamDependentTypeVisitor() : Result(false) {}
-    bool VisitTypedefType(const TypedefType *Type) {
+    bool VisitObjCTypeParamType(const ObjCTypeParamType *Type) {
       if (isa<ObjCTypeParamDecl>(Type->getDecl())) {
         Result = true;
         return false;
       }
       return true;
     }
 
     bool Result;

cfe/trunk/test/SemaObjC/kindof.m

 - (void)test:(__kindof id)T;
 @end
 @implementation A2
 - (void)test:(id)T {
 }
 @end
 
 @interface NSGeneric<ObjectType> : NSObject
-- (void)test:(__kindof ObjectType)T;
+- (void)test:(__kindof ObjectType)T; // expected-note{{passing argument to parameter 'T' here}}
 - (void)mapUsingBlock:(id (^)(__kindof ObjectType))block;
 @end
 @implementation NSGeneric
 - (void)test:(id)T {
 }
 - (void)mapUsingBlock:(id (^)(id))block {
 }
 @end
 
+void testGeneric(NSGeneric<NSString*> *generic) {
+  NSObject *NSObject_obj;
+  // Assign from NSObject_obj to __kindof NSString*.
+  [generic test:NSObject_obj]; // expected-warning{{incompatible pointer types sending 'NSObject *' to parameter of type '__kindof NSString *'}}
+  NSString *NSString_str;
+  [generic test:NSString_str];
+}
+
 // Check that clang doesn't crash when a type parameter is illegal.
 @interface Array1<T> : NSObject
 @end
 
 @interface I1 : NSObject
 @end
 
 @interface Array1<__kindof I1*>(extensions1) // expected-error{{expected type parameter name}}
 @end
 
 @interface Array2<T1, T2, T3> : NSObject
 @end
 
 @interface Array2<T, T, __kindof I1*>(extensions2) // expected-error{{expected type parameter name}} expected-error{{redeclaration of type parameter 'T'}}
 @end

cfe/trunk/test/SemaObjC/parameterized_classes_subst.m

+ (void)useSuperMethod {
   ip = [super array]; // expected-warning{{from 'NSArray<NSString *> *'}}
 }
 @end
 
 // --------------------------------------------------------------------------
 // warning about likely protocol/class name typos.
 // --------------------------------------------------------------------------
 typedef NSArray<NSObject> ArrayOfNSObjectWarning; // expected-warning{{parameterized class 'NSArray' already conforms to the protocols listed; did you forget a '*'?}}
+
+// rdar://25060179
+@interface MyMutableDictionary<KeyType, ObjectType> : NSObject
+- (void)setObject:(ObjectType)obj forKeyedSubscript:(KeyType <NSCopying>)key; // expected-note{{passing argument to parameter 'obj' here}} \
+    // expected-note{{passing argument to parameter 'key' here}}
+@end
+
+void bar(MyMutableDictionary<NSString *, NSString *> *stringsByString,
+                             NSNumber *n1, NSNumber *n2) {
+  // We warn here when the key types do not match.
+  stringsByString[n1] = n2; // expected-warning{{incompatible pointer types sending 'NSNumber *' to parameter of type 'NSString *'}} \
+    // expected-warning{{incompatible pointer types sending 'NSNumber *' to parameter of type 'NSString<NSCopying> *'}}
+}
+
+@interface MyTest<K, V> : NSObject <NSCopying>
+- (V)test:(K)key;
+- (V)test2:(K)key; // expected-note{{previous definition is here}}
+- (void)mapUsingBlock:(id (^)(V))block;
+- (void)mapUsingBlock2:(id (^)(V))block; // expected-note{{previous definition is here}}
+@end
+
+@implementation MyTest
+- (id)test:(id)key {
+  return key;
+}
+- (int)test2:(id)key{ // expected-warning{{conflicting return type in implementation}}
+  return 0;
+}
+- (void)mapUsingBlock:(id (^)(id))block {
+}
+- (void)mapUsingBlock2:(id)block { // expected-warning{{conflicting parameter types in implementation}}
+}
+@end