Objective-C [Sema]. Fixes a bug in comparing qualified

Objective-C pointer types. In this case, checker incorrectly
claims incompatible pointer types if redundant protocol conformance 
is specified. rdar://18491222

llvm-svn: 219630

3 files changed, 70 insertions, 53 deletions

diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 577e82089b8..09638916cf0 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -6742,58 +6742,40 @@ bool ASTContext::canAssignObjCInterfaces(const ObjCObjectType *LHS,
   if (LHS->getNumProtocols() == 0)
     return true;
 
-  // Okay, we know the LHS has protocol qualifiers.  If the RHS doesn't, 
-  // more detailed analysis is required.
-  if (RHS->getNumProtocols() == 0) {
-    // OK, if LHS is a superclass of RHS *and*
-    // this superclass is assignment compatible with LHS.
-    // false otherwise.
-    bool IsSuperClass = 
-      LHS->getInterface()->isSuperClassOf(RHS->getInterface());
-    if (IsSuperClass) {
-      // OK if conversion of LHS to SuperClass results in narrowing of types
-      // ; i.e., SuperClass may implement at least one of the protocols
-      // in LHS's protocol list. Example, SuperObj<P1> = lhs<P1,P2> is ok.
-      // But not SuperObj<P1,P2,P3> = lhs<P1,P2>.
-      llvm::SmallPtrSet<ObjCProtocolDecl *, 8> SuperClassInheritedProtocols;
-      CollectInheritedProtocols(RHS->getInterface(), SuperClassInheritedProtocols);
-      // If super class has no protocols, it is not a match.
-      if (SuperClassInheritedProtocols.empty())
-        return false;
+  // Okay, we know the LHS has protocol qualifiers. But RHS may or may not.
+  // More detailed analysis is required.
+  // OK, if LHS is same or a superclass of RHS *and*
+  // this LHS, or as RHS's super class is assignment compatible with LHS.
+  bool IsSuperClass =
+    LHS->getInterface()->isSuperClassOf(RHS->getInterface());
+  if (IsSuperClass) {
+    // OK if conversion of LHS to SuperClass results in narrowing of types
+    // ; i.e., SuperClass may implement at least one of the protocols
+    // in LHS's protocol list. Example, SuperObj<P1> = lhs<P1,P2> is ok.
+    // But not SuperObj<P1,P2,P3> = lhs<P1,P2>.
+    llvm::SmallPtrSet<ObjCProtocolDecl *, 8> SuperClassInheritedProtocols;
+    CollectInheritedProtocols(RHS->getInterface(), SuperClassInheritedProtocols);
+    // Also, if RHS has explicit quelifiers, include them for comparing with LHS's
+    // qualifiers.
+    for (auto *RHSPI : RHS->quals())
+      SuperClassInheritedProtocols.insert(RHSPI->getCanonicalDecl());
+    // If there is no protocols associated with RHS, it is not a match.
+    if (SuperClassInheritedProtocols.empty())
+      return false;
       
-      for (const auto *LHSProto : LHS->quals()) {
-        bool SuperImplementsProtocol = false;        
-        for (auto *SuperClassProto : SuperClassInheritedProtocols) {
-          if (SuperClassProto->lookupProtocolNamed(LHSProto->getIdentifier())) {
-            SuperImplementsProtocol = true;
-            break;
-          }
+    for (const auto *LHSProto : LHS->quals()) {
+      bool SuperImplementsProtocol = false;
+      for (auto *SuperClassProto : SuperClassInheritedProtocols)
+        if (SuperClassProto->lookupProtocolNamed(LHSProto->getIdentifier())) {
+          SuperImplementsProtocol = true;
+          break;
         }
-        if (!SuperImplementsProtocol)
-          return false;
-      }
-      return true;
-    }
-    return false;
-  }
-
-  for (const auto *LHSPI : LHS->quals()) {
-    bool RHSImplementsProtocol = false;
-
-    // If the RHS doesn't implement the protocol on the left, the types
-    // are incompatible.
-    for (auto *RHSPI : RHS->quals()) {
-      if (RHSPI->lookupProtocolNamed(LHSPI->getIdentifier())) {
-        RHSImplementsProtocol = true;
-        break;
-      }
+      if (!SuperImplementsProtocol)
+        return false;
     }
-    // FIXME: For better diagnostics, consider passing back the protocol name.
-    if (!RHSImplementsProtocol)
-      return false;
+    return true;
   }
-  // The RHS implements all protocols listed on the LHS.
-  return true;
+  return false;
 }
 
 bool ASTContext::areComparableObjCPointerTypes(QualType LHS, QualType RHS) {
diff --git a/clang/test/SemaObjC/compare-qualified-class.m b/clang/test/SemaObjC/compare-qualified-class.m
index 60ef851e1f3..a5256152144 100644
--- a/clang/test/SemaObjC/compare-qualified-class.m
+++ b/clang/test/SemaObjC/compare-qualified-class.m
@@ -28,3 +28,38 @@ int main () {
            classA == classD; // expected-warning {{comparison of distinct pointer types ('Class<SomeProtocol>' and 'Class<SomeProtocol1>')}}
 }
 
+// rdar://18491222
+@protocol NSObject @end
+
+@interface NSObject @end
+@protocol ProtocolX <NSObject>
+@end
+
+@protocol ProtocolY <NSObject>
+@end
+
+@interface ClassA : NSObject
+@end
+
+@interface ClassB : ClassA <ProtocolY, ProtocolX>
+@end
+
+@interface OtherClass : NSObject
+@property (nonatomic, copy) ClassB<ProtocolX> *aProperty;
+- (ClassA<ProtocolY> *)aMethod;
+- (ClassA<ProtocolY> *)anotherMethod;
+@end
+
+@implementation OtherClass
+- (ClassA<ProtocolY> *)aMethod {
+    // This does not work, even though ClassB subclasses from A and conforms to Y
+    // because the property type explicity adds ProtocolX conformance
+    // even though ClassB already conforms to ProtocolX
+    return self.aProperty;
+}
+- (ClassA<ProtocolY> *)anotherMethod {
+    // This works, even though all it is doing is removing an explicit
+    // protocol conformance that ClassB already conforms to
+    return (ClassB *)self.aProperty;
+}
+@end
diff --git a/clang/test/SemaObjC/conditional-expr.m b/clang/test/SemaObjC/conditional-expr.m
index d8862c584a0..71e108cce67 100644
--- a/clang/test/SemaObjC/conditional-expr.m
+++ b/clang/test/SemaObjC/conditional-expr.m
@@ -101,10 +101,10 @@ int f8(int a, A<P0> *x, A *y) {
 }
 
 void f9(int a, A<P0> *x, A<P1> *y) {
-  id l0 = (a ? x : y ); // expected-warning {{incompatible operand types ('A<P0> *' and 'A<P1> *')}}
-  A<P0> *l1 = (a ? x : y ); // expected-warning {{incompatible operand types ('A<P0> *' and 'A<P1> *')}}
-  A<P1> *l2 = (a ? x : y ); // expected-warning {{incompatible operand types ('A<P0> *' and 'A<P1> *')}}
-  [ (a ? x : y ) intProp ]; // expected-warning {{incompatible operand types ('A<P0> *' and 'A<P1> *')}}
+  id l0 = (a ? x : y );     // Ok. y is of A<P1> object type and A is qualified by P0.
+  A<P0> *l1 = (a ? x : y ); // Ok. y is of A<P1> object type and A is qualified by P0.
+  A<P1> *l2 = (a ? x : y ); // expected-warning {{incompatible pointer types initializing 'A<P1> *' with an expression of type 'A<P0> *'}}
+  (void)[ (a ? x : y ) intProp ]; // Ok. Common type is A<P0> * and P0's property intProp is accessed.
 }
 
 void f10(int a, id<P0> x, id y) {
@@ -116,5 +116,5 @@ void f11(int a, id<P0> x, id<P1> y) {
 }
 
 void f12(int a, A<P0> *x, A<P1> *y) {
-  A<P1>* l0 = (a ? x : y ); // expected-warning {{incompatible operand types ('A<P0> *' and 'A<P1> *')}}
+  A<P1>* l0 = (a ? x : y ); // expected-warning {{incompatible pointer types initializing 'A<P1> *' with an expression of type 'A<P0> *'}}
 }