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