I haven't been watching the modules work closely, and I really understand how hard some of these test cases can be (I've had similarly complex/circularly dependent/etc test cases for debug info in some cases) - but I'll ask anyway: is this test case as simple as it can be? The number of classes and members in PR20399/vector and PR20399/stl_map.h seems pretty high - are all the different access levels necessary too?
If the complexity is required, maybe a summary comment explaining the twisty maze of expansions/ordering of things this tickles might be helpful? On Mon, Sep 29, 2014 at 5:45 PM, Richard Smith <[email protected]> wrote: > Author: rsmith > Date: Mon Sep 29 19:45:29 2014 > New Revision: 218651 > > URL: http://llvm.org/viewvc/llvm-project?rev=218651&view=rev > Log: > PR20399: Do not assert when adding an implicit member coming from a module > at > writing time. > > Patch by Vassil Vassilev! > > Added: > cfe/trunk/test/Modules/Inputs/PR20399/ > cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h > cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h > cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap > cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h > cfe/trunk/test/Modules/Inputs/PR20399/vector > cfe/trunk/test/Modules/pr20399.cpp > Modified: > cfe/trunk/lib/Serialization/ASTWriter.cpp > > Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=218651&r1=218650&r2=218651&view=diff > > ============================================================================== > --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original) > +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Mon Sep 29 19:45:29 2014 > @@ -5708,8 +5708,6 @@ void ASTWriter::CompletedTagDefinition(c > } > > void ASTWriter::AddedVisibleDecl(const DeclContext *DC, const Decl *D) { > - assert(!WritingAST && "Already writing the AST!"); > - > // TU and namespaces are handled elsewhere. > if (isa<TranslationUnitDecl>(DC) || isa<NamespaceDecl>(DC)) > return; > @@ -5718,12 +5716,12 @@ void ASTWriter::AddedVisibleDecl(const D > return; // Not a source decl added to a DeclContext from PCH. > > assert(!getDefinitiveDeclContext(DC) && "DeclContext not definitive!"); > + assert(!WritingAST && "Already writing the AST!"); > AddUpdatedDeclContext(DC); > UpdatingVisibleDecls.push_back(D); > } > > void ASTWriter::AddedCXXImplicitMember(const CXXRecordDecl *RD, const > Decl *D) { > - assert(!WritingAST && "Already writing the AST!"); > assert(D->isImplicit()); > if (!(!D->isFromASTFile() && RD->isFromASTFile())) > return; // Not a source member added to a class from PCH. > @@ -5732,17 +5730,18 @@ void ASTWriter::AddedCXXImplicitMember(c > > // A decl coming from PCH was modified. > assert(RD->isCompleteDefinition()); > + assert(!WritingAST && "Already writing the AST!"); > DeclUpdates[RD].push_back(DeclUpdate(UPD_CXX_ADDED_IMPLICIT_MEMBER, D)); > } > > void ASTWriter::AddedCXXTemplateSpecialization(const ClassTemplateDecl > *TD, > const > ClassTemplateSpecializationDecl *D) { > // The specializations set is kept in the canonical template. > - assert(!WritingAST && "Already writing the AST!"); > TD = TD->getCanonicalDecl(); > if (!(!D->isFromASTFile() && TD->isFromASTFile())) > return; // Not a source specialization added to a template from PCH. > > + assert(!WritingAST && "Already writing the AST!"); > > DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION, > D)); > } > @@ -5750,11 +5749,11 @@ void ASTWriter::AddedCXXTemplateSpeciali > void ASTWriter::AddedCXXTemplateSpecialization( > const VarTemplateDecl *TD, const VarTemplateSpecializationDecl *D) { > // The specializations set is kept in the canonical template. > - assert(!WritingAST && "Already writing the AST!"); > TD = TD->getCanonicalDecl(); > if (!(!D->isFromASTFile() && TD->isFromASTFile())) > return; // Not a source specialization added to a template from PCH. > > + assert(!WritingAST && "Already writing the AST!"); > > DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION, > D)); > } > @@ -5762,11 +5761,11 @@ void ASTWriter::AddedCXXTemplateSpeciali > void ASTWriter::AddedCXXTemplateSpecialization(const FunctionTemplateDecl > *TD, > const FunctionDecl *D) { > // The specializations set is kept in the canonical template. > - assert(!WritingAST && "Already writing the AST!"); > TD = TD->getCanonicalDecl(); > if (!(!D->isFromASTFile() && TD->isFromASTFile())) > return; // Not a source specialization added to a template from PCH. > > + assert(!WritingAST && "Already writing the AST!"); > > DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION, > D)); > } > > Added: cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h?rev=218651&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h (added) > +++ cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h Mon Sep 29 > 19:45:29 2014 > @@ -0,0 +1,17 @@ > +#ifndef FIRSTHEADER > +#define FIRSTHEADER > + > +#include "SecondHeader.h" // Just a class which gets in the lazy > deserialization chain > + > +#include "stl_map.h" > +#include "vector" > +struct A { > + typedef std::map<int, int*>::iterator el; > +}; > + > +struct B { > + ~B() {} > + std::vector<int> fvec; // Cannot replace with simple mockup > +}; > + > +#endif > > Added: cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h?rev=218651&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h (added) > +++ cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h Mon Sep 29 > 19:45:29 2014 > @@ -0,0 +1,13 @@ > +#ifndef SECONDHEADER > +#define SECONDHEADER > + > +#include "vector" > + > +class Collection { > + template <class T> struct Address { }; > +}; > + > +template <> struct Collection::Address<std::vector<bool> > > + : public Collection::Address<std::vector<bool>::iterator> { }; > + > +#endif > > Added: cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap?rev=218651&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap (added) > +++ cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap Mon Sep 29 > 19:45:29 2014 > @@ -0,0 +1,18 @@ > +module stdlib [system] { > + header "stl_map.h" > + header "vector" > + } > + > +module libCore { > + header "SecondHeader.h" > + use stdlib > + export * > +} > + > +module libGdml { > + header "FirstHeader.h" > + use libCore > + use stdlib > + export * > +} > + > > Added: cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h?rev=218651&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h (added) > +++ cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h Mon Sep 29 19:45:29 > 2014 > @@ -0,0 +1,25 @@ > +namespace std > +{ > + template<typename _Iterator> > + class reverse_iterator {}; > + > + template<typename _Iterator> > + inline int* > + operator-(const int& __x, const reverse_iterator<_Iterator>& __y) {}; > + > + template<typename _Tp> > + struct _Rb_tree_iterator > + { > + typedef _Rb_tree_iterator<_Tp> _Self; > + }; > + > + template <typename _Key, typename _Tp > > + class map > + { > + public: > + typedef _Rb_tree_iterator<int> iterator; > + > + template<typename _K1, typename _T1> > + friend bool operator<(const map<_K1, _T1>&, const map<_K1, _T1>&); > + }; > +} // namespace std > > Added: cfe/trunk/test/Modules/Inputs/PR20399/vector > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/vector?rev=218651&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/PR20399/vector (added) > +++ cfe/trunk/test/Modules/Inputs/PR20399/vector Mon Sep 29 19:45:29 2014 > @@ -0,0 +1,51 @@ > +namespace std > +{ > + template<typename _Tp, typename _Alloc = int> > + class vector > + { > + public: > + int* _M_start; > + int* _M_end_of_storage; > + > + ~vector() > + { this->_M_end_of_storage - this->_M_start; } > + }; > + > + struct _Bit_iterator {}; > + > + inline int* operator-(const _Bit_iterator& __x, const _Bit_iterator& > __y) > + { > + return 0; > + } > + > + struct _Bvector_base > + { > + struct _Bvector_impl > + { > + _Bit_iterator _M_start; > + > + _Bvector_impl() { } > + }; > + > + public: > + ~_Bvector_base() > + { this->_M_deallocate(); } > + > + protected: > + _Bvector_impl _M_impl; > + > + void _M_deallocate() {} > + }; > + > + template<typename _Alloc> > + class vector<bool, _Alloc> : protected _Bvector_base > + { > + typedef _Bvector_base _Base; > + public: > + typedef _Bit_iterator iterator; > + > + vector() > + : _Base() { } > + }; > + > +} // namespace std > > Added: cfe/trunk/test/Modules/pr20399.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr20399.cpp?rev=218651&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/pr20399.cpp (added) > +++ cfe/trunk/test/Modules/pr20399.cpp Mon Sep 29 19:45:29 2014 > @@ -0,0 +1,2 @@ > +// RUN: rm -rf %t > +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t > -fmodule-name=libGdml -emit-module -x c++ -std=c++11 > %S/Inputs/PR20399/module.modulemap > > > _______________________________________________ > 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
