[Clang][Sema] Fix attribute mismatch warning for ObjC class properties

[egorzhdan]

Summary

If a class declares an instance property, and an inheritor class declares a class property with the same name, Clang Sema currently treats the latter as an overridden property, and compares the attributes of the two properties to check for a mismatch. The resulting diagnostics might be misleading, since neither of the properties actually overrides the another one.

rdar://86018435

[arphaman]

I think we would still want to diagnose any mismatches between class properties that override a class property in the superclass. do you think that would be useful? You can adjust the lookup to take that into account.

[egorzhdan]

Diagnose mismatches between class property of a class and a class property of its superclass.

[arphaman]

LGTM, Cheers!

[egorzhdan]

Fix clang-format warning

clang/include/clang/AST/DeclObjC.h

public:
 
   ObjCMethodDecl *getClassMethod(Selector Sel, bool AllowHidden = false) const {
     return getMethod(Sel, false/*isInstance*/, AllowHidden);
   }
 
   bool HasUserDeclaredSetterMethod(const ObjCPropertyDecl *P) const;
   ObjCIvarDecl *getIvarDecl(IdentifierInfo *Id) const;
 
+  ObjCPropertyDecl *getProperty(const IdentifierInfo *Id,
+                                bool IsInstance) const;
+
   ObjCPropertyDecl *
   FindPropertyDeclaration(const IdentifierInfo *PropertyId,
                           ObjCPropertyQueryKind QueryKind) const;
 
   using PropertyMap =
       llvm::DenseMap<std::pair<IdentifierInfo *, unsigned/*isClassProperty*/>,
                      ObjCPropertyDecl *>;
   using ProtocolPropertySet = llvm::SmallDenseSet<const ObjCProtocolDecl *, 8>;

clang/lib/AST/DeclObjC.cpp

ObjCPropertyDecl::getDefaultSynthIvarName(ASTContext &Ctx) const {
   SmallString<128> ivarName;
   {
     llvm::raw_svector_ostream os(ivarName);
     os << '_' << getIdentifier()->getName();
   }
   return &Ctx.Idents.get(ivarName.str());
 }
 
+ObjCPropertyDecl *ObjCContainerDecl::getProperty(const IdentifierInfo *Id,
+                                                 bool IsInstance) const {
+  for (auto *LookupResult : lookup(Id)) {
+    if (auto *Prop = dyn_cast<ObjCPropertyDecl>(LookupResult)) {
+      if (Prop->isInstanceProperty() == IsInstance) {
+        return Prop;
+      }
+    }
+  }
+  return nullptr;
+}
+
 /// FindPropertyDeclaration - Finds declaration of the property given its name
 /// in 'PropertyId' and returns it. It returns 0, if not found.
 ObjCPropertyDecl *ObjCContainerDecl::FindPropertyDeclaration(
     const IdentifierInfo *PropertyId,
     ObjCPropertyQueryKind QueryKind) const {
   // Don't find properties within hidden protocol definitions.
   if (const auto *Proto = dyn_cast<ObjCProtocolDecl>(this)) {
     if (const ObjCProtocolDecl *Def = Proto->getDefinition())

clang/lib/Sema/SemaObjCProperty.cpp

 CheckPropertyAgainstProtocol(Sema &S, ObjCPropertyDecl *Prop,
                              ObjCProtocolDecl *Proto,
                              llvm::SmallPtrSetImpl<ObjCProtocolDecl *> &Known) {
   // Have we seen this protocol before?
   if (!Known.insert(Proto).second)
     return;
 
   // Look for a property with the same name.
-  if (ObjCPropertyDecl *ProtoProp =
-      Proto->lookup(Prop->getDeclName()).find_first<ObjCPropertyDecl>()) {
+  if (ObjCPropertyDecl *ProtoProp = Proto->getProperty(
+          Prop->getIdentifier(), Prop->isInstanceProperty())) {
     S.DiagnosePropertyMismatch(Prop, ProtoProp, Proto->getIdentifier(), true);
     return;
   }
 
   // Check this property against any protocols we inherit.
   for (auto *P : Proto->protocols())
     CheckPropertyAgainstProtocol(S, Prop, P, Known);
 }
if (Res->getType().getObjCLifetime())
     checkPropertyDeclWithOwnership(*this, Res);
 
   llvm::SmallPtrSet<ObjCProtocolDecl *, 16> KnownProtos;
   if (ObjCInterfaceDecl *IFace = dyn_cast<ObjCInterfaceDecl>(ClassDecl)) {
     // For a class, compare the property against a property in our superclass.
     bool FoundInSuper = false;
     ObjCInterfaceDecl *CurrentInterfaceDecl = IFace;
     while (ObjCInterfaceDecl *Super = CurrentInterfaceDecl->getSuperClass()) {
-      if (ObjCPropertyDecl *SuperProp =
-          Super->lookup(Res->getDeclName()).find_first<ObjCPropertyDecl>()) {
+      if (ObjCPropertyDecl *SuperProp = Super->getProperty(
+              Res->getIdentifier(), Res->isInstanceProperty())) {
         DiagnosePropertyMismatch(Res, SuperProp, Super->getIdentifier(), false);
         FoundInSuper = true;
         break;
       }
       CurrentInterfaceDecl = Super;
     }
 
     if (FoundInSuper) {

clang/test/SemaObjC/class-property-inheritance.m

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+@class MyObject;
+
+
+@interface TopClassWithClassProperty0
+@property(nullable, readonly, strong, class) MyObject *foo;
+@end
+
+@interface SubClassWithClassProperty0 : TopClassWithClassProperty0
+@property(nonnull, readonly, copy, class) MyObject *foo; // expected-warning {{'copy' attribute on property 'foo' does not match the property inherited from 'TopClassWithClassProperty0'}}
+@end
+
+
+
+@interface TopClassWithInstanceProperty1
+@property(nullable, readonly, strong) MyObject *foo;
+@end
+
+@interface ClassWithClassProperty1 : TopClassWithInstanceProperty1
+@property(nonnull, readonly, copy, class) MyObject *foo; // no-warning
+@end
+
+@interface SubClassWithInstanceProperty1 : ClassWithClassProperty1
+@property(nullable, readonly, copy) MyObject *foo; // expected-warning {{'copy' attribute on property 'foo' does not match the property inherited from 'TopClassWithInstanceProperty1'}}
+@end
+
+
+@interface TopClassWithClassProperty2
+@property(nullable, readonly, strong, class) MyObject *foo;
+@end
+
+@interface ClassWithInstanceProperty2 : TopClassWithClassProperty2
+@property(nonnull, readonly, copy) MyObject *foo; // no-warning
+@end
+
+@interface SubClassWithClassProperty2 : ClassWithInstanceProperty2
+@property(nonnull, readonly, copy, class) MyObject *foo; // expected-warning {{'copy' attribute on property 'foo' does not match the property inherited from 'TopClassWithClassProperty2'}}
+@end