This revision was automatically updated to reflect the committed changes.
Closed by commit rG42f9d0c0bee3: [objc_direct] Tigthen checks for direct
methods (authored by MadCoder).
Repository:
rG LLVM Github Monorepo
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/CodeGen/CGObjCMac.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 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))
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,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<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);
+
+ // 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<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);
+ };
+
+ if (const auto *attr = ObjCMethod->getAttr<ObjCDirectAttr>()) {
+ 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<ObjCDirectAttr>()) {
+ if (ObjCMethod->getCanonicalDecl() != IMD) {
+ diagContainerMismatch();
+ } else {
+ ObjCMethod->addAttr(
+ ObjCDirectAttr::CreateImplicit(Context, attr->getLocation()));
+ }
}
}
@@ -4779,11 +4830,42 @@
}
}
} 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())) {
+ // 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<DeclContext>(ClassDecl)->addDecl(ObjCMethod);
}
Index: clang/lib/CodeGen/CGObjCMac.cpp
===================================================================
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -928,7 +928,8 @@
/// \param[out] NameOut - The return value.
void GetNameForMethod(const ObjCMethodDecl *OMD,
const ObjCContainerDecl *CD,
- SmallVectorImpl<char> &NameOut);
+ SmallVectorImpl<char> &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<char> &Name) {
+ SmallVectorImpl<char> &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<ObjCCategoryImplDecl>(D->getDeclContext()))
- OS << '(' << *CID << ')';
+ if (!ignoreCategoryNamespace)
+ if (const ObjCCategoryImplDecl *CID =
+ dyn_cast<ObjCCategoryImplDecl>(D->getDeclContext()))
+ OS << '(' << *CID << ')';
OS << ' ' << D->getSelector().getAsString() << ']';
}
Index: clang/lib/AST/DeclObjC.cpp
===================================================================
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -956,32 +956,32 @@
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 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<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
