<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:0 0 0 .8ex;border-left:1px #ccc 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">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>