diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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< diff --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp --- a/clang/lib/AST/DeclObjC.cpp +++ b/clang/lib/AST/DeclObjC.cpp @@ -956,32 +956,32 @@ ObjCMethodDecl *ObjCMethodDecl::getCanonicalDecl() { auto *CtxD = cast(getDeclContext()); + const auto &Sel = getSelector(); if (auto *ImplD = dyn_cast(CtxD)) { if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) { - if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(), - isInstanceMethod())) + // When the container is the ObjCImplementationDecl (the primary + // @implementation), then the canonical Decl is either in + // the class Interface, or in any of its extension. + // + // So when we don't find it in the ObjCInterfaceDecl, + // 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(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(CtxD)->getMethod(getSelector(), - isInstanceMethod()); + cast(CtxD)->getMethod(Sel, isInstanceMethod()); return MD ? MD : this; } diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp --- a/clang/lib/CodeGen/CGObjCMac.cpp +++ b/clang/lib/CodeGen/CGObjCMac.cpp @@ -928,7 +928,8 @@ /// \param[out] NameOut - The return value. void GetNameForMethod(const ObjCMethodDecl *OMD, const ObjCContainerDecl *CD, - SmallVectorImpl &NameOut); + SmallVectorImpl &NameOut, + bool ignoreCategoryNamespace = false); /// GetMethodVarName - Return a unique constant for the given /// selector's name. The return value has type char *. @@ -4033,7 +4034,7 @@ return I->second; SmallString<256> Name; - GetNameForMethod(OMD, CD, Name); + GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true); CodeGenTypes &Types = CGM.getTypes(); llvm::FunctionType *MethodTy = @@ -4088,9 +4089,9 @@ nullptr, true); Builder.CreateStore(result.getScalarVal(), selfAddr); - // Nullable `Class` expressions cannot be messaged with a direct method - // so the only reason why the receive can be null would be because - // of weak linking. + // Nullable `Class` expressions cannot be messaged with a direct method + // so the only reason why the receive can be null would be because + // of weak linking. ReceiverCanBeNull = isWeakLinkedClass(OID); } @@ -5687,14 +5688,16 @@ void CGObjCCommonMac::GetNameForMethod(const ObjCMethodDecl *D, const ObjCContainerDecl *CD, - SmallVectorImpl &Name) { + SmallVectorImpl &Name, + bool ignoreCategoryNamespace) { llvm::raw_svector_ostream OS(Name); assert (CD && "Missing container decl in GetNameForMethod"); OS << '\01' << (D->isInstanceMethod() ? '-' : '+') << '[' << CD->getName(); - if (const ObjCCategoryImplDecl *CID = - dyn_cast(D->getDeclContext())) - OS << '(' << *CID << ')'; + if (!ignoreCategoryNamespace) + if (const ObjCCategoryImplDecl *CID = + dyn_cast(D->getDeclContext())) + OS << '(' << *CID << ')'; OS << ' ' << D->getSelector().getAsString() << ']'; } diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp --- a/clang/lib/Sema/SemaDeclObjC.cpp +++ b/clang/lib/Sema/SemaDeclObjC.cpp @@ -4508,12 +4508,6 @@ method->getLocation())); } - if (!method->isDirectMethod()) - if (const auto *attr = prevMethod->getAttr()) { - method->addAttr( - ObjCDirectAttr::CreateImplicit(S.Context, attr->getLocation())); - } - // Merge nullability of the result type. QualType newReturnType = mergeTypeNullabilityForRedecl( @@ -4738,16 +4732,73 @@ } } + // A method is either tagged direct explicitly, or inherits it from its + // canonical declaration. + // + // We have to do the merge upfront and not in mergeInterfaceMethodToImpl() + // because IDecl->lookupMethod() returns more possible matches than just + // the canonical declaration. + if (!ObjCMethod->isDirectMethod()) { + const ObjCMethodDecl *CanonicalMD = ObjCMethod->getCanonicalDecl(); + if (const auto *attr = CanonicalMD->getAttr()) { + 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()) { - if (!IMD->isDirectMethod()) { - Diag(attr->getLocation(), diag::err_objc_direct_missing_on_decl); + + // The Idecl->lookupMethod() above will find declarations for ObjCMethod + // in one of these places: + // + // (1) the canonical declaration in an @interface container paired + // with the ImplDecl, + // (2) non canonical declarations in @interface not paired with the + // ImplDecl for the same Class, + // (3) any superclass container. + // + // Direct methods only allow for canonical declarations in the matching + // container (case 1). + // + // Direct methods overriding a superclass declaration (case 3) is + // handled during overrides checks in CheckObjCMethodOverrides(). + // + // We deal with same-class container mismatches (Case 2) here. + if (IDecl == IMD->getClassInterface()) { + auto diagContainerMismatch = [&] { + int decl = 0, impl = 0; + + if (auto *Cat = dyn_cast(IMD->getDeclContext())) + decl = Cat->IsClassExtension() ? 1 : 2; + + if (auto *Cat = dyn_cast(ImpDecl)) + impl = 1 + (decl != 0); + + Diag(ObjCMethod->getLocation(), + diag::err_objc_direct_impl_decl_mismatch) + << decl << impl; Diag(IMD->getLocation(), diag::note_previous_declaration); + }; + + if (const auto *attr = ObjCMethod->getAttr()) { + if (ObjCMethod->getCanonicalDecl() != IMD) { + diagContainerMismatch(); + } 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()) { + if (ObjCMethod->getCanonicalDecl() != IMD) { + diagContainerMismatch(); + } else { + ObjCMethod->addAttr( + ObjCDirectAttr::CreateImplicit(Context, attr->getLocation())); + } } } @@ -4779,11 +4830,42 @@ } } } else { - if (!ObjCMethod->isDirectMethod() && - ClassDecl->hasAttr()) { - ObjCMethod->addAttr( - ObjCDirectAttr::CreateImplicit(Context, ObjCMethod->getLocation())); + if (!isa(ClassDecl)) { + if (!ObjCMethod->isDirectMethod() && + ClassDecl->hasAttr()) { + 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(ClassDecl); + if (!IDecl) + IDecl = cast(ClassDecl)->getClassInterface(); + + if (IDecl) + if (auto *IMD = IDecl->lookupMethod(ObjCMethod->getSelector(), + ObjCMethod->isInstanceMethod(), + /*shallowCategoryLookup=*/false, + /*followSuper=*/false)) { + if (isa(IMD->getDeclContext())) { + // Do not emit a diagnostic for the Protocol case: + // diag::err_objc_direct_on_protocol has already been emitted + // during parsing for these with a nicer diagnostic. + } 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(ClassDecl)->addDecl(ObjCMethod); } diff --git a/clang/test/CodeGenObjC/direct-method.m b/clang/test/CodeGenObjC/direct-method.m --- a/clang/test/CodeGenObjC/direct-method.m +++ b/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 directMethodInCategory]"( +- (int)directMethodInCategory { + return 42; +} +// CHECK-LABEL: define hidden i32 @"\01-[Foo 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 directMethodInCategory]" + // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInCategoryNoDecl]" [f setGetDynamic_setDirect:1]; - return [f getDirect_setDynamic]; + return [f getDirect_setDynamic] + + [f directMethodInExtension] + + [f directMethodInCategory] + + [f directMethodInCategoryNoDecl]; } __attribute__((objc_root_class)) diff --git a/clang/test/SemaObjC/method-direct-one-definition.m b/clang/test/SemaObjC/method-direct-one-definition.m new file mode 100644 --- /dev/null +++ b/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