[objc_direct] Small updates to help with adoption.

Add fixits for messaging self in MRR or using super, as the intent is
clear, and it turns out people do that a lot more than expected.

Allow for objc_direct_members on main interfaces, it's extremely useful
for internal only classes, and proves to be quite annoying for adoption.

Add some better warnings around properties direct/non-direct clashes (it
was done for methods but properties were a miss).

Radar-Id: rdar://problem/58355212
Signed-off-by: Pierre Habouzit <phabouzit@apple.com>
This commit is contained in:
Pierre Habouzit 2020-01-30 16:48:11 -08:00
parent 6eb969b7c5
commit bebb8e2596
10 changed files with 383 additions and 28 deletions

View File

@ -1905,7 +1905,7 @@ def ObjCDirect : Attr {
def ObjCDirectMembers : Attr {
let Spellings = [Clang<"objc_direct_members">];
let Subjects = SubjectList<[ObjCImpl, ObjCCategory], ErrorDiag>;
let Subjects = SubjectList<[ObjCImpl, ObjCInterface, ObjCCategory], ErrorDiag>;
let LangOpts = [ObjC];
let Documentation = [ObjCDirectMembersDocs];
}

View File

@ -4118,7 +4118,7 @@ documentation for more information.
def ObjCDirectMembersDocs : Documentation {
let Category = DocCatDecl;
let Content = [{
The ``objc_direct_members`` attribute can be placed on an Objective-C
The ``objc_direct_members`` attribute can be placed on an Objective-C
``@interface`` or ``@implementation`` to mark that methods declared
therein should be considered direct by default. See the documentation
for ``objc_direct`` for more information about direct methods.
@ -4127,9 +4127,7 @@ When ``objc_direct_members`` is placed on an ``@interface`` block, every
method in the block is considered to be declared as direct. This includes any
implicit method declarations introduced by property declarations. If the method
redeclares a non-direct method, the declaration is ill-formed, exactly as if the
method was annotated with the ``objc_direct`` attribute. ``objc_direct_members``
cannot be placed on the primary interface of a class, only on category or class
extension interfaces.
method was annotated with the ``objc_direct`` attribute.
When ``objc_direct_members`` is placed on an ``@implementation`` block,
methods defined in the block are considered to be declared as direct unless

View File

@ -1002,8 +1002,8 @@ 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">;
"%select{|direct }0%select{method|property}1 declaration conflicts "
"with previous %select{|direct }2declaration of method %3">;
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">;

View File

@ -4859,8 +4859,8 @@ Decl *Sema::ActOnMethodDeclaration(
} else if (ObjCMethod->isDirectMethod() || IMD->isDirectMethod()) {
Diag(ObjCMethod->getLocation(),
diag::err_objc_direct_duplicate_decl)
<< ObjCMethod->isDirectMethod() << IMD->isDirectMethod()
<< ObjCMethod->getDeclName();
<< ObjCMethod->isDirectMethod() << /* method */ 0
<< IMD->isDirectMethod() << ObjCMethod->getDeclName();
Diag(IMD->getLocation(), diag::note_previous_declaration);
}
}

View File

@ -2570,6 +2570,16 @@ ExprResult Sema::BuildClassMessage(TypeSourceInfo *ReceiverTypeInfo,
diag::err_illegal_message_expr_incomplete_type))
return ExprError();
if (Method && Method->isDirectMethod() && SuperLoc.isValid()) {
Diag(SuperLoc, diag::err_messaging_super_with_direct_method)
<< FixItHint::CreateReplacement(
SuperLoc, getLangOpts().ObjCAutoRefCount
? "self"
: Method->getClassInterface()->getName());
Diag(Method->getLocation(), diag::note_direct_method_declared_at)
<< Method->getDeclName();
}
// Warn about explicit call of +initialize on its own class. But not on 'super'.
if (Method && Method->getMethodFamily() == OMF_initialize) {
if (!SuperLoc.isValid()) {
@ -2774,9 +2784,7 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver,
ReceiverType->isIntegerType())) {
// Implicitly convert integers and pointers to 'id' but emit a warning.
// But not in ARC.
Diag(Loc, diag::warn_bad_receiver_type)
<< ReceiverType
<< Receiver->getSourceRange();
Diag(Loc, diag::warn_bad_receiver_type) << ReceiverType << RecRange;
if (ReceiverType->isPointerType()) {
Receiver = ImpCastExprToType(Receiver, Context.getObjCIdType(),
CK_CPointerToObjCPointerCast).get();
@ -2927,11 +2935,10 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver,
// definition is found in a module that's not visible.
const ObjCInterfaceDecl *forwardClass = nullptr;
if (RequireCompleteType(Loc, OCIType->getPointeeType(),
getLangOpts().ObjCAutoRefCount
? diag::err_arc_receiver_forward_instance
: diag::warn_receiver_forward_instance,
Receiver? Receiver->getSourceRange()
: SourceRange(SuperLoc))) {
getLangOpts().ObjCAutoRefCount
? diag::err_arc_receiver_forward_instance
: diag::warn_receiver_forward_instance,
RecRange)) {
if (getLangOpts().ObjCAutoRefCount)
return ExprError();
@ -2993,8 +3000,7 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver,
return ExprError();
} else {
// Reject other random receiver types (e.g. structs).
Diag(Loc, diag::err_bad_receiver_type)
<< ReceiverType << Receiver->getSourceRange();
Diag(Loc, diag::err_bad_receiver_type) << ReceiverType << RecRange;
return ExprError();
}
}
@ -3017,14 +3023,27 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver,
// is what we think it is, so we reject it.
if (ReceiverType->isObjCClassType() && !isImplicit &&
!(Receiver->isObjCSelfExpr() && getLangOpts().ObjCAutoRefCount)) {
Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
DiagnosticBuilder Builder = Diag(
Receiver->getExprLoc(), diag::err_messaging_class_with_direct_method);
if (Receiver->isObjCSelfExpr()) {
Builder.AddFixItHint(FixItHint::CreateReplacement(
RecRange, Method->getClassInterface()->getName()));
}
Builder.~DiagnosticBuilder();
Diag(Method->getLocation(), diag::note_direct_method_declared_at)
<< Method->getDeclName();
}
if (SuperLoc.isValid()) {
Diag(SuperLoc, diag::err_messaging_super_with_direct_method);
DiagnosticBuilder Builder =
Diag(SuperLoc, diag::err_messaging_super_with_direct_method);
if (ReceiverType->isObjCClassType()) {
Builder.AddFixItHint(FixItHint::CreateReplacement(
SuperLoc, Method->getClassInterface()->getName()));
} else {
Builder.AddFixItHint(FixItHint::CreateReplacement(SuperLoc, "self"));
}
Builder.~DiagnosticBuilder();
Diag(Method->getLocation(), diag::note_direct_method_declared_at)
<< Method->getDeclName();
}

View File

@ -2421,6 +2421,40 @@ void Sema::ProcessPropertyDecl(ObjCPropertyDecl *property) {
DiagnosePropertyAccessorMismatch(property, GetterMethod,
property->getLocation());
// synthesizing accessors must not result in a direct method that is not
// monomorphic
if (!GetterMethod) {
if (const ObjCCategoryDecl *CatDecl = dyn_cast<ObjCCategoryDecl>(CD)) {
auto *ExistingGetter = CatDecl->getClassInterface()->lookupMethod(
property->getGetterName(), !IsClassProperty, true, false, CatDecl);
if (ExistingGetter) {
if (ExistingGetter->isDirectMethod() || property->isDirectProperty()) {
Diag(property->getLocation(), diag::err_objc_direct_duplicate_decl)
<< property->isDirectProperty() << 1 /* property */
<< ExistingGetter->isDirectMethod()
<< ExistingGetter->getDeclName();
Diag(ExistingGetter->getLocation(), diag::note_previous_declaration);
}
}
}
}
if (!property->isReadOnly() && !SetterMethod) {
if (const ObjCCategoryDecl *CatDecl = dyn_cast<ObjCCategoryDecl>(CD)) {
auto *ExistingSetter = CatDecl->getClassInterface()->lookupMethod(
property->getSetterName(), !IsClassProperty, true, false, CatDecl);
if (ExistingSetter) {
if (ExistingSetter->isDirectMethod() || property->isDirectProperty()) {
Diag(property->getLocation(), diag::err_objc_direct_duplicate_decl)
<< property->isDirectProperty() << 1 /* property */
<< ExistingSetter->isDirectMethod()
<< ExistingSetter->getDeclName();
Diag(ExistingSetter->getLocation(), diag::note_previous_declaration);
}
}
}
}
if (!property->isReadOnly() && SetterMethod) {
if (Context.getCanonicalType(SetterMethod->getReturnType()) !=
Context.VoidTy)

View File

@ -0,0 +1,30 @@
// Objective-C recovery
// RUN: not %clang_cc1 -triple x86_64-apple-darwin10 -fdiagnostics-parseable-fixits -x objective-c %s 2>&1 | FileCheck -check-prefix=CHECK-MRR %s
// RUN: not %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-arc -fdiagnostics-parseable-fixits -x objective-c %s 2>&1 | FileCheck -check-prefix=CHECK-ARC %s
__attribute__((objc_root_class))
@interface Root
+ (void)classDirectMethod __attribute__((objc_direct));
+ (void)classDirectMethod2 __attribute__((objc_direct));
- (void)instanceDirectMethod __attribute__((objc_direct));
@end
@interface A : Root
@end
@implementation A
+ (void)classMethod {
// CHECK-MRR: {18:4-18:8}:"Root"
[self classDirectMethod];
}
+ (void)classMethod2 {
// CHECK-MRR: {23:4-23:9}:"Root"
// CHECK-ARC: {23:4-23:9}:"self"
[super classDirectMethod2];
}
- (void)instanceMethod {
// CHECK-MRR: {28:4-28:9}:"self"
// CHECK-ARC: {28:4-28:9}:"self"
[super instanceDirectMethod];
}
@end

View File

@ -107,7 +107,7 @@
// CHECK-NEXT: ObjCClassStub (SubjectMatchRule_objc_interface)
// CHECK-NEXT: ObjCDesignatedInitializer (SubjectMatchRule_objc_method)
// CHECK-NEXT: ObjCDirect (SubjectMatchRule_objc_method)
// CHECK-NEXT: ObjCDirectMembers (SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_category)
// CHECK-NEXT: ObjCDirectMembers (SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_category)
// CHECK-NEXT: ObjCException (SubjectMatchRule_objc_interface)
// CHECK-NEXT: ObjCExplicitProtocolImpl (SubjectMatchRule_objc_protocol)
// CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)

View File

@ -0,0 +1,273 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
__attribute__((objc_root_class))
@interface Inteface_Implementation
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal;
@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly, direct) int direct_direct;
@end
@implementation Inteface_Implementation
- (int)normal_normal {
return 42;
}
- (int)direct_normal {
return 42;
}
- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method implementation was previously declared not direct}}
return 42;
}
- (int)direct_direct __attribute__((objc_direct)) {
return 42;
}
@end
__attribute__((objc_root_class))
@interface Inteface_Extension
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal;
@property(nonatomic, readonly) int normal_direct;
@property(nonatomic, readonly, direct) int direct_direct;
@end
@interface Inteface_Extension ()
@property(nonatomic, readwrite) int normal_normal;
@property(nonatomic, readwrite) int direct_normal;
@property(nonatomic, readwrite, direct) int normal_direct;
@property(nonatomic, readwrite, direct) int direct_direct;
@end
@implementation Inteface_Extension
@end
__attribute__((objc_root_class))
@interface Extension_Implementation
@end
@interface Extension_Implementation ()
@property(nonatomic, readwrite) int normal_normal;
@property(nonatomic, readwrite, direct) int direct_normal;
@property(nonatomic, readwrite) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readwrite, direct) int direct_direct;
@end
@implementation Extension_Implementation
- (int)normal_normal {
return 42;
}
- (int)direct_normal {
return 42;
}
- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method implementation was previously declared not direct}}
return 42;
}
- (int)direct_direct __attribute__((objc_direct)) {
return 42;
}
@end
__attribute__((objc_root_class))
@interface Inteface_Category
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-note {{previous declaration is here}}
@end
@interface Inteface_Category (SomeCategory)
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly) int direct_normal; // expected-error {{property declaration conflicts with previous direct declaration of method 'direct_normal'}}
@property(nonatomic, readonly, direct) int normal_direct; // expected-error {{direct property declaration conflicts with previous declaration of method 'normal_direct'}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-error {{direct property declaration conflicts with previous direct declaration of method 'direct_direct'}}
@end
@implementation Inteface_Category
@end
__attribute__((objc_root_class))
@interface Extension_Category
@end
@interface Extension_Category ()
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-note {{previous declaration is here}}
@end
@interface Extension_Category (SomeCategory)
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly) int direct_normal; // expected-error {{property declaration conflicts with previous direct declaration of method 'direct_normal'}}
@property(nonatomic, readonly, direct) int normal_direct; // expected-error {{direct property declaration conflicts with previous declaration of method 'normal_direct'}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-error {{direct property declaration conflicts with previous direct declaration of method 'direct_direct'}}
@end
@implementation Extension_Category
@end
__attribute__((objc_root_class))
@interface Implementation_Category
@end
@interface Implementation_Category (SomeCategory)
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-note {{previous declaration is here}}
@end
@implementation Implementation_Category
- (int)normal_normal {
return 42;
}
- (int)direct_normal { // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
return 42;
}
- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
return 42;
}
- (int)direct_direct __attribute__((objc_direct)) { // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
return 42;
}
@end
__attribute__((objc_root_class))
@interface Category_Category
@end
@interface Category_Category (SomeCategory)
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-note {{previous declaration is here}}
@end
@interface Category_Category (SomeOtherCategory)
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly) int direct_normal; // expected-error {{property declaration conflicts with previous direct declaration of method 'direct_normal'}}
@property(nonatomic, readonly, direct) int normal_direct; // expected-error {{direct property declaration conflicts with previous declaration of method 'normal_direct'}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-error {{direct property declaration conflicts with previous direct declaration of method 'direct_direct'}}
@end
@implementation Category_Category
@end
__attribute__((objc_root_class))
@interface Category_CategoryImplementation
@end
@interface Category_CategoryImplementation (SomeCategory)
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal;
@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly, direct) int direct_direct;
@end
@implementation Category_CategoryImplementation (SomeCategory)
- (int)normal_normal {
return 42;
}
- (int)direct_normal {
return 42;
}
- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method implementation was previously declared not direct}}
return 42;
}
- (int)direct_direct __attribute__((objc_direct)) {
return 42;
}
@end
@implementation Category_CategoryImplementation
@end
__attribute__((objc_root_class))
@interface Interface_CategoryImplementation
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-note {{previous declaration is here}}
@end
@interface Interface_CategoryImplementation (SomeCategory)
@end
@implementation Interface_CategoryImplementation (SomeCategory)
- (int)normal_normal {
return 42;
}
- (int)direct_normal { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
return 42;
}
- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
return 42;
}
- (int)direct_direct __attribute__((objc_direct)) { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
return 42;
}
@end
@implementation Interface_CategoryImplementation
@end
__attribute__((objc_root_class))
@interface Extension_CategoryImplementation
@end
@interface Extension_CategoryImplementation ()
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-note {{previous declaration is here}}
@end
@interface Extension_CategoryImplementation (SomeCategory)
@end
@implementation Extension_CategoryImplementation (SomeCategory)
- (int)normal_normal {
return 42;
}
- (int)direct_normal { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
return 42;
}
- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
return 42;
}
- (int)direct_direct __attribute__((objc_direct)) { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
return 42;
}
@end
__attribute__((objc_root_class))
@interface OtherCategory_CategoryImplementation
@end
@interface OtherCategory_CategoryImplementation (SomeCategory)
@end
@interface OtherCategory_CategoryImplementation (SomeOtherCategory)
@property(nonatomic, readonly) int normal_normal;
@property(nonatomic, readonly, direct) int direct_normal; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
@property(nonatomic, readonly, direct) int direct_direct; // expected-note {{previous declaration is here}}
@end
@implementation OtherCategory_CategoryImplementation (SomeCategory)
- (int)normal_normal {
return 42;
}
- (int)direct_normal { // expected-error {{direct method was declared in a category but is implemented in a different category}}
return 42;
}
- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method was declared in a category but is implemented in a different category}}
return 42;
}
- (int)direct_direct __attribute__((objc_direct)) { // expected-error {{direct method was declared in a category but is implemented in a different category}}
return 42;
}
@end
@implementation OtherCategory_CategoryImplementation
@end

View File

@ -18,6 +18,7 @@ __attribute__((objc_root_class))
+ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
- (void)otherRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherRootDirect' declared here}}
+ (void)otherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherClassRootDirect' declared here}}
+ (void)otherOtherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherOtherClassRootDirect' declared here}}
- (void)notDirectInIface; // expected-note {{previous declaration is here}}
+ (void)classNotDirectInIface; // expected-note {{previous declaration is here}}
@end
@ -48,11 +49,6 @@ __attribute__((objc_direct_members))
+ (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
@end
__attribute__((objc_root_class, objc_direct_members)) // expected-error {{'objc_direct_members' attribute only applies to Objective-C implementation declarations and Objective-C containers}}
@interface SubDirectFail : Root
- (instancetype)init;
@end
@interface Sub : Root <Proto>
/* invalid overrides with directs */
- (void)rootRegular __attribute__((objc_direct)); // expected-error {{methods that override superclass methods cannot be direct}}
@ -94,6 +90,8 @@ __attribute__((objc_direct_members))
+ (void)otherClassRootDirect {
[self someRootDirectMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
}
+ (void)otherOtherClassRootDirect {
}
- (void)rootExtensionDirect {
}
+ (void)classRootExtensionDirect {
@ -135,6 +133,9 @@ __attribute__((objc_direct_members))
- (void)someValidSubMethod {
[super otherRootDirect]; // expected-error {{messaging super with a direct method}}
}
+ (void)someValidSubMethod {
[super otherOtherClassRootDirect]; // expected-error {{messaging super with a direct method}}
}
@end
extern void callMethod(id obj, Class cls);