[cfe-commits] r62007 - in /cfe/trunk: include/clang/AST/DeclObjC.h lib/AST/DeclObjC.cpp lib/Sema/SemaDeclObjC.cpp test/SemaObjC/property-4.m
Fariborz Jahanian
fjahanian at apple.com
Fri Jan 9 15:40:20 PST 2009
We need to talk about it based on priorities. I also would like to
mention that removing mergeProperties of my last patch
has broken some test cases in my code gen. test suite. This is
because, as I mentioned before, *all* clients need to become
smarter and do some of the work which was previously done in the AST
generation phase. I still have not come up
with a solution for this. But it will impact how aggressive we want
to become lazy everwhere :). We
can talk about this next week.
- fariborz
On Jan 9, 2009, at 2:43 PM, steve naroff wrote:
> Hi Fariborz,
>
> Thanks for improving this...it's much cleaner from many
> perspectives. Now that we're not actually merging, what about
> Sema::MergeOneProtocolPropertiesIntoClass() and
> Sema::MergeProtocolPropertiesIntoClass()? It seems like they could
> be simplified as well (and named appropriately)...
>
> I'm also very concerned about the performance of these two routines
> (lots of linear searching on a per-property basis). If the searching
> were done lazily (i.e. based on the properties use), it wouldn't
> bother me as much. Since we don't do this kind of exhaustive
> searching for methods, I don't know why we do it for properties.
> Here is an example:
>
> @interface YY
>
> @property int p;
> - (int)method;
>
> @end;
>
> @interface XX : YY
>
> @property float p;
> - (float)method;
>
> @end
>
> Both GCC and clang warn about the property but not the method.
> What's the benefit of treating them differently?
>
> Please advise,
>
> snaroff
>
> On Jan 9, 2009, at 4:04 PM, Fariborz Jahanian wrote:
>
>> Author: fjahanian
>> Date: Fri Jan 9 15:04:52 2009
>> New Revision: 62007
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=62007&view=rev
>> Log:
>> This patch removes mergeProperties and does the property lookup
>> in designated protocols lazily.
>>
>> Modified:
>> cfe/trunk/include/clang/AST/DeclObjC.h
>> cfe/trunk/lib/AST/DeclObjC.cpp
>> cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>> cfe/trunk/test/SemaObjC/property-4.m
>>
>> Modified: cfe/trunk/include/clang/AST/DeclObjC.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=62007&r1=62006&r2=62007&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/include/clang/AST/DeclObjC.h (original)
>> +++ cfe/trunk/include/clang/AST/DeclObjC.h Fri Jan 9 15:04:52 2009
>> @@ -270,10 +270,6 @@
>>
>> void addProperties(ObjCPropertyDecl **Properties, unsigned
>> NumProperties);
>>
>> - // FIXME: Replace with appropriate lookup. Currently used by
>> interfaces and
>> - // categories.
>> - void mergeProperties(ObjCPropertyDecl **Properties, unsigned
>> NumProperties);
>> -
>> typedef ObjCPropertyDecl * const * prop_iterator;
>> prop_iterator prop_begin() const { return PropertyDecl; }
>> prop_iterator prop_end() const {
>> @@ -640,7 +636,7 @@
>> // found, we search referenced protocols and class categories.
>> ObjCMethodDecl *lookupInstanceMethod(Selector Sel);
>> ObjCMethodDecl *lookupClassMethod(Selector Sel);
>> -
>> +
>> bool isForwardDecl() const { return isForwardProtoDecl; }
>> void setForwardDecl(bool val) { isForwardProtoDecl = val; }
>>
>>
>> Modified: cfe/trunk/lib/AST/DeclObjC.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=62007&r1=62006&r2=62007&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/lib/AST/DeclObjC.cpp (original)
>> +++ cfe/trunk/lib/AST/DeclObjC.cpp Fri Jan 9 15:04:52 2009
>> @@ -378,31 +378,6 @@
>> return sum;
>> }
>>
>> -/// mergeProperties - Adds properties to the end of list of
>> current properties
>> -/// for this class.
>> -
>> -void ObjCContainerDecl::mergeProperties(ObjCPropertyDecl
>> **Properties,
>> - unsigned NumNewProperties) {
>> - if (NumNewProperties == 0) return;
>> -
>> - if (PropertyDecl) {
>> - ObjCPropertyDecl **newPropertyDecl =
>> - new ObjCPropertyDecl*[NumNewProperties + NumPropertyDecl];
>> - ObjCPropertyDecl **buf = newPropertyDecl;
>> - // put back original properties in buffer.
>> - memcpy(buf, PropertyDecl,
>> NumPropertyDecl*sizeof(ObjCPropertyDecl*));
>> - // Add new properties to this buffer.
>> - memcpy(buf+NumPropertyDecl, Properties,
>> - NumNewProperties*sizeof(ObjCPropertyDecl*));
>> - delete[] PropertyDecl;
>> - PropertyDecl = newPropertyDecl;
>> - NumPropertyDecl += NumNewProperties;
>> - }
>> - else {
>> - addProperties(Properties, NumNewProperties);
>> - }
>> -}
>> -
>> /// addProperties - Insert property declaration AST nodes into
>> /// ObjCContainerDecl's PropertyDecl field.
>> ///
>> @@ -425,6 +400,16 @@
>> if (property->getIdentifier() == PropertyId)
>> return property;
>> }
>> + const ObjCProtocolDecl *PID = dyn_cast<ObjCProtocolDecl>(this);
>> + if (PID) {
>> + for (ObjCProtocolDecl::protocol_iterator P = PID-
>> >protocol_begin(),
>> + E = PID->protocol_end();
>> + P != E; ++P)
>> + if (ObjCPropertyDecl *property =
>> + (*P)->FindPropertyDeclaration(PropertyId))
>> + return property;
>> + }
>> +
>> const ObjCInterfaceDecl *OID = dyn_cast<ObjCInterfaceDecl>(this);
>> if (OID) {
>> // Look through categories.
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=62007&r1=62006&r2=62007&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Fri Jan 9 15:04:52 2009
>> @@ -340,7 +340,6 @@
>> void
>> Sema::MergeOneProtocolPropertiesIntoClass(Decl *CDecl,
>> ObjCProtocolDecl *PDecl) {
>> - llvm::SmallVector<ObjCPropertyDecl*, 16> mergeProperties;
>> ObjCInterfaceDecl *IDecl =
>> dyn_cast_or_null<ObjCInterfaceDecl>(CDecl);
>> if (!IDecl) {
>> // Category
>> @@ -355,14 +354,10 @@
>> CP != CE; ++CP)
>> if ((*CP)->getIdentifier() == Pr->getIdentifier())
>> break;
>> - if (CP == CE)
>> - // Add this property to list of properties for thie class.
>> - mergeProperties.push_back(Pr);
>> - else
>> + if (CP != CE)
>> // Property protocol already exist in class. Diagnose any
>> mismatch.
>> DiagnosePropertyMismatch((*CP), Pr, PDecl->getIdentifier());
>> }
>> - CatDecl->mergeProperties(&mergeProperties[0],
>> mergeProperties.size());
>> return;
>> }
>> for (ObjCProtocolDecl::prop_iterator P = PDecl->prop_begin(),
>> @@ -374,14 +369,10 @@
>> CP != CE; ++CP)
>> if ((*CP)->getIdentifier() == Pr->getIdentifier())
>> break;
>> - if (CP == CE)
>> - // Add this property to list of properties for thie class.
>> - mergeProperties.push_back(Pr);
>> - else
>> + if (CP != CE)
>> // Property protocol already exist in class. Diagnose any
>> mismatch.
>> DiagnosePropertyMismatch((*CP), Pr, PDecl->getIdentifier());
>> }
>> - IDecl->mergeProperties(&mergeProperties[0],
>> mergeProperties.size());
>> }
>>
>> /// MergeProtocolPropertiesIntoClass - This routine merges properties
>>
>> Modified: cfe/trunk/test/SemaObjC/property-4.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-4.m?rev=62007&r1=62006&r2=62007&view=diff
>>
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =
>> =====================================================================
>> --- cfe/trunk/test/SemaObjC/property-4.m (original)
>> +++ cfe/trunk/test/SemaObjC/property-4.m Fri Jan 9 15:04:52 2009
>> @@ -24,7 +24,6 @@
>> int newO;
>> int oldO;
>> }
>> - at property (retain) id MayCauseError; // expected-warning
>> {{property 'MayCauseError' 'copy' attribute does not match the
>> property inherited from 'GCObject'}} \
>> - expected-warning {{property 'MayCauseError' 'copy'
>> attribute does not match the property inherited from
>> 'ProtocolObject'}}
>> + at property (retain) id MayCauseError; // expected-warning
>> {{property 'MayCauseError' 'copy' attribute does not match the
>> property inherited from 'ProtocolObject'}}
>> @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