[cfe-commits] r62007 - in /cfe/trunk: include/clang/AST/DeclObjC.h lib/AST/DeclObjC.cpp lib/Sema/SemaDeclObjC.cpp test/SemaObjC/property-4.m
steve naroff
snaroff at apple.com
Fri Jan 9 14:43:40 PST 2009
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