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

[vsapsai]

Summary

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 ignores the updated
underlying type and remains id.

Fix by desugaring ObjCTypeParamType to the underlying type, the same
way we are doing with TypedefType.

rdar://problem/54329242

[vsapsai]

Ping.

[erik.pilkington]

I'm a bit curious about clients that use getCanonicalType() to get a full desugaring, instead of doing a single step. It seems like they'd still get the out of date type parameter type. Has that ever worked?

[vsapsai]

In D66696#1663856, @erik.pilkington wrote:

I'm a bit curious about clients that use getCanonicalType() to get a full desugaring, instead of doing a single step. It seems like they'd still get the out of date type parameter type. Has that ever worked?

Good point. I believe it never worked but don't have specific examples. Suspect that we haven't seen such problems because ObjCTypeParamType has limited usage before it is substituted with a concrete type.

While checking the code I've noticed we can make a small cleanup - in

QualType getObjCTypeParamType(const ObjCTypeParamDecl *Decl,
                              ArrayRef<ObjCProtocolDecl *> protocols,
                              QualType Canonical = QualType()) const;

remove the parameter Canonical because no caller provides it and it's not the best idea to allow to provide Canonical type that can be inconsistent with ObjCTypeParamDecl type. Plan to do the cleanup in a separate commit.

[vsapsai]

Found a use case where getCanonicalType() causes problems:

@interface NSObject
@end

@protocol SomeProtocol
@end

@interface NSString : NSObject
@end
@interface NSNumber : NSObject
@end

@interface Container<T : NSString *>
@end

@interface Container<T> (extension)
T FunctionInExtension(void);
@end

void test(Container *c) {
  NSNumber *n = FunctionInExtension();
}

In this case we don't warn about NSNumber/NSString mismatch but should. At line 8125 in Sema::CheckAssignmentConstraints we get a canonical type for the right-hand side and it turned out to be id, which is compatible with NSNumber.

I believe it's a separate issue and the currently proposed change is correct but insufficient. Prior to using ObjCTypeParamType for generic parameters, we were using TypedefType and I think we need to make ObjCTypeParamType like TypedefType in more places.

[erik.pilkington]

Okay, LGTM.

[vsapsai]

Thanks for the review.

clang/include/clang/AST/Type.h

class ObjCTypeParamType : public Type,
   }
 
   ObjCTypeParamType(const ObjCTypeParamDecl *D,
                     QualType can,
                     ArrayRef<ObjCProtocolDecl *> protocols);
 
 public:
   bool isSugared() const { return true; }
-  QualType desugar() const { return getCanonicalTypeInternal(); }
+  QualType desugar() const;
 
   static bool classof(const Type *T) {
     return T->getTypeClass() == ObjCTypeParam;
   }
 
   void Profile(llvm::FoldingSetNodeID &ID);
   static void Profile(llvm::FoldingSetNodeID &ID,
                       const ObjCTypeParamDecl *OTPDecl,

clang/lib/AST/Type.cpp

ObjCTypeParamType::ObjCTypeParamType(const ObjCTypeParamDecl *D,
     : Type(ObjCTypeParam, can, can->isDependentType(),
            can->isInstantiationDependentType(),
            can->isVariablyModifiedType(),
            /*ContainsUnexpandedParameterPack=*/false),
       OTPDecl(const_cast<ObjCTypeParamDecl*>(D)) {
   initialize(protocols);
 }
 
+QualType ObjCTypeParamType::desugar() const {
+  return getDecl()->getUnderlyingType();
+}
+
 ObjCObjectType::ObjCObjectType(QualType Canonical, QualType Base,
                                ArrayRef<QualType> typeArgs,
                                ArrayRef<ObjCProtocolDecl *> protocols,
                                bool isKindOf)
     : Type(ObjCObject, Canonical, Base->isDependentType(),
            Base->isInstantiationDependentType(),
            Base->isVariablyModifiedType(),
            Base->containsUnexpandedParameterPack()),

clang/test/SemaObjC/parameterized_classes_subst.m

 - (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
+
+// --------------------------------------------------------------------------
+// Use a type parameter as a type argument.
+// --------------------------------------------------------------------------
+// Type bounds in a category/extension are omitted. rdar://problem/54329242
+@interface ParameterizedContainer<T : id<NSCopying>>
+- (ParameterizedContainer<T> *)inInterface;
+@end
+@interface ParameterizedContainer<T> (Cat)
+- (ParameterizedContainer<T> *)inCategory;
+@end
+@interface ParameterizedContainer<U> ()
+- (ParameterizedContainer<U> *)inExtension;
+@end