[cfe-commits] r124609 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaObjCProperty.cpp test/SemaObjC/custom-atomic-property.m
Argyrios Kyrtzidis
kyrtzidis at apple.com
Mon Jan 31 15:26:10 PST 2011
On Jan 31, 2011, at 1:49 PM, jahanian wrote:
>
> On Jan 31, 2011, at 1:34 PM, Argyrios Kyrtzidis wrote:
>
>> Author: akirtzidis
>> Date: Mon Jan 31 15:34:11 2011
>> New Revision: 124609
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=124609&view=rev
>> Log:
>> Add -Wcustom-atomic-properties which warns if an atomic-by-default property has custom getter or setter.
>>
>> The rationale is that it is highly likely that the user's getter/setter isn't atomically implemented. Off by default.
>> Addresses rdar://8782645.
>>
>> -Wcustom-atomic-properties and -Wimplicit-atomic-properties are under the -Watomic-properties group.
>>
>> Added:
>> cfe/trunk/test/SemaObjC/custom-atomic-property.m
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/lib/Sema/SemaObjCProperty.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=124609&r1=124608&r2=124609&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Jan 31 15:34:11 2011
>> @@ -151,6 +151,9 @@
>> def Reorder : DiagGroup<"reorder">;
>> def UndeclaredSelector : DiagGroup<"undeclared-selector">;
>> def ImplicitAtomic : DiagGroup<"implicit-atomic-properties">;
>> +def CustomAtomic : DiagGroup<"custom-atomic-properties">;
>> +def AtomicProperties : DiagGroup<"atomic-properties",
>> + [ImplicitAtomic, CustomAtomic]>;
>> def Selector : DiagGroup<"selector">;
>> def NonfragileAbi2 : DiagGroup<"nonfragile-abi2">;
>> def Protocol : DiagGroup<"protocol">;
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=124609&r1=124608&r2=124609&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jan 31 15:34:11 2011
>> @@ -386,6 +386,10 @@
>> def warn_atomic_property_rule : Warning<
>> "writable atomic property %0 cannot pair a synthesized setter/getter "
>> "with a user defined setter/getter">;
>> +def warn_default_atomic_custom_getter_setter : Warning<
>> + "atomic by default property %0 has a user defined setter/getter "
>> + "(property should be marked 'atomic' if this is intended)">,
>> + InGroup<CustomAtomic>, DefaultIgnore;
>> def err_use_continuation_class : Error<
>> "illegal redeclaration of property in continuation class %0"
>> " (attribute must be 'readwrite', while its primary must be 'readonly')">;
>>
>> Modified: cfe/trunk/lib/Sema/SemaObjCProperty.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaObjCProperty.cpp?rev=124609&r1=124608&r2=124609&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaObjCProperty.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaObjCProperty.cpp Mon Jan 31 15:34:11 2011
>> @@ -1081,7 +1081,32 @@
>> E = IDecl->prop_end();
>> I != E; ++I) {
>> ObjCPropertyDecl *Property = (*I);
>> + ObjCMethodDecl *GetterMethod = 0;
>> + ObjCMethodDecl *SetterMethod = 0;
>> + bool LookedUpGetterSetter = false;
>> +
>> unsigned Attributes = Property->getPropertyAttributes();
>> + unsigned AttributesAsWrittern = Property->getPropertyAttributesAsWritten();
>> +
>> + if (!(AttributesAsWrittern & ObjCPropertyDecl::OBJC_PR_atomic) &&
>> + !(AttributesAsWrittern & ObjCPropertyDecl::OBJC_PR_nonatomic)) {
>> + GetterMethod = IMPDecl->getInstanceMethod(Property->getGetterName());
>> + SetterMethod = IMPDecl->getInstanceMethod(Property->getSetterName());
>> + LookedUpGetterSetter = true;
>> + if (GetterMethod) {
>> + Diag(GetterMethod->getLocation(),
>> + diag::warn_default_atomic_custom_getter_setter)
>> + << Property->getIdentifier();
>> + Diag(Property->getLocation(), diag::note_property_declare);
>> + }
>> + if (SetterMethod) {
>> + Diag(SetterMethod->getLocation(),
>> + diag::warn_default_atomic_custom_getter_setter)
>> + << Property->getIdentifier();
>> + Diag(Property->getLocation(), diag::note_property_declare);
>> + }
>> + }
>> +
>> // We only care about readwrite atomic property.
>> if ((Attributes & ObjCPropertyDecl::OBJC_PR_nonatomic) ||
>> !(Attributes & ObjCPropertyDecl::OBJC_PR_readwrite))
>> @@ -1090,10 +1115,11 @@
>> = IMPDecl->FindPropertyImplDecl(Property->getIdentifier())) {
>> if (PIDecl->getPropertyImplementation() == ObjCPropertyImplDecl::Dynamic)
>> continue;
>
> Why don't you warn if property is dynamic? They still have custom setter/getter.
It warns with dynamic too.
>> - ObjCMethodDecl *GetterMethod =
>> - IMPDecl->getInstanceMethod(Property->getGetterName());
>> - ObjCMethodDecl *SetterMethod =
>> - IMPDecl->getInstanceMethod(Property->getSetterName());
>> + if (!LookedUpGetterSetter) {
>> + GetterMethod = IMPDecl->getInstanceMethod(Property->getGetterName());
>> + SetterMethod = IMPDecl->getInstanceMethod(Property->getSetterName());
>> + LookedUpGetterSetter = true;
>> + }
>> if ((GetterMethod && !SetterMethod) || (!GetterMethod && SetterMethod)) {
>> SourceLocation MethodLoc =
>> (GetterMethod ? GetterMethod->getLocation()
>
> I think you don't want to warn if getter/setter are synthesized. Does this handle it?
Yes, I added a test case to have it verified.
>
>>
>> Added: cfe/trunk/test/SemaObjC/custom-atomic-property.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/custom-atomic-property.m?rev=124609&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/SemaObjC/custom-atomic-property.m (added)
>> +++ cfe/trunk/test/SemaObjC/custom-atomic-property.m Mon Jan 31 15:34:11 2011
>> @@ -0,0 +1,10 @@
>> +// RUN: %clang_cc1 -fsyntax-only -Wcustom-atomic-properties -verify %s
>> +
>> + at interface Foo
>> + at property (assign) Foo *myProp; // expected-note {{property declared here}} expected-note {{property declared here}}
>> + at end
>> +
>> + at implementation Foo
>> + -(Foo*)myProp {return 0;} // expected-warning {{atomic by default property 'myProp' has a user defined setter/getter (property should be marked 'atomic' if this is intended)}}
>> + -(void)setMyProp:(Foo*)e {} // expected-warning {{atomic by default property 'myProp' has a user defined setter/getter (property should be marked 'atomic' if this is intended)}}
>
> It is better for diagnostic to say setter for setters and getter for getters, instead of setter/getter for both.
Done in r124620.
Thanks for the review!
-Argiris
>
> - Fariborz
>
>> + at end
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
More information about the cfe-commits
mailing list