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; >> } >> -...@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'}} >> +...@property (retain) id MayCauseError; // expected-warning >> {{property 'MayCauseError' 'copy' attribute does not match the >> property inherited from 'ProtocolObject'}} >> @end >> >> >> >> _______________________________________________ >> 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
