I agree. We need to make the diagnostic clearly indicate why this is a problem, instead of being just clinical. Essentially, the user won't get the intended behavior.
On Jun 19, 2012, at 6:24 PM, Jordan Rose <[email protected]> wrote: > I'd prefer to make the error message a little clearer. The mistake isn't that > the property is the same as the ivar, it's that the property /won't/ use the > ivar as its backing store. Something along the lines of: > > "autosynthesized property %1 will use %select{|synthesized}2 instance > variable %3, not existing instance variable %4" > > (Also, there's a typo in the original warning…double "auto".) > > > On Jun 19, 2012, at 15:51 , Fariborz Jahanian <[email protected]> wrote: > >> Author: fjahanian >> Date: Tue Jun 19 17:51:22 2012 >> New Revision: 158756 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=158756&view=rev >> Log: >> objective-c: warn when autosynthesizing a property which has same >> name as an existing ivar since this is common source of error >> when people remove @synthesize to take advantage of autosynthesis. >> // rdar://11671080 >> >> Modified: >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/lib/Sema/SemaObjCProperty.cpp >> cfe/trunk/test/SemaObjC/default-synthesize-2.m >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=158756&r1=158755&r2=158756&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jun 19 17:51:22 >> 2012 >> @@ -630,6 +630,9 @@ >> "auto property synthesis will not synthesize property" >> " declared in a protocol">, >> InGroup<DiagGroup<"objc-protocol-property-synthesis">>; >> +def warn_autosynthesis_property_ivar_match :Warning< >> + "auto autosynthesized property has same name as an existing ivar">, >> + InGroup<DiagGroup<"objc-autosynthesis-property-ivar-name-match">>; >> def warn_missing_explicit_synthesis : Warning < >> "auto property synthesis is synthesizing property not explicitly >> synthesized">, >> InGroup<DiagGroup<"objc-missing-property-synthesis">>, DefaultIgnore; >> >> Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=158756&r1=158755&r2=158756&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Tue Jun 19 17:51:22 2012 >> @@ -755,6 +755,22 @@ >> } >> >> if (!Ivar) { >> + if (AtLoc.isInvalid()) { >> + // Check when default synthesizing a property that there is >> + // an ivar matching property name and issue warning; since this >> + // is the most common case of not using an ivar used for backing >> + // property in non-default synthesis case. >> + ObjCInterfaceDecl *ClassDeclared=0; >> + ObjCIvarDecl *originalIvar = >> + IDecl->lookupInstanceVariable(property->getIdentifier(), >> + ClassDeclared); >> + if (originalIvar) { >> + Diag(PropertyDiagLoc, >> + diag::warn_autosynthesis_property_ivar_match); >> + Diag(property->getLocation(), diag::note_property_declare); >> + Diag(originalIvar->getLocation(), diag::note_ivar_decl); >> + } >> + } >> // In ARC, give the ivar a lifetime qualifier based on the >> // property attributes. >> if (getLangOpts().ObjCAutoRefCount && >> >> Modified: cfe/trunk/test/SemaObjC/default-synthesize-2.m >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/default-synthesize-2.m?rev=158756&r1=158755&r2=158756&view=diff >> ============================================================================== >> --- cfe/trunk/test/SemaObjC/default-synthesize-2.m (original) >> +++ cfe/trunk/test/SemaObjC/default-synthesize-2.m Tue Jun 19 17:51:22 2012 >> @@ -41,12 +41,13 @@ >> // Test3 >> @interface Test3 >> { >> - id uid; >> + id uid; // expected-note {{ivar is declared here}} >> } >> -@property (readwrite, assign) id uid; >> +@property (readwrite, assign) id uid; // expected-note {{property declared >> here}} >> @end >> >> -@implementation Test3 >> +// rdar://11671080 >> +@implementation Test3 // expected-warning {{auto autosynthesized property >> has same name as an existing ivar}} >> // Oops, forgot to write @synthesize! will be default synthesized >> - (void) myMethod { >> self.uid = 0; // Use of the âsetterâ >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
