r218651 - PR20399: Do not assert when adding an implicit member coming from a module at
Vassil Vassilev
vasil.georgiev.vasilev at cern.ch
Thu Oct 2 00:34:37 PDT 2014
On 10/02/2014 01:27 AM, David Blaikie wrote:
>
>
> On Wed, Oct 1, 2014 at 3:01 AM, Vassil Vassilev
> <vasil.georgiev.vasilev at cern.ch
> <mailto:vasil.georgiev.vasilev at cern.ch>> wrote:
>
> We tried to trim down the reproducer, we basically touched each
> line to try to simplify it but unsuccessfully.
>
>
> Hmm - well, I've managed to simplify it a bit further taking a rather
> similarly brute force approach (see r218840). I couldn't quite get
> down to something where I could obviously see the dependencies, but
> it's something at least.
Thanks! I agree it looks much better now. Out of curiosity: does it hit
the same assert in ASTWriter::AddedCXXImplicitMember?
Vassil
>
> Maybe somebody else more familiar with the modules could further
> reduce it but frankly I doubt it becoming a couple of lines.
>
> There is a description of pr20399 here
> http://llvm.org/bugs/show_bug.cgi?id=20399 . If you think it is
> worth mentioning it somewhere in the test case I could come up
> with a patch.
> The idea is that we need to somehow force clang to deserialize a
> CXXRecordDecl and require generation some of its implicit
> operators at pch/pcm build time.
> Vassil
>
> On 09/30/2014 06:32 PM, David Blaikie wrote:
>> 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
>> <richard-llvm at metafoo.co.uk <mailto:richard-llvm at metafoo.co.uk>>
>> 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
>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141002/dba42c12/attachment.html>
More information about the cfe-commits
mailing list