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."
Daniel Jasper
djasper at google.com
Wed Mar 25 00:02:21 PDT 2015
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 <mail at justinbogner.com>
wrote:
> 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
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150325/0f1deedd/attachment.html>
More information about the cfe-commits
mailing list