[cfe-commits] r158756 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaObjCProperty.cpp test/SemaObjC/default-synthesize-2.m

[Ted Kremenek]

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 <jordan_rose at apple.com> 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 <fjahanian at apple.com> 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}}
>> } 
>> - at property (readwrite, assign) id uid; 
>> + at property (readwrite, assign) id uid;  // expected-note {{property declared here}}
>> @end
>> 
>> - at implementation Test3
>> +// rdar://11671080
>> + at 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
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits