<div dir="ltr">Thanks, should hopefully be fixed in r203548.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Mar 10, 2014 at 11:30 PM, Michael Spencer <span dir="ltr"><<a href="mailto:bigcheesegs@gmail.com" target="_blank">bigcheesegs@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, Mar 10, 2014 at 8:10 PM, Richard Smith<br>
<<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br>
> Author: rsmith<br>
> Date: Mon Mar 10 22:10:46 2014<br>
> New Revision: 203534<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=203534&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=203534&view=rev</a><br>
> Log:<br>
> If a visibility update record is found for a DeclContext after that Decl has<br>
> already been loaded, apply that update record to the Decl immediately, rather<br>
> than adding it to a pending list and never applying it.<br>
><br>
> Added:<br>
> cfe/trunk/test/Modules/Inputs/update-after-load/<br>
> cfe/trunk/test/Modules/Inputs/update-after-load/a.h<br>
> cfe/trunk/test/Modules/Inputs/update-after-load/b.h<br>
> cfe/trunk/test/Modules/Inputs/update-after-load/module.map<br>
> cfe/trunk/test/Modules/Inputs/update-after-load/modules.timestamp<br>
> cfe/trunk/test/Modules/update-after-load.cpp<br>
> Modified:<br>
> cfe/trunk/include/clang/Serialization/Module.h<br>
> cfe/trunk/lib/Serialization/ASTReader.cpp<br>
> cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
> cfe/trunk/lib/Serialization/Module.cpp<br>
><br>
> Modified: cfe/trunk/include/clang/Serialization/Module.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/Module.h?rev=203534&r1=203533&r2=203534&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/Module.h?rev=203534&r1=203533&r2=203534&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/include/clang/Serialization/Module.h (original)<br>
> +++ cfe/trunk/include/clang/Serialization/Module.h Mon Mar 10 22:10:46 2014<br>
> @@ -44,13 +44,21 @@ enum ModuleKind {<br>
> MK_MainFile ///< File is a PCH file treated as the actual main file.<br>
> };<br>
><br>
> +/// A custom deleter for DeclContextInfo::NameLookupTableData, to allow<br>
> +/// an incomplete type to be used there.<br>
> +struct NameLookupTableDataDeleter {<br>
> + void operator()(<br>
> + OnDiskChainedHashTable<reader::ASTDeclContextNameLookupTrait> *Ptr) const;<br>
> +};<br>
> +<br>
> /// \brief Information about the contents of a DeclContext.<br>
> struct DeclContextInfo {<br>
> DeclContextInfo()<br>
> - : NameLookupTableData(), LexicalDecls(), NumLexicalDecls() {}<br>
> + : NameLookupTableData(), LexicalDecls(), NumLexicalDecls() {}<br>
><br>
> - OnDiskChainedHashTable<reader::ASTDeclContextNameLookupTrait><br>
> - *NameLookupTableData; // an ASTDeclContextNameLookupTable.<br>
> + /// An ASTDeclContextNameLookupTable.<br>
> + std::unique_ptr<OnDiskChainedHashTable<reader::ASTDeclContextNameLookupTrait>,<br>
> + NameLookupTableDataDeleter> NameLookupTableData;<br>
> const KindDeclIDPair *LexicalDecls;<br>
> unsigned NumLexicalDecls;<br>
> };<br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=203534&r1=203533&r2=203534&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=203534&r1=203533&r2=203534&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Mar 10 22:10:46 2014<br>
> @@ -457,6 +457,11 @@ ASTReader::setDeserializationListener(AS<br>
> }<br>
><br>
><br>
> +void NameLookupTableDataDeleter::<br>
> +operator()(ASTDeclContextNameLookupTable *Ptr) const {<br>
> + delete Ptr;<br>
> +}<br>
> +<br>
><br>
> unsigned ASTSelectorLookupTrait::ComputeHash(Selector Sel) {<br>
> return serialization::ComputeHash(Sel);<br>
> @@ -836,11 +841,10 @@ bool ASTReader::ReadDeclContextStorage(M<br>
> Error("Expected visible lookup table block");<br>
> return true;<br>
> }<br>
> - Info.NameLookupTableData<br>
> - = ASTDeclContextNameLookupTable::Create(<br>
> - (const unsigned char *)Blob.data() + Record[0],<br>
> - (const unsigned char *)Blob.data(),<br>
> - ASTDeclContextNameLookupTrait(*this, M));<br>
> + Info.NameLookupTableData.reset(ASTDeclContextNameLookupTable::Create(<br>
> + (const unsigned char *)Blob.data() + Record[0],<br>
> + (const unsigned char *)Blob.data(),<br>
> + ASTDeclContextNameLookupTrait(*this, M)));<br>
> }<br>
><br>
> return false;<br>
> @@ -2493,8 +2497,12 @@ bool ASTReader::ReadASTBlock(ModuleFile<br>
> ASTDeclContextNameLookupTrait(*this, F));<br>
> if (ID == PREDEF_DECL_TRANSLATION_UNIT_ID) { // Is it the TU?<br>
> DeclContext *TU = Context.getTranslationUnitDecl();<br>
> - F.DeclContextInfos[TU].NameLookupTableData = Table;<br>
> + F.DeclContextInfos[TU].NameLookupTableData.reset(Table);<br>
> TU->setHasExternalVisibleStorage(true);<br>
> + } else if (Decl *D = DeclsLoaded[ID - NUM_PREDEF_DECL_IDS]) {<br>
> + auto *DC = cast<DeclContext>(D);<br>
> + DC->getPrimaryContext()->setHasExternalVisibleStorage(true);<br>
> + F.DeclContextInfos[DC].NameLookupTableData.reset(Table);<br>
> } else<br>
> PendingVisibleUpdates[ID].push_back(std::make_pair(Table, &F));<br>
> break;<br>
> @@ -6078,7 +6086,7 @@ namespace {<br>
> return false;<br>
><br>
> // Look for this name within this module.<br>
> - ASTDeclContextNameLookupTable *LookupTable =<br>
> + const auto &LookupTable =<br>
> Info->second.NameLookupTableData;<br>
> ASTDeclContextNameLookupTable::iterator Pos<br>
> = LookupTable->find(This->Name);<br>
> @@ -6208,7 +6216,7 @@ namespace {<br>
> if (!FoundInfo)<br>
> return false;<br>
><br>
> - ASTDeclContextNameLookupTable *LookupTable =<br>
> + const auto &LookupTable =<br>
> Info->second.NameLookupTableData;<br>
> bool FoundAnything = false;<br>
> for (ASTDeclContextNameLookupTable::data_iterator<br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=203534&r1=203533&r2=203534&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=203534&r1=203533&r2=203534&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Mon Mar 10 22:10:46 2014<br>
> @@ -2609,13 +2609,9 @@ Decl *ASTReader::ReadDeclRecord(DeclID I<br>
> // There are updates. This means the context has external visible<br>
> // storage, even if the original stored version didn't.<br>
> LookupDC->setHasExternalVisibleStorage(true);<br>
> - DeclContextVisibleUpdates &U = I->second;<br>
> - for (DeclContextVisibleUpdates::iterator UI = U.begin(), UE = U.end();<br>
> - UI != UE; ++UI) {<br>
> - DeclContextInfo &Info = UI->second->DeclContextInfos[DC];<br>
> - delete Info.NameLookupTableData;<br>
> - Info.NameLookupTableData = UI->first;<br>
> - }<br>
> + for (const auto &Update : I->second)<br>
> + Update.second->DeclContextInfos[DC].NameLookupTableData.reset(<br>
> + Update.first);<br>
> PendingVisibleUpdates.erase(I);<br>
> }<br>
> }<br>
><br>
> Modified: cfe/trunk/lib/Serialization/Module.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/Module.cpp?rev=203534&r1=203533&r2=203534&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/Module.cpp?rev=203534&r1=203533&r2=203534&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/Module.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/Module.cpp Mon Mar 10 22:10:46 2014<br>
> @@ -45,13 +45,6 @@ ModuleFile::ModuleFile(ModuleKind Kind,<br>
> {}<br>
><br>
> ModuleFile::~ModuleFile() {<br>
> - for (DeclContextInfosMap::iterator I = DeclContextInfos.begin(),<br>
> - E = DeclContextInfos.end();<br>
> - I != E; ++I) {<br>
> - if (I->second.NameLookupTableData)<br>
> - delete I->second.NameLookupTableData;<br>
> - }<br>
> -<br>
> delete static_cast<ASTIdentifierLookupTable *>(IdentifierLookupTable);<br>
> delete static_cast<HeaderFileInfoLookupTable *>(HeaderFileInfoTable);<br>
> delete static_cast<ASTSelectorLookupTable *>(SelectorLookupTable);<br>
><br>
> Added: cfe/trunk/test/Modules/Inputs/update-after-load/a.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/update-after-load/a.h?rev=203534&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/update-after-load/a.h?rev=203534&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/Inputs/update-after-load/a.h (added)<br>
> +++ cfe/trunk/test/Modules/Inputs/update-after-load/a.h Mon Mar 10 22:10:46 2014<br>
> @@ -0,0 +1 @@<br>
> +namespace llvm {}<br>
><br>
> Added: cfe/trunk/test/Modules/Inputs/update-after-load/b.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/update-after-load/b.h?rev=203534&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/update-after-load/b.h?rev=203534&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/Inputs/update-after-load/b.h (added)<br>
> +++ cfe/trunk/test/Modules/Inputs/update-after-load/b.h Mon Mar 10 22:10:46 2014<br>
> @@ -0,0 +1,2 @@<br>
> +#include "a.h"<br>
> +namespace llvm { void f(); }<br>
><br>
> Added: cfe/trunk/test/Modules/Inputs/update-after-load/module.map<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/update-after-load/module.map?rev=203534&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/update-after-load/module.map?rev=203534&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/Inputs/update-after-load/module.map (added)<br>
> +++ cfe/trunk/test/Modules/Inputs/update-after-load/module.map Mon Mar 10 22:10:46 2014<br>
> @@ -0,0 +1 @@<br>
> +module a { header "a.h" } module b { header "b.h" }<br>
><br>
> Added: cfe/trunk/test/Modules/Inputs/update-after-load/modules.timestamp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/update-after-load/modules.timestamp?rev=203534&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/update-after-load/modules.timestamp?rev=203534&view=auto</a><br>
> ==============================================================================<br>
> (empty)<br>
><br>
> Added: cfe/trunk/test/Modules/update-after-load.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/update-after-load.cpp?rev=203534&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/update-after-load.cpp?rev=203534&view=auto</a><br>
> ==============================================================================<br>
> --- cfe/trunk/test/Modules/update-after-load.cpp (added)<br>
> +++ cfe/trunk/test/Modules/update-after-load.cpp Mon Mar 10 22:10:46 2014<br>
> @@ -0,0 +1,8 @@<br>
> +// RUN: rm -rf %t<br>
> +// RUN: %clang_cc1 -fmodules -I %S/Inputs/update-after-load -verify -fmodules-cache-path=%t %s<br>
> +<br>
> +// expected-no-diagnostics<br>
> +#include "a.h"<br>
> +namespace llvm {}<br>
> +#include "b.h"<br>
> +void llvm::f() {}<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>
<br>
</div></div>This breaks the VS 2012 build. It tries to create a default<br>
DeclContextInfo::DeclContextInfo(const DeclContextInfo&), but that<br>
fails because of the unique_ptr.<br>
<span class="HOEnZb"><font color="#888888"><br>
- Michael Spencer<br>
</font></span></blockquote></div><br></div>