<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Oct 2, 2014 at 12:34 AM, Vassil Vassilev <span dir="ltr"><<a href="mailto:vasil.georgiev.vasilev@cern.ch" target="_blank">vasil.georgiev.vasilev@cern.ch</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000"><span class="">
    <div>On 10/02/2014 01:27 AM, David Blaikie
      wrote:<br>
    </div>
    <blockquote type="cite">
      
      <div dir="ltr"><br>
        <div class="gmail_extra"><br>
          <div class="gmail_quote">On Wed, Oct 1, 2014 at 3:01 AM,
            Vassil Vassilev <span dir="ltr"><<a href="mailto:vasil.georgiev.vasilev@cern.ch" target="_blank">vasil.georgiev.vasilev@cern.ch</a>></span>
            wrote:<br>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
              <div bgcolor="#FFFFFF" text="#000000">
                <div>We tried to trim down the reproducer, we basically
                  touched each line to try to simplify it but
                  unsuccessfully.</div>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div><span style="font-family:monospace">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.</span><br>
            </div>
          </div>
        </div>
      </div>
    </blockquote></span>
    Thanks! I agree it looks much better now. Out of curiosity: does it
    hit the same assert in ASTWriter::AddedCXXImplicitMember?</div></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000"><span class="HOEnZb"><font color="#888888"><br>
    Vassil</font></span><div><div class="h5"><br>
    <blockquote type="cite">
      <div dir="ltr">
        <div class="gmail_extra">
          <div class="gmail_quote">
            <div> </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
              <div bgcolor="#FFFFFF" text="#000000">
                <div> Maybe somebody else more familiar with the modules
                  could further reduce it but frankly I doubt it
                  becoming a couple of lines.<br>
                  <br>
                  There is a description of pr20399 here <a href="http://llvm.org/bugs/show_bug.cgi?id=20399" target="_blank">http://llvm.org/bugs/show_bug.cgi?id=20399</a>
                  . If you think it is worth mentioning it somewhere in
                  the test case I could come up with a patch.<br>
                  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.<span><font color="#888888"><br>
                      Vassil</font></span>
                  <div>
                    <div><br>
                      On 09/30/2014 06:32 PM, David Blaikie wrote:<br>
                    </div>
                  </div>
                </div>
                <div>
                  <div>
                    <blockquote type="cite">
                      <div dir="ltr">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 <span style="font-family:monospace">PR20399/vector
                          and </span><span style="font-family:monospace">PR20399/stl_map.h

                          seems pretty high - are all the different
                          access levels necessary too?<br>
                          <br>
                          If the complexity is required, maybe a summary
                          comment explaining the twisty maze of
                          expansions/ordering of things this tickles
                          might be helpful?</span></div>
                      <div class="gmail_extra"><br>
                        <div class="gmail_quote">On Mon, Sep 29, 2014 at
                          5:45 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard-llvm@metafoo.co.uk" target="_blank">richard-llvm@metafoo.co.uk</a>></span>
                          wrote:<br>
                          <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author:

                            rsmith<br>
                            Date: Mon Sep 29 19:45:29 2014<br>
                            New Revision: 218651<br>
                            <br>
                            URL: <a href="http://llvm.org/viewvc/llvm-project?rev=218651&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=218651&view=rev</a><br>
                            Log:<br>
                            PR20399: Do not assert when adding an
                            implicit member coming from a module at<br>
                            writing time.<br>
                            <br>
                            Patch by Vassil Vassilev!<br>
                            <br>
                            Added:<br>
                                cfe/trunk/test/Modules/Inputs/PR20399/<br>
                               
                            cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h<br>
                               
                            cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h<br>
                               
                            cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap<br>
                               
                            cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h<br>
                               
                            cfe/trunk/test/Modules/Inputs/PR20399/vector<br>
                                cfe/trunk/test/Modules/pr20399.cpp<br>
                            Modified:<br>
                               
                            cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
                            <br>
                            Modified:
                            cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
                            URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=218651&r1=218650&r2=218651&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=218651&r1=218650&r2=218651&view=diff</a><br>
==============================================================================<br>
                            ---
                            cfe/trunk/lib/Serialization/ASTWriter.cpp
                            (original)<br>
                            +++
                            cfe/trunk/lib/Serialization/ASTWriter.cpp
                            Mon Sep 29 19:45:29 2014<br>
                            @@ -5708,8 +5708,6 @@ void
                            ASTWriter::CompletedTagDefinition(c<br>
                             }<br>
                            <br>
                             void ASTWriter::AddedVisibleDecl(const
                            DeclContext *DC, const Decl *D) {<br>
                            -  assert(!WritingAST && "Already
                            writing the AST!");<br>
                            -<br>
                               // TU and namespaces are handled
                            elsewhere.<br>
                               if (isa<TranslationUnitDecl>(DC) ||
                            isa<NamespaceDecl>(DC))<br>
                                 return;<br>
                            @@ -5718,12 +5716,12 @@ void
                            ASTWriter::AddedVisibleDecl(const D<br>
                                 return; // Not a source decl added to a
                            DeclContext from PCH.<br>
                            <br>
                               assert(!getDefinitiveDeclContext(DC)
                            && "DeclContext not definitive!");<br>
                            +  assert(!WritingAST && "Already
                            writing the AST!");<br>
                               AddUpdatedDeclContext(DC);<br>
                               UpdatingVisibleDecls.push_back(D);<br>
                             }<br>
                            <br>
                             void
                            ASTWriter::AddedCXXImplicitMember(const
                            CXXRecordDecl *RD, const Decl *D) {<br>
                            -  assert(!WritingAST && "Already
                            writing the AST!");<br>
                               assert(D->isImplicit());<br>
                               if (!(!D->isFromASTFile() &&
                            RD->isFromASTFile()))<br>
                                 return; // Not a source member added to
                            a class from PCH.<br>
                            @@ -5732,17 +5730,18 @@ void
                            ASTWriter::AddedCXXImplicitMember(c<br>
                            <br>
                               // A decl coming from PCH was modified.<br>
                               assert(RD->isCompleteDefinition());<br>
                            +  assert(!WritingAST && "Already
                            writing the AST!");<br>
                             
                             DeclUpdates[RD].push_back(DeclUpdate(UPD_CXX_ADDED_IMPLICIT_MEMBER,
                            D));<br>
                             }<br>
                            <br>
                             void
                            ASTWriter::AddedCXXTemplateSpecialization(const
                            ClassTemplateDecl *TD,<br>
                                                                  const
                            ClassTemplateSpecializationDecl *D) {<br>
                               // The specializations set is kept in the
                            canonical template.<br>
                            -  assert(!WritingAST && "Already
                            writing the AST!");<br>
                               TD = TD->getCanonicalDecl();<br>
                               if (!(!D->isFromASTFile() &&
                            TD->isFromASTFile()))<br>
                                 return; // Not a source specialization
                            added to a template from PCH.<br>
                            <br>
                            +  assert(!WritingAST && "Already
                            writing the AST!");<br>
                             
 DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,<br>
                                                                    D));<br>
                             }<br>
                            @@ -5750,11 +5749,11 @@ void
                            ASTWriter::AddedCXXTemplateSpeciali<br>
                             void
                            ASTWriter::AddedCXXTemplateSpecialization(<br>
                                 const VarTemplateDecl *TD, const
                            VarTemplateSpecializationDecl *D) {<br>
                               // The specializations set is kept in the
                            canonical template.<br>
                            -  assert(!WritingAST && "Already
                            writing the AST!");<br>
                               TD = TD->getCanonicalDecl();<br>
                               if (!(!D->isFromASTFile() &&
                            TD->isFromASTFile()))<br>
                                 return; // Not a source specialization
                            added to a template from PCH.<br>
                            <br>
                            +  assert(!WritingAST && "Already
                            writing the AST!");<br>
                             
 DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,<br>
                                                                    D));<br>
                             }<br>
                            @@ -5762,11 +5761,11 @@ void
                            ASTWriter::AddedCXXTemplateSpeciali<br>
                             void
                            ASTWriter::AddedCXXTemplateSpecialization(const
                            FunctionTemplateDecl *TD,<br>
                                                                       
                                const FunctionDecl *D) {<br>
                               // The specializations set is kept in the
                            canonical template.<br>
                            -  assert(!WritingAST && "Already
                            writing the AST!");<br>
                               TD = TD->getCanonicalDecl();<br>
                               if (!(!D->isFromASTFile() &&
                            TD->isFromASTFile()))<br>
                                 return; // Not a source specialization
                            added to a template from PCH.<br>
                            <br>
                            +  assert(!WritingAST && "Already
                            writing the AST!");<br>
                             
 DeclUpdates[TD].push_back(DeclUpdate(UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION,<br>
                                                                    D));<br>
                             }<br>
                            <br>
                            Added:
                            cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h<br>
                            URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h?rev=218651&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h?rev=218651&view=auto</a><br>
==============================================================================<br>
                            ---
                            cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h
                            (added)<br>
                            +++
                            cfe/trunk/test/Modules/Inputs/PR20399/FirstHeader.h
                            Mon Sep 29 19:45:29 2014<br>
                            @@ -0,0 +1,17 @@<br>
                            +#ifndef FIRSTHEADER<br>
                            +#define FIRSTHEADER<br>
                            +<br>
                            +#include "SecondHeader.h" // Just a class
                            which gets in the lazy deserialization chain<br>
                            +<br>
                            +#include "stl_map.h"<br>
                            +#include "vector"<br>
                            +struct A {<br>
                            +   typedef std::map<int,
                            int*>::iterator el;<br>
                            +};<br>
                            +<br>
                            +struct B {<br>
                            +  ~B() {}<br>
                            +  std::vector<int> fvec; // Cannot
                            replace with simple mockup<br>
                            +};<br>
                            +<br>
                            +#endif<br>
                            <br>
                            Added:
                            cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h<br>
                            URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h?rev=218651&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h?rev=218651&view=auto</a><br>
==============================================================================<br>
                            ---
                            cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h
                            (added)<br>
                            +++
                            cfe/trunk/test/Modules/Inputs/PR20399/SecondHeader.h
                            Mon Sep 29 19:45:29 2014<br>
                            @@ -0,0 +1,13 @@<br>
                            +#ifndef SECONDHEADER<br>
                            +#define SECONDHEADER<br>
                            +<br>
                            +#include "vector"<br>
                            +<br>
                            +class Collection {<br>
                            +  template <class T> struct Address {
                            };<br>
                            +};<br>
                            +<br>
                            +template <> struct
                            Collection::Address<std::vector<bool>
                            ><br>
                            +   : public
                            Collection::Address<std::vector<bool>::iterator>
                            { };<br>
                            +<br>
                            +#endif<br>
                            <br>
                            Added:
                            cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap<br>
                            URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap?rev=218651&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap?rev=218651&view=auto</a><br>
==============================================================================<br>
                            ---
                            cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap
                            (added)<br>
                            +++
                            cfe/trunk/test/Modules/Inputs/PR20399/module.modulemap
                            Mon Sep 29 19:45:29 2014<br>
                            @@ -0,0 +1,18 @@<br>
                            +module stdlib [system] {<br>
                            +    header "stl_map.h"<br>
                            +    header "vector"<br>
                            + }<br>
                            +<br>
                            +module libCore {<br>
                            +  header "SecondHeader.h"<br>
                            +    use stdlib<br>
                            +  export *<br>
                            +}<br>
                            +<br>
                            +module libGdml {<br>
                            +  header "FirstHeader.h"<br>
                            +  use libCore<br>
                            +    use stdlib<br>
                            +  export *<br>
                            +}<br>
                            +<br>
                            <br>
                            Added:
                            cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h<br>
                            URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h?rev=218651&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h?rev=218651&view=auto</a><br>
==============================================================================<br>
                            ---
                            cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h
                            (added)<br>
                            +++
                            cfe/trunk/test/Modules/Inputs/PR20399/stl_map.h
                            Mon Sep 29 19:45:29 2014<br>
                            @@ -0,0 +1,25 @@<br>
                            +namespace std<br>
                            +{<br>
                            +  template<typename _Iterator><br>
                            +  class reverse_iterator {};<br>
                            +<br>
                            +  template<typename _Iterator><br>
                            +  inline int*<br>
                            +  operator-(const int& __x, const
                            reverse_iterator<_Iterator>& __y)
                            {};<br>
                            +<br>
                            +  template<typename _Tp><br>
                            +  struct _Rb_tree_iterator<br>
                            +  {<br>
                            +    typedef _Rb_tree_iterator<_Tp>   
                                _Self;<br>
                            +  };<br>
                            +<br>
                            +  template <typename _Key, typename _Tp
                            ><br>
                            +  class map<br>
                            +  {<br>
                            +  public:<br>
                            +    typedef _Rb_tree_iterator<int>   
                                iterator;<br>
                            +<br>
                            +    template<typename _K1, typename
                            _T1><br>
                            +    friend bool operator<(const
                            map<_K1, _T1>&, const map<_K1,
                            _T1>&);<br>
                            +  };<br>
                            +} // namespace std<br>
                            <br>
                            Added:
                            cfe/trunk/test/Modules/Inputs/PR20399/vector<br>
                            URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/vector?rev=218651&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/PR20399/vector?rev=218651&view=auto</a><br>
==============================================================================<br>
                            ---
                            cfe/trunk/test/Modules/Inputs/PR20399/vector
                            (added)<br>
                            +++
                            cfe/trunk/test/Modules/Inputs/PR20399/vector
                            Mon Sep 29 19:45:29 2014<br>
                            @@ -0,0 +1,51 @@<br>
                            +namespace std<br>
                            +{<br>
                            +  template<typename _Tp, typename _Alloc
                            = int><br>
                            +  class vector<br>
                            +  {<br>
                            +  public:<br>
                            +    int* _M_start;<br>
                            +    int* _M_end_of_storage;<br>
                            +<br>
                            +    ~vector()<br>
                            +    { this->_M_end_of_storage -
                            this->_M_start; }<br>
                            +  };<br>
                            +<br>
                            +  struct _Bit_iterator {};<br>
                            +<br>
                            +  inline int* operator-(const
                            _Bit_iterator& __x, const
                            _Bit_iterator& __y)<br>
                            +  {<br>
                            +    return 0;<br>
                            +  }<br>
                            +<br>
                            +  struct _Bvector_base<br>
                            +  {<br>
                            +    struct _Bvector_impl<br>
                            +    {<br>
                            +      _Bit_iterator     _M_start;<br>
                            +<br>
                            +      _Bvector_impl() { }<br>
                            +    };<br>
                            +<br>
                            +    public:<br>
                            +    ~_Bvector_base()<br>
                            +    { this->_M_deallocate(); }<br>
                            +<br>
                            +  protected:<br>
                            +    _Bvector_impl _M_impl;<br>
                            +<br>
                            +    void _M_deallocate() {}<br>
                            +  };<br>
                            +<br>
                            +  template<typename _Alloc><br>
                            +  class vector<bool, _Alloc> :
                            protected _Bvector_base<br>
                            +  {<br>
                            +    typedef _Bvector_base                 
                                _Base;<br>
                            +  public:<br>
                            +    typedef _Bit_iterator         
                            iterator;<br>
                            +<br>
                            +    vector()<br>
                            +      : _Base() { }<br>
                            +  };<br>
                            +<br>
                            +} // namespace std<br>
                            <br>
                            Added: cfe/trunk/test/Modules/pr20399.cpp<br>
                            URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr20399.cpp?rev=218651&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/pr20399.cpp?rev=218651&view=auto</a><br>
==============================================================================<br>
                            --- cfe/trunk/test/Modules/pr20399.cpp
                            (added)<br>
                            +++ cfe/trunk/test/Modules/pr20399.cpp Mon
                            Sep 29 19:45:29 2014<br>
                            @@ -0,0 +1,2 @@<br>
                            +// RUN: rm -rf %t<br>
                            +// RUN: %clang_cc1 -fmodules
                            -fmodules-cache-path=%t
                            -fmodule-name=libGdml -emit-module -x c++
                            -std=c++11
                            %S/Inputs/PR20399/module.modulemap<br>
                            <br>
                            <br>
_______________________________________________<br>
                            cfe-commits mailing list<br>
                            <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
                            <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
                          </blockquote>
                        </div>
                        <br>
                      </div>
                      <br>
                      <fieldset></fieldset>
                      <br>
                      <pre>_______________________________________________
cfe-commits mailing list
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a>
</pre>
                    </blockquote>
                    <br>
                  </div>
                </div>
              </div>
            </blockquote>
          </div>
          <br>
        </div>
      </div>
    </blockquote>
    <br>
  </div></div></div>

</blockquote></div><br></div></div>