Rafael Espindola <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
