r233172 - 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."
Justin Bogner
mail at justinbogner.com
Tue Mar 24 23:35:46 PDT 2015
Rafael Espindola <rafael.espindola at 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 at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list