From: Richard Smith Date: Tue, 1 Sep 2015 01:37:34 +0000 (+0000) Subject: [modules] Preserve DeclID order when merging lookup tables to give a more X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7c23a18345a8fcf31ddd75e87e482bbd0f3306fe;p=clang [modules] Preserve DeclID order when merging lookup tables to give a more predictable diagnostic experience. The hash-of-DeclID order we were using before gave different results on Win32 due to a different predefined declaration of __builtin_va_list. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@246521 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Serialization/ASTReader.cpp b/lib/Serialization/ASTReader.cpp index 91a977bb69..7c83641256 100644 --- a/lib/Serialization/ASTReader.cpp +++ b/lib/Serialization/ASTReader.cpp @@ -959,7 +959,7 @@ ASTDeclContextNameLookupTrait::ReadKey(const unsigned char *d, unsigned) { void ASTDeclContextNameLookupTrait::ReadDataInto(internal_key_type, const unsigned char *d, unsigned DataLen, - data_type &Val) { + data_type_builder &Val) { using namespace llvm::support; for (unsigned NumDecls = DataLen / 4; NumDecls; --NumDecls) { uint32_t LocalID = endian::readNext(d); diff --git a/lib/Serialization/ASTReaderInternals.h b/lib/Serialization/ASTReaderInternals.h index 5c239e3a02..5e2728522f 100644 --- a/lib/Serialization/ASTReaderInternals.h +++ b/lib/Serialization/ASTReaderInternals.h @@ -47,8 +47,31 @@ public: static const int MaxTables = 4; /// The lookup result is a list of global declaration IDs. - // FIXME: LLVM doesn't really have a good data structure for this. - typedef llvm::DenseSet data_type; + typedef llvm::SmallVector data_type; + struct data_type_builder { + data_type &Data; + llvm::DenseSet Found; + + data_type_builder(data_type &D) : Data(D) {} + void insert(DeclID ID) { + // Just use a linear scan unless we have more than a few IDs. + if (Found.empty() && !Data.empty()) { + if (Data.size() <= 4) { + for (auto I : Found) + if (I == ID) + return; + Data.push_back(ID); + return; + } + + // Switch to tracking found IDs in the set. + Found.insert(Data.begin(), Data.end()); + } + + if (Found.insert(ID).second) + Data.push_back(ID); + } + }; typedef unsigned hash_value_type; typedef unsigned offset_type; typedef ModuleFile *file_type; @@ -76,10 +99,12 @@ public: internal_key_type ReadKey(const unsigned char *d, unsigned); void ReadDataInto(internal_key_type, const unsigned char *d, - unsigned DataLen, data_type &Val); + unsigned DataLen, data_type_builder &Val); - static void MergeDataInto(const data_type &From, data_type &To) { - To.insert(From.begin(), From.end()); + static void MergeDataInto(const data_type &From, data_type_builder &To) { + To.Data.reserve(To.Data.size() + From.size()); + for (DeclID ID : From) + To.insert(ID); } file_type ReadFileRef(const unsigned char *&d); diff --git a/lib/Serialization/MultiOnDiskHashTable.h b/lib/Serialization/MultiOnDiskHashTable.h index bf06a77912..704c6343a1 100644 --- a/lib/Serialization/MultiOnDiskHashTable.h +++ b/lib/Serialization/MultiOnDiskHashTable.h @@ -38,6 +38,7 @@ public: typedef typename Info::external_key_type external_key_type; typedef typename Info::internal_key_type internal_key_type; typedef typename Info::data_type data_type; + typedef typename Info::data_type_builder data_type_builder; typedef unsigned hash_value_type; private: @@ -135,8 +136,9 @@ private: // FIXME: Don't rely on the OnDiskHashTable format here. auto L = InfoObj.ReadKeyDataLength(LocalPtr); const internal_key_type &Key = InfoObj.ReadKey(LocalPtr, L.first); + data_type_builder ValueBuilder(Merged->Data[Key]); InfoObj.ReadDataInto(Key, LocalPtr + L.first, L.second, - Merged->Data[Key]); + ValueBuilder); } Merged->Files.push_back(ODT->File); @@ -218,12 +220,14 @@ public: Result = It->second; } + data_type_builder ResultBuilder(Result); + for (auto *ODT : tables()) { auto &HT = ODT->Table; auto It = HT.find_hashed(Key, KeyHash); if (It != HT.end()) HT.getInfoObj().ReadDataInto(Key, It.getDataPtr(), It.getDataLen(), - Result); + ResultBuilder); } return Result; @@ -233,13 +237,14 @@ public: /// sense if merging values across keys is meaningful. data_type findAll() { data_type Result; + data_type_builder ResultBuilder(Result); if (!PendingOverrides.empty()) removeOverriddenTables(); if (MergedTable *M = getMergedTable()) { for (auto &KV : M->Data) - Info::MergeDataInto(KV.second, Result); + Info::MergeDataInto(KV.second, ResultBuilder); } for (auto *ODT : tables()) { @@ -251,7 +256,7 @@ public: // FIXME: Don't rely on the OnDiskHashTable format here. auto L = InfoObj.ReadKeyDataLength(LocalPtr); const internal_key_type &Key = InfoObj.ReadKey(LocalPtr, L.first); - InfoObj.ReadDataInto(Key, LocalPtr + L.first, L.second, Result); + InfoObj.ReadDataInto(Key, LocalPtr + L.first, L.second, ResultBuilder); } } diff --git a/test/Modules/cxx-templates.cpp b/test/Modules/cxx-templates.cpp index ea6f05d331..fd6b4f5a2b 100644 --- a/test/Modules/cxx-templates.cpp +++ b/test/Modules/cxx-templates.cpp @@ -28,15 +28,15 @@ void g() { f(1.0); f(); f(); // expected-error {{no matching function}} - // expected-note@Inputs/cxx-templates-b.h:3 {{couldn't infer template argument}} - // expected-note-re@Inputs/cxx-templates-a.h:4 {{requires {{single|1}} argument}} + // expected-note@Inputs/cxx-templates-a.h:3 {{couldn't infer template argument}} + // expected-note@Inputs/cxx-templates-a.h:4 {{requires 1 argument}} N::f(0); N::f(1.0); N::f(); N::f(); // expected-error {{no matching function}} // expected-note@Inputs/cxx-templates-b.h:6 {{couldn't infer template argument}} - // expected-note-re@Inputs/cxx-templates-a.h:7 {{requires {{single|1}} argument}} + // expected-note@Inputs/cxx-templates-b.h:7 {{requires single argument}} template_param_kinds_1<0>(); // ok, from cxx-templates-a.h template_param_kinds_1(); // ok, from cxx-templates-b.h diff --git a/test/Modules/merge-using-decls.cpp b/test/Modules/merge-using-decls.cpp index 15d4af4837..98989d12f9 100644 --- a/test/Modules/merge-using-decls.cpp +++ b/test/Modules/merge-using-decls.cpp @@ -33,7 +33,7 @@ template int UseAll(); // Which of these two sets of diagnostics is chosen is not important. It's OK // if this varies with ORDER, but it must be consistent across runs. -#if 1 +#if ORDER == 1 // Here, we're instantiating the definition from 'A' and merging the definition // from 'B' into it.