<div dir="ltr">The problem is that the test is for testing non-determinism. And obviously there might just be instances where the test passes out of luck.<div><br></div><div>I have temporarily disabled one more of those tests in r233173, which should hopefully appease all the bots for now.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Mar 25, 2015 at 7:35 AM, Justin Bogner <span dir="ltr"><<a href="mailto:mail@justinbogner.com" target="_blank">mail@justinbogner.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">Rafael Espindola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> writes:<br>
> Author: rafael<br>
> Date: Tue Mar 24 23:43:15 2015<br>
> New Revision: 233172<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=233172&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=233172&view=rev</a><br>
> Log:<br>
> Revert "[Modules] When writing out the on-disk hash table for the decl<br>
> context lookup tables, we need to establish a stable ordering for<br>
> constructing the hash table. This is trickier than it might seem."<br>
><br>
> This reverts commit r233156. It broke the bots.<br>
<br>
</span>Which bots did it break? Lots of bots (and my local check-clang) went<br>
red from the revert, because the tests in r233162 depend on it. I can't<br>
actually find any bots that look like r233156 broke (at least that were<br>
still broken after r233162+r233163), but I'm probably not looking in the<br>
right place.<br>
<div class="HOEnZb"><div class="h5"><br>
> Modified:<br>
> cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
><br>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=233172&r1=233171&r2=233172&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=233172&r1=233171&r2=233172&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)<br>
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Mar 24 23:43:15 2015<br>
> @@ -3761,34 +3761,15 @@ ASTWriter::GenerateNameLookupTable(const<br>
> !DC->HasLazyExternalLexicalLookups &&<br>
> "must call buildLookups first");<br>
><br>
> - // Create the on-disk hash table representation.<br>
> llvm::OnDiskChainedHashTableGenerator<ASTDeclContextNameLookupTrait><br>
> Generator;<br>
> ASTDeclContextNameLookupTrait Trait(*this);<br>
><br>
> - // Store the sortable lookup results -- IE, those with meaningful names. We<br>
> - // will sort them by the DeclarationName in order to stabilize the ordering<br>
> - // of the hash table. We can't do this for constructors or conversion<br>
> - // functions which are handled separately.<br>
> - SmallVector<std::pair<DeclarationName, DeclContext::lookup_result>, 16><br>
> - NamedResults;<br>
> -<br>
> - // We can't directly construct a nonce constructor or conversion name without<br>
> - // tripping asserts, so we just recall the first one we see. Only the kind<br>
> - // will actually be serialized.<br>
> + // Create the on-disk hash table representation.<br>
> DeclarationName ConstructorName;<br>
> DeclarationName ConversionName;<br>
> - // Retain a mapping from each actual declaration name to the results<br>
> - // associated with it. We use a map here because the order in which we<br>
> - // discover these lookup results isn't ordered. We have to re-establish<br>
> - // a stable ordering which we do by walking the children of the decl context,<br>
> - // and emitting these in the order in which their names first appeared. Note<br>
> - // that the names' first appearance may not be one of the results or a result<br>
> - // at all. We just use this to establish an ordering.<br>
> - llvm::SmallDenseMap<DeclarationName, DeclContext::lookup_result, 8><br>
> - ConstructorResults;<br>
> - llvm::SmallDenseMap<DeclarationName, DeclContext::lookup_result, 4><br>
> - ConversionResults;<br>
> + SmallVector<NamedDecl *, 8> ConstructorDecls;<br>
> + SmallVector<NamedDecl *, 4> ConversionDecls;<br>
><br>
> visitLocalLookupResults(DC, [&](DeclarationName Name,<br>
> DeclContext::lookup_result Result) {<br>
> @@ -3805,90 +3786,34 @@ ASTWriter::GenerateNameLookupTable(const<br>
> // has the DeclarationName of the inherited constructors.<br>
> if (!ConstructorName)<br>
> ConstructorName = Name;<br>
> - ConstructorResults.insert(std::make_pair(Name, Result));<br>
> + ConstructorDecls.append(Result.begin(), Result.end());<br>
> return;<br>
><br>
> case DeclarationName::CXXConversionFunctionName:<br>
> if (!ConversionName)<br>
> ConversionName = Name;<br>
> - ConversionResults.insert(std::make_pair(Name, Result));<br>
> + ConversionDecls.append(Result.begin(), Result.end());<br>
> return;<br>
><br>
> default:<br>
> - NamedResults.push_back(std::make_pair(Name, Result));<br>
> + break;<br>
> }<br>
><br>
> + Generator.insert(Name, Result, Trait);<br>
> });<br>
><br>
> - // Sort and emit the named results first. This is the easy case.<br>
> - std::sort(<br>
> - NamedResults.begin(), NamedResults.end(),<br>
> - [](const std::pair<DeclarationName, DeclContext::lookup_result> &LHS,<br>
> - const std::pair<DeclarationName, DeclContext::lookup_result> &RHS) {<br>
> - return LHS.first < RHS.first;<br>
> - });<br>
> - for (auto Results : NamedResults)<br>
> - Generator.insert(Results.first, Results.second, Trait);<br>
> -<br>
> - // We have to specially handle the constructor and conversion function<br>
> - // declarations found.<br>
> - if (ConstructorResults.size() == 1) {<br>
> - // A special case that is easy is when we have just one constructor<br>
> - // declaration name.<br>
> + // Add the constructors.<br>
> + if (!ConstructorDecls.empty()) {<br>
> Generator.insert(ConstructorName,<br>
> - ConstructorResults.lookup(ConstructorName), Trait);<br>
> - ConstructorResults.clear();<br>
> - }<br>
> - if (ConversionResults.size() == 1) {<br>
> - // A special case that is easy is when we have just one conversion function<br>
> - // declaration name.<br>
> - Generator.insert(ConversionName, ConversionResults.lookup(ConversionName),<br>
> + DeclContext::lookup_result(ConstructorDecls),<br>
> Trait);<br>
> - ConversionResults.clear();<br>
> }<br>
> - if (!ConstructorResults.empty() || !ConversionResults.empty()) {<br>
> - SmallVector<NamedDecl *, 8> ConstructorDecls;<br>
> - SmallVector<NamedDecl *, 8> ConversionDecls;<br>
> -<br>
> - // Walk the decls in the context and collapse the results in that order. We<br>
> - // handle both constructors and conversion functions in a single walk as<br>
> - // the walk is a relative cache-hostile linked list walk.<br>
> - for (Decl *ChildD : DC->decls())<br>
> - if (auto *ChildND = dyn_cast<NamedDecl>(ChildD)) {<br>
> - auto ConstructorResultsIt =<br>
> - ConstructorResults.find(ChildND->getDeclName());<br>
> - if (ConstructorResultsIt != ConstructorResults.end()) {<br>
> - ConstructorDecls.append(ConstructorResultsIt->second.begin(),<br>
> - ConstructorResultsIt->second.end());<br>
> - ConstructorResults.erase(ConstructorResultsIt);<br>
> - }<br>
> -<br>
> - auto ConversionResultsIt =<br>
> - ConversionResults.find(ChildND->getDeclName());<br>
> - if (ConversionResultsIt != ConversionResults.end()) {<br>
> - ConversionDecls.append(ConversionResultsIt->second.begin(),<br>
> - ConversionResultsIt->second.end());<br>
> - ConversionResults.erase(ConversionResultsIt);<br>
> - }<br>
> -<br>
> - // If we handle all of the results, we're done.<br>
> - if (ConstructorResults.empty() && ConversionResults.empty())<br>
> - break;<br>
> - }<br>
> - assert(ConstructorResults.empty() && "Have constructor lookup results not "<br>
> - "associated with a declaration name "<br>
> - "within the context!");<br>
> - assert(ConversionResults.empty() && "Have conversion function lookup "<br>
> - "results not associated with a "<br>
> - "declaration name within the context!");<br>
> -<br>
> - if (!ConstructorDecls.empty())<br>
> - Generator.insert(ConstructorName,<br>
> - DeclContext::lookup_result(ConstructorDecls), Trait);<br>
> -<br>
> - if (!ConversionDecls.empty())<br>
> - Generator.insert(ConversionName,<br>
> - DeclContext::lookup_result(ConversionDecls), Trait);<br>
> +<br>
> + // Add the conversion functions.<br>
> + if (!ConversionDecls.empty()) {<br>
> + Generator.insert(ConversionName,<br>
> + DeclContext::lookup_result(ConversionDecls),<br>
> + Trait);<br>
> }<br>
><br>
> // Create the on-disk hash table in a buffer.<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>
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>
</div></div></blockquote></div><br></div>