egorzhdan created this revision.
egorzhdan requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116412
Files:
clang/lib/Sema/SemaObjCProperty.cpp
clang/test/SemaObjC/class-property-inheritance.m
Index: clang/test/SemaObjC/class-property-inheritance.m
===================================================================
--- /dev/null
+++ clang/test/SemaObjC/class-property-inheritance.m
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+@class MyObject;
+
+@interface TopClassWithInstanceProperty
+@property(nullable, readonly, strong) MyObject *foo;
+@end
+
+@interface SubClassWithClassProperty : TopClassWithInstanceProperty
+@property(nonnull, readonly, copy, class) MyObject *foo;
+@end
+
+@interface TopClassWithClassProperty
+@property(nullable, readonly, class) MyObject *foo;
+@end
+
+@interface SubClassWithInstanceProperty : TopClassWithClassProperty
+@property(nonnull, readonly, copy) MyObject *foo;
+@end
Index: clang/lib/Sema/SemaObjCProperty.cpp
===================================================================
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -225,43 +225,56 @@
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>()) {
- DiagnosePropertyMismatch(Res, SuperProp, Super->getIdentifier(), false);
- FoundInSuper = true;
- break;
+ // For an instance property, check consistency with the properties of
+ // superclasses.
+ if (Res->isInstanceProperty()) {
+ 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()) {
+ ObjCPropertyDecl *SuperProp = nullptr;
+ for (auto *LookupResult : Super->lookup(Res->getDeclName())) {
+ if (auto *Prop = dyn_cast<ObjCPropertyDecl>(LookupResult)) {
+ if (Prop->isInstanceProperty()) {
+ SuperProp = Prop;
+ break;
+ }
+ }
+ }
+ if (SuperProp) {
+ DiagnosePropertyMismatch(Res, SuperProp, Super->getIdentifier(),
+ false);
+ FoundInSuper = true;
+ break;
+ }
+ CurrentInterfaceDecl = Super;
}
- CurrentInterfaceDecl = Super;
- }
- if (FoundInSuper) {
- // Also compare the property against a property in our protocols.
- for (auto *P : CurrentInterfaceDecl->protocols()) {
- CheckPropertyAgainstProtocol(*this, Res, P, KnownProtos);
+ if (FoundInSuper) {
+ // Also compare the property against a property in our protocols.
+ for (auto *P : CurrentInterfaceDecl->protocols()) {
+ CheckPropertyAgainstProtocol(*this, Res, P, KnownProtos);
+ }
+ } else {
+ // Slower path: look in all protocols we referenced.
+ for (auto *P : IFace->all_referenced_protocols()) {
+ CheckPropertyAgainstProtocol(*this, Res, P, KnownProtos);
+ }
}
+ } else if (ObjCCategoryDecl *Cat = dyn_cast<ObjCCategoryDecl>(ClassDecl)) {
+ // We don't check if class extension. Because properties in class
+ // extension are meant to override some of the attributes and checking has
+ // already done when property in class extension is constructed.
+ if (!Cat->IsClassExtension())
+ for (auto *P : Cat->protocols())
+ CheckPropertyAgainstProtocol(*this, Res, P, KnownProtos);
} else {
- // Slower path: look in all protocols we referenced.
- for (auto *P : IFace->all_referenced_protocols()) {
+ ObjCProtocolDecl *Proto = cast<ObjCProtocolDecl>(ClassDecl);
+ for (auto *P : Proto->protocols())
CheckPropertyAgainstProtocol(*this, Res, P, KnownProtos);
- }
}
- } else if (ObjCCategoryDecl *Cat = dyn_cast<ObjCCategoryDecl>(ClassDecl)) {
- // We don't check if class extension. Because properties in class extension
- // are meant to override some of the attributes and checking has already done
- // when property in class extension is constructed.
- if (!Cat->IsClassExtension())
- for (auto *P : Cat->protocols())
- CheckPropertyAgainstProtocol(*this, Res, P, KnownProtos);
- } else {
- ObjCProtocolDecl *Proto = cast<ObjCProtocolDecl>(ClassDecl);
- for (auto *P : Proto->protocols())
- CheckPropertyAgainstProtocol(*this, Res, P, KnownProtos);
}
ActOnDocumentableDecl(Res);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
