r218651 - PR20399: Do not assert when adding an implicit member coming from a module at
Vassil Vassilev
vvasilev at cern.ch
Thu Oct 2 08:50:31 PDT 2014
> On 2.10.2014 г., at 17:21, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
>> On Thu, Oct 2, 2014 at 12:34 AM, Vassil Vassilev <vasil.georgiev.vasilev at cern.ch> wrote:
>>> 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> 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?
>
> I'm not sure which location the assertion was in, but it was the same "Already Writing the AST" assertion text (that's what I looked for while I was reducing it - so it might've switched from one instance of that assert to another, though I suspect not) that reproduced with my simplification.
I see. Thanks!
Vassil
>
>>
>> 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> 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
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> 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/256e5f65/attachment.html>
More information about the cfe-commits
mailing list