MadCoder updated this revision to Diff 234684.
MadCoder added a comment.
Small update because I uploaded before the full completion of the test-suite
and somehow it's sentient and punished me... (some diagnostics were too verbose
and stuttering things with `Sema::CheckObjCMethodOverrides`).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71694/new/
https://reviews.llvm.org/D71694
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/DeclObjC.cpp
clang/lib/Sema/SemaDeclObjC.cpp
clang/test/CodeGenObjC/direct-method.m
clang/test/SemaObjC/method-direct-one-definition.m
Index: clang/test/SemaObjC/method-direct-one-definition.m
===================================================================
--- /dev/null
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-protocol-method-implementation %s
+
+__attribute__((objc_root_class))
+@interface A
+@end
+
+@interface A (Cat)
+- (void)A_Cat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation A
+- (void)A_Cat { // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface B
+- (void)B_primary __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B ()
+- (void)B_extension __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B (Cat)
+- (void)B_Cat __attribute__((objc_direct));
+@end
+
+@interface B (OtherCat)
+- (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation B (Cat)
+- (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
+}
+- (void)B_extension { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
+}
+- (void)B_Cat {
+}
+- (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface C
+- (void)C1 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+- (void)C2; // expected-note {{previous declaration is here}}
+@end
+
+@interface C (Cat)
+- (void)C1; // expected-error {{method declaration conflicts with previous direct declaration of method 'C1'}}
+- (void)C2 __attribute__((objc_direct)); // expected-error {{direct method declaration conflicts with previous declaration of method 'C2'}}
+@end
Index: clang/test/CodeGenObjC/direct-method.m
===================================================================
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -172,10 +172,19 @@
@interface Foo ()
@property(nonatomic, readwrite) int getDirect_setDynamic;
@property(nonatomic, readwrite, direct) int getDynamic_setDirect;
+- (int)directMethodInExtension __attribute__((objc_direct));
+@end
+
+@interface Foo (Cat)
+- (int)directMethodInCategory __attribute__((objc_direct));
@end
__attribute__((objc_direct_members))
@implementation Foo
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInExtension]"(
+- (int)directMethodInExtension {
+ return 42;
+}
// CHECK-LABEL: define hidden i32 @"\01-[Foo getDirect_setDynamic]"(
// CHECK-LABEL: define internal void @"\01-[Foo setGetDirect_setDynamic:]"(
// CHECK-LABEL: define internal i32 @"\01-[Foo getDynamic_setDirect]"(
@@ -183,6 +192,17 @@
// CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
@end
+@implementation Foo (Cat)
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategory]"(
+- (int)directMethodInCategory {
+ return 42;
+}
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategoryNoDecl]"(
+- (int)directMethodInCategoryNoDecl __attribute__((objc_direct)) {
+ return 42;
+}
+@end
+
int useRoot(Root *r) {
// CHECK-LABEL: define i32 @useRoot
// CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
@@ -195,8 +215,14 @@
// CHECK-LABEL: define i32 @useFoo
// CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
// CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo getDirect_setDynamic]"
+ // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInExtension]"
+ // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo(Cat) directMethodInCategory]"
+ // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo(Cat) directMethodInCategoryNoDecl]"
[f setGetDynamic_setDirect:1];
- return [f getDirect_setDynamic];
+ return [f getDirect_setDynamic] +
+ [f directMethodInExtension] +
+ [f directMethodInCategory] +
+ [f directMethodInCategoryNoDecl];
}
__attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===================================================================
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4508,12 +4508,6 @@
method->getLocation()));
}
- if (!method->isDirectMethod())
- if (const auto *attr = prevMethod->getAttr<ObjCDirectAttr>()) {
- method->addAttr(
- ObjCDirectAttr::CreateImplicit(S.Context, attr->getLocation()));
- }
-
// Merge nullability of the result type.
QualType newReturnType
= mergeTypeNullabilityForRedecl(
@@ -4738,15 +4732,60 @@
}
}
+ // Merge directness from the canonical declaration upfront
+ if (!ObjCMethod->isDirectMethod()) {
+ const ObjCMethodDecl *CanonicalMD = ObjCMethod->getCanonicalDecl();
+ if (const auto *attr = CanonicalMD->getAttr<ObjCDirectAttr>()) {
+ ObjCMethod->addAttr(
+ ObjCDirectAttr::CreateImplicit(Context, attr->getLocation()));
+ }
+ }
+
// Merge information from the @interface declaration into the
// @implementation.
if (ObjCInterfaceDecl *IDecl = ImpDecl->getClassInterface()) {
if (auto *IMD = IDecl->lookupMethod(ObjCMethod->getSelector(),
ObjCMethod->isInstanceMethod())) {
mergeInterfaceMethodToImpl(*this, ObjCMethod, IMD);
- if (const auto *attr = ObjCMethod->getAttr<ObjCDirectAttr>()) {
- if (!IMD->isDirectMethod()) {
- Diag(attr->getLocation(), diag::err_objc_direct_missing_on_decl);
+
+ if (IDecl == IMD->getClassInterface()) {
+ bool directContainerMismatch = false;
+
+ // if the match is from the same Class (not super),
+ // validate that there is no declaration/implementation
+ // mismatch for direct methods.
+ //
+ // If the method comes from super, there will be a more precise
+ // diagnostics in CheckObjCMethodOverrides.
+
+ if (const auto *attr = ObjCMethod->getAttr<ObjCDirectAttr>()) {
+ if (ObjCMethod->getCanonicalDecl() != IMD) {
+ directContainerMismatch = true;
+ } else if (!IMD->isDirectMethod()) {
+ Diag(attr->getLocation(), diag::err_objc_direct_missing_on_decl);
+ Diag(IMD->getLocation(), diag::note_previous_declaration);
+ }
+ } else if (const auto *attr = IMD->getAttr<ObjCDirectAttr>()) {
+ if (ObjCMethod->getCanonicalDecl() != IMD) {
+ directContainerMismatch = true;
+ } else {
+ ObjCMethod->addAttr(
+ ObjCDirectAttr::CreateImplicit(Context, attr->getLocation()));
+ }
+ }
+
+ if (directContainerMismatch) {
+ int decl = 0, impl = 0;
+
+ if (auto *Cat = dyn_cast<ObjCCategoryDecl>(IMD->getDeclContext()))
+ decl = Cat->IsClassExtension() ? 1 : 2;
+
+ if (auto *Cat = dyn_cast<ObjCCategoryImplDecl>(ImpDecl))
+ impl = 1 + (decl != 0);
+
+ Diag(ObjCMethod->getLocation(),
+ diag::err_objc_direct_impl_decl_mismatch)
+ << decl << impl;
Diag(IMD->getLocation(), diag::note_previous_declaration);
}
}
@@ -4779,11 +4818,41 @@
}
}
} else {
- if (!ObjCMethod->isDirectMethod() &&
- ClassDecl->hasAttr<ObjCDirectMembersAttr>()) {
- ObjCMethod->addAttr(
- ObjCDirectAttr::CreateImplicit(Context, ObjCMethod->getLocation()));
+ if (!isa<ObjCProtocolDecl>(ClassDecl)) {
+ if (!ObjCMethod->isDirectMethod() &&
+ ClassDecl->hasAttr<ObjCDirectMembersAttr>()) {
+ ObjCMethod->addAttr(
+ ObjCDirectAttr::CreateImplicit(Context, ObjCMethod->getLocation()));
+ }
+
+ // There can be a single declaration in any @interface container
+ // for a given direct method, look for clashes as we add them
+ //
+ // For valid code, we should always know the primary interface
+ // declaration by now, however for invalid code we'll keep parsing
+ // but we won't find the primary interface and IDecl will be nil.
+ ObjCInterfaceDecl *IDecl = dyn_cast<ObjCInterfaceDecl>(ClassDecl);
+ if (!IDecl)
+ IDecl = cast<ObjCCategoryDecl>(ClassDecl)->getClassInterface();
+
+ if (IDecl)
+ if (auto *IMD = IDecl->lookupMethod(ObjCMethod->getSelector(),
+ ObjCMethod->isInstanceMethod(),
+ /*shallowCategoryLookup=*/false,
+ /*followSuper=*/false)) {
+ if (isa<ObjCProtocolDecl>(IMD->getDeclContext())) {
+ // diag::err_objc_direct_on_protocol is already emitted for these
+ // with a better diagnostic, so don't do it twice
+ } else if (ObjCMethod->isDirectMethod() || IMD->isDirectMethod()) {
+ Diag(ObjCMethod->getLocation(),
+ diag::err_objc_direct_duplicate_decl)
+ << ObjCMethod->isDirectMethod() << IMD->isDirectMethod()
+ << ObjCMethod->getDeclName();
+ Diag(IMD->getLocation(), diag::note_previous_declaration);
+ }
+ }
}
+
cast<DeclContext>(ClassDecl)->addDecl(ObjCMethod);
}
Index: clang/lib/AST/DeclObjC.cpp
===================================================================
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -956,32 +956,31 @@
ObjCMethodDecl *ObjCMethodDecl::getCanonicalDecl() {
auto *CtxD = cast<Decl>(getDeclContext());
+ const auto &Sel = getSelector();
if (auto *ImplD = dyn_cast<ObjCImplementationDecl>(CtxD)) {
if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) {
- if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(),
- isInstanceMethod()))
+ // When the container is the principal @implementation,
+ // the canonical Decl is either in an @interface, or in an extension.
+ //
+ // So when we don't find it in the principal @interface,
+ // sift through extensions too.
+ if (ObjCMethodDecl *MD = IFD->getMethod(Sel, isInstanceMethod()))
return MD;
- // readwrite properties may have been re-declared in an extension.
- // look harder.
- if (isPropertyAccessor())
- for (auto *Ext : IFD->known_extensions())
- if (ObjCMethodDecl *MD =
- Ext->getMethod(getSelector(), isInstanceMethod()))
- return MD;
+ for (auto *Ext : IFD->known_extensions())
+ if (ObjCMethodDecl *MD = Ext->getMethod(Sel, isInstanceMethod()))
+ return MD;
}
} else if (auto *CImplD = dyn_cast<ObjCCategoryImplDecl>(CtxD)) {
if (ObjCCategoryDecl *CatD = CImplD->getCategoryDecl())
- if (ObjCMethodDecl *MD = CatD->getMethod(getSelector(),
- isInstanceMethod()))
+ if (ObjCMethodDecl *MD = CatD->getMethod(Sel, isInstanceMethod()))
return MD;
}
if (isRedeclaration()) {
// It is possible that we have not done deserializing the ObjCMethod yet.
ObjCMethodDecl *MD =
- cast<ObjCContainerDecl>(CtxD)->getMethod(getSelector(),
- isInstanceMethod());
+ cast<ObjCContainerDecl>(CtxD)->getMethod(Sel, isInstanceMethod());
return MD ? MD : this;
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -995,6 +995,12 @@
def err_objc_direct_on_protocol : Error<
"'objc_direct' attribute cannot be applied to %select{methods|properties}0 "
"declared in an Objective-C protocol">;
+def err_objc_direct_duplicate_decl : Error<
+ "%select{|direct }0method declaration conflicts "
+ "with previous %select{|direct }1declaration of method %2">;
+def err_objc_direct_impl_decl_mismatch : Error<
+ "direct method was declared in %select{the primary interface|an extension|a category}0 "
+ "but is implemented in %select{the primary interface|a category|a different category}1">;
def err_objc_direct_missing_on_decl : Error<
"direct method implementation was previously declared not direct">;
def err_objc_direct_on_override : Error<
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
