MadCoder created this revision.
MadCoder added reviewers: rjmccall, dexonsmith, erik.pilkington, ahatanak,
arphaman.
MadCoder added a project: clang.
Herald added a subscriber: cfe-commits.
Because the name of a direct method must be agreed upon by the caller and the
implementation, certain bad practices that one can get away with when using
dynamism are fatal with direct methods.
To avoid really weird and unscruttable linker error, tighten the front-end
error reporting.
Rule 1:
Direct methods can only have at most one declaration in an @interface
container. Any redeclaration is strictly forbidden.
Today some amount of redeclaration is tolerated between the main interface
and categories for dynamic methods, but we can't have that.
Rule 2:
Direct method implementations can only be declared in a matching @interface
container: when implemented in the primary @implementation then the declaration
must be in the primary @interface or an extension, and when implemented in a
category, the declaration must be in the @interface for the same category.
Also fix another issue with ObjCMethod::getCanonicalDecl(): when an
implementation lives in the primary @interface, then its canonical declaration
can be in any extension, even when it's not an accessor.
Add Sema tests to cover the new errors, and CG tests to beef up testing around
function names for categories and extensions.
Radar-Id: rdar://problem/58054563
Repository:
rG LLVM Github Monorepo
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,17 +4732,52 @@
}
}
+ // 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);
+
+ bool directContainerMismatch = false;
if (const auto *attr = ObjCMethod->getAttr<ObjCDirectAttr>()) {
- if (!IMD->isDirectMethod()) {
+ 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);
}
// Warn about defining -dealloc in a category.
@@ -4779,11 +4808,35 @@
}
}
} 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())) {
+ 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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
