r218651 - PR20399: Do not assert when adding an implicit member coming from a module at
David Blaikie
dblaikie at gmail.com
Thu Oct 2 08:21:38 PDT 2014
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.
>
> 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 listcfe-commits at cs.uiuc.eduhttp://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/da41e033/attachment.html>
More information about the cfe-commits
mailing list