[cfe-commits] r60634 - in /cfe/trunk: include/clang/AST/DeclObjC.h lib/AST/DeclObjC.cpp lib/Sema/Sema.h lib/Sema/SemaDeclObjC.cpp test/SemaObjC/property-category-3.m

Daniel Dunbar daniel at zuster.org
Tue Jan 6 22:47:25 PST 2009


Fariborz (& Steve),

I know this is how some other existing code was being handled, but I
am a little concerned about this implementation strategy (copying the
properties into the class). This feels counter to clang's general
strategy of having the AST closely reflect the source. In addition, it
seems like this means more malloc trashing than is really necessary.

My question is, how much work would it be to *not* copy the properties
in this fashion? Is it more complicated than just rewriting the type
checking and the property lookup mechanisms?

I can imagine that this will slow down the property lookup, but I
think the common usage scenario is that many classes are declared but
few will actually participate in lookup. And there is always the
option of lazily computing the lookup information if we really need
the best of both worlds.

 - Daniel

On Sat, Dec 6, 2008 at 3:03 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
> Author: fjahanian
> Date: Sat Dec  6 17:03:39 2008
> New Revision: 60634
>
> URL: http://llvm.org/viewvc/llvm-project?rev=60634&view=rev
> Log:
> Use of properties declared in protocols in the category
> via the category's protocol list1s, with appropriate
> diagnsostics and a test case.
>
> Added:
>    cfe/trunk/test/SemaObjC/property-category-3.m
> Modified:
>    cfe/trunk/include/clang/AST/DeclObjC.h
>    cfe/trunk/lib/AST/DeclObjC.cpp
>    cfe/trunk/lib/Sema/Sema.h
>    cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclObjC.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=60634&r1=60633&r2=60634&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclObjC.h (original)
> +++ cfe/trunk/include/clang/AST/DeclObjC.h Sat Dec  6 17:03:39 2008
> @@ -883,6 +883,8 @@
>
>   void addProperties(ObjCPropertyDecl **Properties, unsigned NumProperties);
>
> +  void mergeProperties(ObjCPropertyDecl **Properties, unsigned NumProperties);
> +
>   void addPropertyMethods(ASTContext &Context,
>                           ObjCPropertyDecl* Property,
>                           llvm::SmallVector<ObjCMethodDecl*, 32> &insMethods,
>
> Modified: cfe/trunk/lib/AST/DeclObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=60634&r1=60633&r2=60634&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclObjC.cpp (original)
> +++ cfe/trunk/lib/AST/DeclObjC.cpp Sat Dec  6 17:03:39 2008
> @@ -513,6 +513,31 @@
>   ::addPropertyMethods(this, Context, property, insMethods, InsMap);
>  }
>
> +/// mergeProperties - Adds properties to the end of list of current properties
> +/// for this category.
> +
> +void ObjCCategoryDecl::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);
> +  }
> +}
> +
>  /// addPropertyMethods - Goes through list of properties declared in this class
>  /// and builds setter/getter method declartions depending on the setter/getter
>  /// attributes of the property.
>
> Modified: cfe/trunk/lib/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=60634&r1=60633&r2=60634&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/Sema.h (original)
> +++ cfe/trunk/lib/Sema/Sema.h Sat Dec  6 17:03:39 2008
> @@ -1075,10 +1075,10 @@
>                                 const IdentifierInfo *Name);
>   void ComparePropertiesInBaseAndSuper(ObjCInterfaceDecl *IDecl);
>
> -  void MergeProtocolPropertiesIntoClass(ObjCInterfaceDecl *IDecl,
> +  void MergeProtocolPropertiesIntoClass(Decl *CDecl,
>                                         DeclTy *MergeProtocols);
>
> -  void MergeOneProtocolPropertiesIntoClass(ObjCInterfaceDecl *IDecl,
> +  void MergeOneProtocolPropertiesIntoClass(Decl *CDecl,
>                                            ObjCProtocolDecl *PDecl);
>
>   virtual void ActOnAtEnd(SourceLocation AtEndLoc, DeclTy *classDecl,
>
> Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=60634&r1=60633&r2=60634&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Sat Dec  6 17:03:39 2008
> @@ -311,11 +311,35 @@
>
>  /// MergeOneProtocolPropertiesIntoClass - This routine goes thru the list
>  /// of properties declared in a protocol and adds them to the list
> -/// of properties for current class if it is not there already.
> +/// of properties for current class/category if it is not there already.
>  void
> -Sema::MergeOneProtocolPropertiesIntoClass(ObjCInterfaceDecl *IDecl,
> +Sema::MergeOneProtocolPropertiesIntoClass(Decl *CDecl,
>                                           ObjCProtocolDecl *PDecl) {
>   llvm::SmallVector<ObjCPropertyDecl*, 16> mergeProperties;
> +  ObjCInterfaceDecl *IDecl = dyn_cast_or_null<ObjCInterfaceDecl>(CDecl);
> +  if (!IDecl) {
> +    // Category
> +    ObjCCategoryDecl *CatDecl = static_cast<ObjCCategoryDecl*>(CDecl);
> +    assert (CatDecl && "MergeOneProtocolPropertiesIntoClass");
> +    for (ObjCProtocolDecl::classprop_iterator P = PDecl->classprop_begin(),
> +         E = PDecl->classprop_end(); P != E; ++P) {
> +      ObjCPropertyDecl *Pr = (*P);
> +      ObjCCategoryDecl::classprop_iterator CP, CE;
> +      // Is this property already in  category's list of properties?
> +      for (CP = CatDecl->classprop_begin(), CE = CatDecl->classprop_end();
> +           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
> +        // Property protocol already exist in class. Diagnose any mismatch.
> +        DiagnosePropertyMismatch((*CP), Pr, PDecl->getIdentifier());
> +    }
> +    CatDecl->mergeProperties(&mergeProperties[0], mergeProperties.size());
> +    return;
> +  }
>   for (ObjCProtocolDecl::classprop_iterator P = PDecl->classprop_begin(),
>        E = PDecl->classprop_end(); P != E; ++P) {
>     ObjCPropertyDecl *Pr = (*P);
> @@ -337,13 +361,39 @@
>
>  /// MergeProtocolPropertiesIntoClass - This routine merges properties
>  /// declared in 'MergeItsProtocols' objects (which can be a class or an
> -/// inherited protocol into the list of properties for class 'IDecl'
> +/// inherited protocol into the list of properties for class/category 'CDecl'
>  ///
>
>  void
> -Sema::MergeProtocolPropertiesIntoClass(ObjCInterfaceDecl *IDecl,
> +Sema::MergeProtocolPropertiesIntoClass(Decl *CDecl,
>                                        DeclTy *MergeItsProtocols) {
>   Decl *ClassDecl = static_cast<Decl *>(MergeItsProtocols);
> +  ObjCInterfaceDecl *IDecl = dyn_cast_or_null<ObjCInterfaceDecl>(CDecl);
> +
> +  if (!IDecl) {
> +    // Category
> +    ObjCCategoryDecl *CatDecl = static_cast<ObjCCategoryDecl*>(CDecl);
> +    assert (CatDecl && "MergeProtocolPropertiesIntoClass");
> +    if (ObjCCategoryDecl *MDecl = dyn_cast<ObjCCategoryDecl>(ClassDecl)) {
> +      for (ObjCCategoryDecl::protocol_iterator P = MDecl->protocol_begin(),
> +           E = MDecl->protocol_end(); P != E; ++P)
> +      // Merge properties of category (*P) into IDECL's
> +      MergeOneProtocolPropertiesIntoClass(CatDecl, *P);
> +
> +      // Go thru the list of protocols for this category and recursively merge
> +      // their properties into this class as well.
> +      for (ObjCCategoryDecl::protocol_iterator P = CatDecl->protocol_begin(),
> +           E = CatDecl->protocol_end(); P != E; ++P)
> +        MergeProtocolPropertiesIntoClass(CatDecl, *P);
> +    } else {
> +      ObjCProtocolDecl *MD = cast<ObjCProtocolDecl>(ClassDecl);
> +      for (ObjCProtocolDecl::protocol_iterator P = MD->protocol_begin(),
> +           E = MD->protocol_end(); P != E; ++P)
> +        MergeOneProtocolPropertiesIntoClass(CatDecl, (*P));
> +    }
> +    return;
> +  }
> +
>   if (ObjCInterfaceDecl *MDecl = dyn_cast<ObjCInterfaceDecl>(ClassDecl)) {
>     for (ObjCInterfaceDecl::protocol_iterator P = MDecl->protocol_begin(),
>          E = MDecl->protocol_end(); P != E; ++P)
> @@ -1063,8 +1113,8 @@
>     // By the same token, they are also used to add new properties. No
>     // need to compare the added property to those in the class.
>
> -    // FIXME: If we merge properties into class we should probably
> -    // merge them into category as well?
> +    // Merge protocol properties into category
> +    MergeProtocolPropertiesIntoClass(C, C);
>     for (ObjCCategoryDecl::classprop_iterator i = C->classprop_begin(),
>          e = C->classprop_end(); i != e; ++i) {
>       diagnosePropertySetterGetterMismatch((*i), InsMap[(*i)->getGetterName()],
>
> Added: cfe/trunk/test/SemaObjC/property-category-3.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/property-category-3.m?rev=60634&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/property-category-3.m (added)
> +++ cfe/trunk/test/SemaObjC/property-category-3.m Sat Dec  6 17:03:39 2008
> @@ -0,0 +1,21 @@
> + at protocol P
> +  @property(readonly) int X;
> + at end
> +
> + at protocol P1<P>
> +  @property (copy) id ID;
> + at end
> +
> + at interface I
> + at end
> +
> + at interface I (Cat) <P>
> + at property float X; // expected-warning {{property type 'float' does not match property type inherited from 'P'}}
> + at end
> +
> + at interface I (Cat2) <P1>
> + at property (retain) id ID; // expected-warning {{property 'ID' 'copy' attribute does not match the property inherited from 'P1'}}
> + 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