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 @@ -4581,6 +4581,62 @@ << (Triple.isMacOSX() ? "macOS 10.11" : "iOS 9"); } +static void mergeObjCDirectMembers(Sema &S, Decl *CD, ObjCMethodDecl *Method) { + if (!Method->isDirectMethod() && !Method->hasAttr() && + CD->hasAttr()) { + Method->addAttr( + ObjCDirectAttr::CreateImplicit(S.Context, Method->getLocation())); + } +} + +static void checkObjCDirectMethodClashes(Sema &S, ObjCInterfaceDecl *IDecl, + ObjCMethodDecl *Method, + ObjCImplDecl *ImpDecl = nullptr) { + auto Sel = Method->getSelector(); + bool isInstance = Method->isInstanceMethod(); + bool diagnosed = false; + + auto diagClash = [&](const ObjCMethodDecl *IMD) { + if (diagnosed || IMD->isImplicit()) + return; + if (Method->isDirectMethod() || IMD->isDirectMethod()) { + S.Diag(Method->getLocation(), diag::err_objc_direct_duplicate_decl) + << Method->isDirectMethod() << /* method */ 0 << IMD->isDirectMethod() + << Method->getDeclName(); + S.Diag(IMD->getLocation(), diag::note_previous_declaration); + diagnosed = true; + } + }; + + // Look for any other declaration of this method anywhere we can see in this + // compilation unit. + // + // We do not use IDecl->lookupMethod() because we have specific needs: + // + // - we absolutely do not need to walk protocols, because + // diag::err_objc_direct_on_protocol has already been emitted + // during parsing if there's a conflict, + // + // - when we do not find a match in a given @interface container, + // we need to attempt looking it up in the @implementation block if the + // translation unit sees it to find more clashes. + + if (auto *IMD = IDecl->getMethod(Sel, isInstance)) + diagClash(IMD); + else if (auto *Impl = IDecl->getImplementation()) + if (Impl != ImpDecl) + if (auto *IMD = IDecl->getImplementation()->getMethod(Sel, isInstance)) + diagClash(IMD); + + for (const auto *Cat : IDecl->visible_categories()) + if (auto *IMD = Cat->getMethod(Sel, isInstance)) + diagClash(IMD); + else if (auto CatImpl = Cat->getImplementation()) + if (CatImpl != ImpDecl) + if (auto *IMD = Cat->getMethod(Sel, isInstance)) + diagClash(IMD); +} + Decl *Sema::ActOnMethodDeclaration( Scope *S, SourceLocation MethodLoc, SourceLocation EndLoc, tok::TokenKind MethodType, ObjCDeclSpec &ReturnQT, ParsedType ReturnType, @@ -4809,9 +4865,9 @@ Diag(ObjCMethod->getLocation(), diag::warn_dealloc_in_category) << ObjCMethod->getDeclName(); } - } else if (ImpDecl->hasAttr()) { - ObjCMethod->addAttr( - ObjCDirectAttr::CreateImplicit(Context, ObjCMethod->getLocation())); + } else { + mergeObjCDirectMembers(*this, ClassDecl, ObjCMethod); + checkObjCDirectMethodClashes(*this, IDecl, ObjCMethod, ImpDecl); } // Warn if a method declared in a protocol to which a category or @@ -4832,39 +4888,16 @@ } } else { if (!isa(ClassDecl)) { - if (!ObjCMethod->isDirectMethod() && - ClassDecl->hasAttr()) { - ObjCMethod->addAttr( - ObjCDirectAttr::CreateImplicit(Context, ObjCMethod->getLocation())); - } + mergeObjCDirectMembers(*this, ClassDecl, ObjCMethod); - // 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(); - + // 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. 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() << /* method */ 0 - << IMD->isDirectMethod() << ObjCMethod->getDeclName(); - Diag(IMD->getLocation(), diag::note_previous_declaration); - } - } + checkObjCDirectMethodClashes(*this, IDecl, ObjCMethod); } cast(ClassDecl)->addDecl(ObjCMethod); diff --git a/clang/test/SemaObjC/method-direct-one-definition.m b/clang/test/SemaObjC/method-direct-one-definition.m --- a/clang/test/SemaObjC/method-direct-one-definition.m +++ b/clang/test/SemaObjC/method-direct-one-definition.m @@ -30,6 +30,15 @@ - (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}} @end +@implementation B +- (void)B_primary { +} +- (void)B_extension { +} +- (void)B_implOnly __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}} } @@ -39,6 +48,8 @@ } - (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}} } +- (void)B_implOnly __attribute__((objc_direct)) { // expected-error {{direct method declaration conflicts with previous direct declaration of method 'B_implOnly'}} +} @end __attribute__((objc_root_class)) diff --git a/clang/test/SemaObjC/method-direct.m b/clang/test/SemaObjC/method-direct.m --- a/clang/test/SemaObjC/method-direct.m +++ b/clang/test/SemaObjC/method-direct.m @@ -12,6 +12,7 @@ __attribute__((objc_root_class)) @interface Root +- (void)unavailableInChild; - (void)rootRegular; // expected-note {{previous declaration is here}} + (void)classRootRegular; // expected-note {{previous declaration is here}} - (void)rootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}}; @@ -52,6 +53,7 @@ __attribute__((objc_direct_members)) @interface SubDirectMembers : Root @property int foo; // expected-note {{previous declaration is here}} +- (void)unavailableInChild __attribute__((unavailable)); // should not warn - (instancetype)init; @end @@ -81,6 +83,8 @@ __attribute__((objc_direct_members)) @implementation Root +- (void)unavailableInChild { +} - (void)rootRegular { } + (void)classRootRegular {