[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

[Argyrios Kyrtzidis]

On Aug 31, 2011, at 6:27 PM, Douglas Gregor wrote:

> 
> 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.

Because I'm handling them specially (e.g. figuring out whether there is a need for a warning); not sure how I can adapt ObjCChainedCategoriesVisitor to work with ASTReader::loadDeclUpdateRecords.

> 
>> +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.

This is not using ModuleManager's visitDepthFirst, it's using visit which skips only the modules that the current module imported:

  /// \param Visitor A visitor function that will be invoked with each
  /// module and the given user data pointer. The return value must be
  /// convertible to bool; when false, the visitation continues to
  /// modules that the current module depends on. When true, the
  /// visitation skips any modules that the current module depends on.


> 
> 	- Doug
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110831/31fd6108/attachment.html>