From 34bdecd9a8f5572d75b0f70bb3b7da8913d7585c Mon Sep 17 00:00:00 2001 From: Jordan Rose Date: Fri, 16 Jan 2015 23:04:31 +0000 Subject: [PATCH] Suggest objc_method_family(none) for a property named -newFoo or similar. As mentioned in the previous commit, if a property (declared with @property) has a name that matches a special Objective-C method family, the getter picks up that family despite being declared by the property. The most correct way to solve this problem is to add the 'objc_method_family' attribute to the getter with an argument of 'none', which unfortunately requires an explicit declaration of the getter. This commit adds a note to the existing error (ARC) or warning (MRR) for such a poorly-named property that suggests the solution; if there's already a declaration of the getter, it even includes a fix-it. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@226339 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 9 ++++-- lib/Sema/SemaObjCProperty.cpp | 34 +++++++++++++++++++++ test/SemaObjC/arc-decls.m | 35 ++++++++++++++++++---- 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index d47d5e1eca..86ff9aee3e 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -797,6 +797,12 @@ def warn_cocoa_naming_owned_rule : Warning< "property follows Cocoa naming" " convention for returning 'owned' objects">, InGroup>; +def err_cocoa_naming_owned_rule : Error< + "property follows Cocoa naming" + " convention for returning 'owned' objects">; +def note_cocoa_naming_declare_family : Note< + "explicitly declare getter %objcinstance0 with '%1' to return an 'unowned' " + "object">; def warn_auto_synthesizing_protocol_property :Warning< "auto property synthesis will not synthesize property %0" " declared in protocol %1">, @@ -828,9 +834,6 @@ def warn_property_getter_owning_mismatch : Warning< def error_property_setter_ambiguous_use : Error< "synthesized properties %0 and %1 both claim setter %2 -" " use of this setter will cause unexpected behavior">; -def err_cocoa_naming_owned_rule : Error< - "property follows Cocoa naming" - " convention for returning 'owned' objects">; def warn_default_atomic_custom_getter_setter : Warning< "atomic by default property %0 has a user defined %select{getter|setter}1 " "(property should be marked 'atomic' if this is intended)">, diff --git a/lib/Sema/SemaObjCProperty.cpp b/lib/Sema/SemaObjCProperty.cpp index 72b6020351..f4f43360f5 100644 --- a/lib/Sema/SemaObjCProperty.cpp +++ b/lib/Sema/SemaObjCProperty.cpp @@ -19,6 +19,7 @@ #include "clang/AST/ExprObjC.h" #include "clang/Basic/SourceManager.h" #include "clang/Lex/Lexer.h" +#include "clang/Lex/Preprocessor.h" #include "clang/Sema/Initialization.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SmallString.h" @@ -1854,6 +1855,39 @@ void Sema::DiagnoseOwningPropertyGetterSynthesis(const ObjCImplementationDecl *D Diag(PD->getLocation(), diag::err_cocoa_naming_owned_rule); else Diag(PD->getLocation(), diag::warn_cocoa_naming_owned_rule); + + // Look for a getter explicitly declared alongside the property. + // If we find one, use its location for the note. + SourceLocation noteLoc = PD->getLocation(); + SourceLocation fixItLoc; + for (auto *getterRedecl : method->redecls()) { + if (getterRedecl->isImplicit()) + continue; + if (getterRedecl->getDeclContext() != PD->getDeclContext()) + continue; + noteLoc = getterRedecl->getLocation(); + fixItLoc = getterRedecl->getLocEnd(); + } + + Preprocessor &PP = getPreprocessor(); + TokenValue tokens[] = { + tok::kw___attribute, tok::l_paren, tok::l_paren, + PP.getIdentifierInfo("objc_method_family"), tok::l_paren, + PP.getIdentifierInfo("none"), tok::r_paren, + tok::r_paren, tok::r_paren + }; + StringRef spelling = "__attribute__((objc_method_family(none)))"; + StringRef macroName = PP.getLastMacroWithSpelling(noteLoc, tokens); + if (!macroName.empty()) + spelling = macroName; + + auto noteDiag = Diag(noteLoc, diag::note_cocoa_naming_declare_family) + << method->getDeclName() << spelling; + if (fixItLoc.isValid()) { + SmallString<64> fixItText(" "); + fixItText += spelling; + noteDiag << FixItHint::CreateInsertion(fixItLoc, fixItText); + } } } } diff --git a/test/SemaObjC/arc-decls.m b/test/SemaObjC/arc-decls.m index 7fcf576f70..c1c319d95e 100644 --- a/test/SemaObjC/arc-decls.m +++ b/test/SemaObjC/arc-decls.m @@ -54,13 +54,13 @@ void func() // rdar://15757510 @interface J -@property (retain) id newFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} -@property (strong) id copyBar; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} -@property (copy) id allocBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} +@property (retain) id newFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-newFoo' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}} +@property (strong) id copyBar; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-copyBar' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}} +@property (copy) id allocBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-allocBaz' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}} @property (copy, nonatomic) id new; -@property (retain) id newDFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} -@property (strong) id copyDBar; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} -@property (copy) id allocDBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} +@property (retain) id newDFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-newDFoo' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}} +@property (strong) id copyDBar; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-copyDBar' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}} +@property (copy) id allocDBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note{{explicitly declare getter '-allocDBaz' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}} @end @implementation J @@ -76,6 +76,29 @@ void func() @end +@interface MethodFamilyDiags +@property (retain) id newFoo; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} +- (id)newFoo; // expected-note {{explicitly declare getter '-newFoo' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}} + +#define OBJC_METHOD_FAMILY_NONE __attribute__((objc_method_family(none))) +- (id)newBar; // expected-note {{explicitly declare getter '-newBar' with 'OBJC_METHOD_FAMILY_NONE' to return an 'unowned' object}} +@property (retain) id newBar; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} + +@property (retain) id newBaz; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note {{explicitly declare getter '-newBaz' with 'OBJC_METHOD_FAMILY_NONE' to return an 'unowned' object}} +#undef OBJC_METHOD_FAMILY_NONE + +@property (retain, readonly) id newGarply; // expected-error {{property follows Cocoa naming convention for returning 'owned' objects}} expected-note {{explicitly declare getter '-newGarply' with '__attribute__((objc_method_family(none)))' to return an 'unowned' object}} +@end + +@interface MethodFamilyDiags (Redeclarations) +- (id)newGarply; // no note here +@end + +@implementation MethodFamilyDiags +@synthesize newGarply; +@end + + // rdar://10187884 @interface Super - (void)bar:(id)b; // expected-note {{parameter declared here}} -- 2.40.0