[cfe-commits] r138926 - in /cfe/trunk: include/clang/AST/ASTMutationListener.h include/clang/Serialization/ASTBitCodes.h include/clang/Serialization/ASTReader.h include/clang/Serialization/ASTWriter.h include/clang/Serialization/Module.h lib/AST/DeclObjC.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTReaderDecl.cpp lib/Serialization/ASTWriter.cpp test/Modules/objc-categories.m test/PCH/chain-categories.m

[Douglas Gregor]

On Aug 31, 2011, at 5:58 PM, Argyrios Kyrtzidis wrote:

> Author: akirtzidis
> Date: Wed Aug 31 19:58:55 2011
> New Revision: 138926
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=138926&view=rev
> Log:
> Support importing of ObjC categories from modules.
> 
> The initial incentive was to fix a crash when PCH chaining categories
> to an interface, but the fix was done in the "modules way" that I hear
> is popular with the kids these days.
> 
> Each module stores the local chain of categories and we combine them
> when the interface is loaded.

Excellent!

> We also warn if non-dependent modules
> introduce duplicate named categories.

Bonus points for this one :)

> Added:
>    cfe/trunk/test/Modules/objc-categories.m
>    cfe/trunk/test/PCH/chain-categories.m
> Modified:
>    cfe/trunk/include/clang/AST/ASTMutationListener.h
>    cfe/trunk/include/clang/Serialization/ASTBitCodes.h
>    cfe/trunk/include/clang/Serialization/ASTReader.h
>    cfe/trunk/include/clang/Serialization/ASTWriter.h
>    cfe/trunk/include/clang/Serialization/Module.h
>    cfe/trunk/lib/AST/DeclObjC.cpp
>    cfe/trunk/lib/Serialization/ASTReader.cpp
>    cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
>    cfe/trunk/lib/Serialization/ASTWriter.cpp
> 
> Modified: cfe/trunk/include/clang/AST/ASTMutationListener.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTMutationListener.h?rev=138926&r1=138925&r2=138926&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/ASTMutationListener.h (original)
> +++ cfe/trunk/include/clang/AST/ASTMutationListener.h Wed Aug 31 19:58:55 2011
> @@ -22,6 +22,8 @@
>   class ClassTemplateSpecializationDecl;
>   class FunctionDecl;
>   class FunctionTemplateDecl;
> +  class ObjCCategoryDecl;
> +  class ObjCInterfaceDecl;
> 
> /// \brief An abstract interface that should be implemented by listeners
> /// that want to be notified when an AST entity gets modified after its
> @@ -54,6 +56,10 @@
> 
>   /// \brief A static data member was implicitly instantiated.
>   virtual void StaticDataMemberInstantiated(const VarDecl *D) {}
> +
> +  /// \brief A new objc category class was added for an interface.
> +  virtual void AddedObjCCategoryToInterface(const ObjCCategoryDecl *CatD,
> +                                            const ObjCInterfaceDecl *IFD) {}
> };
> 
> } // end namespace clang
> 
> Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=138926&r1=138925&r2=138926&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Wed Aug 31 19:58:55 2011
> @@ -64,6 +64,11 @@
>     /// \brief a Decl::Kind/DeclID pair.
>     typedef std::pair<uint32_t, DeclID> KindDeclIDPair;
> 
> +    // FIXME: Turn these into classes so we can have some type safety when
> +    // we go from local ID to global and vice-versa.
> +    typedef DeclID LocalDeclID;
> +    typedef DeclID GlobalDeclID;
> +
>     /// \brief An ID number that refers to a type in an AST file.
>     ///
>     /// The ID of a type is partitioned into two parts: the lower
> @@ -402,7 +407,11 @@
> 
>       /// \brief Record code for the source manager line table information,
>       /// which stores information about #line directives.
> -      SOURCE_MANAGER_LINE_TABLE = 48
> +      SOURCE_MANAGER_LINE_TABLE = 48,
> +
> +      /// \brief Record code for ObjC categories in a module that are chained to
> +      /// an interface.
> +      OBJC_CHAINED_CATEGORIES
>     };

Is there any particular reason why you didn't just add another DeclUpdateKind, so the "add a category" updates will be handled the same way as adding a template specialization or defining an already-declared class? The DECL_UPDATES mechanism is a good, general way to deal with this sort of issue.

> +namespace {
> +  /// \brief Given an ObjC interface, goes through the modules and links to the
> +  /// interface all the categories for it.
> +  class ObjCChainedCategoriesVisitor {
> +    ASTReader &Reader;
> +    serialization::GlobalDeclID InterfaceID;
> +    ObjCInterfaceDecl *Interface;
> +    ObjCCategoryDecl *GlobHeadCat, *GlobTailCat;
> +    llvm::DenseMap<DeclarationName, ObjCCategoryDecl *> NameCategoryMap;
> +
> +  public:
> +    ObjCChainedCategoriesVisitor(ASTReader &Reader,
> +                                 serialization::GlobalDeclID InterfaceID,
> +                                 ObjCInterfaceDecl *Interface)
> +      : Reader(Reader), InterfaceID(InterfaceID), Interface(Interface),
> +        GlobHeadCat(0), GlobTailCat(0) { }
> +
> +    static bool visit(Module &M, void *UserData) {
> +      return static_cast<ObjCChainedCategoriesVisitor *>(UserData)->visit(M);
> +    }
> +
> +    bool visit(Module &M) {
> +      if (Reader.isDeclIDFromModule(InterfaceID, M))
> +        return true; // We reached the module where the interface originated
> +                    // from. Stop traversing the imported modules.

I don't think this is what you want… returning "true" stops the entire visitation, so you might end up skipping some modules that you didn't want to skip.

	- Doug