[ASTImporter] Add importer specific lookup
Summary:
There are certain cases when normal C/C++ lookup (localUncachedLookup)
does not find AST nodes. E.g.:
Example 1:
template <class T>
struct X {
friend void foo(); // this is never found in the DC of the TU.
};
Example 2:
// The fwd decl to Foo is not found in the lookupPtr of the DC of the
// translation unit decl.
struct A { struct Foo *p; };
In these cases we create a new node instead of returning with the old one.
To fix it we create a new lookup table which holds every node and we are
not interested in any C++ specific visibility considerations.
Simply, we must know if there is an existing Decl in a given DC.
Reviewers: a_sidorin, a.sidorin
Subscribers: mgorny, rnkovacs, dkrupp, Szelethus, cfe-commits
Differential Revision: https://reviews.llvm.org/D53708
llvm-svn: 349351
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c67990c..6a893c5 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "clang/AST/ASTImporter.h"
+#include "clang/AST/ASTImporterLookupTable.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/ASTStructuralEquivalence.h"
@@ -134,28 +135,6 @@
To->setIsUsed();
}
- Optional<unsigned> ASTImporter::getFieldIndex(Decl *F) {
- assert(F && (isa<FieldDecl>(*F) || isa<IndirectFieldDecl>(*F)) &&
- "Try to get field index for non-field.");
-
- auto *Owner = dyn_cast<RecordDecl>(F->getDeclContext());
- if (!Owner)
- return None;
-
- unsigned Index = 0;
- for (const auto *D : Owner->decls()) {
- if (D == F)
- return Index;
-
- if (isa<FieldDecl>(*D) || isa<IndirectFieldDecl>(*D))
- ++Index;
- }
-
- llvm_unreachable("Field was not found in its parent context.");
-
- return None;
- }
-
// FIXME: Temporary until every import returns Expected.
template <>
LLVM_NODISCARD Error
@@ -313,12 +292,14 @@
if (ToD)
return true; // Already imported.
ToD = CreateFun(std::forward<Args>(args)...);
+ // Keep track of imported Decls.
+ Importer.MapImported(FromD, ToD);
+ Importer.AddToLookupTable(ToD);
InitializeImportedDecl(FromD, ToD);
return false; // A new Decl is created.
}
void InitializeImportedDecl(Decl *FromD, Decl *ToD) {
- Importer.MapImported(FromD, ToD);
ToD->IdentifierNamespace = FromD->IdentifierNamespace;
if (FromD->hasAttrs())
for (const Attr *FromAttr : FromD->getAttrs())
@@ -2198,8 +2179,7 @@
MergeWithNamespace = cast<NamespaceDecl>(DC)->getAnonymousNamespace();
} else {
SmallVector<NamedDecl *, 4> ConflictingDecls;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_Namespace))
continue;
@@ -2310,8 +2290,7 @@
if (!DC->isFunctionOrMethod()) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
unsigned IDNS = Decl::IDNS_Ordinary;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
@@ -2399,8 +2378,7 @@
if (!DC->isFunctionOrMethod()) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
unsigned IDNS = Decl::IDNS_Ordinary;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
@@ -2502,8 +2480,8 @@
// We may already have an enum of the same name; try to find and match it.
if (!DC->isFunctionOrMethod() && SearchName) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(SearchName, FoundDecls);
+ auto FoundDecls =
+ Importer.findDeclsInToCtx(DC, SearchName);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
@@ -2615,9 +2593,8 @@
RecordDecl *PrevDecl = nullptr;
if (!DC->isFunctionOrMethod()) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(SearchName, FoundDecls);
-
+ auto FoundDecls =
+ Importer.findDeclsInToCtx(DC, SearchName);
if (!FoundDecls.empty()) {
// We're going to have to compare D against potentially conflicting Decls, so complete it.
if (D->hasExternalLexicalStorage() && !D->isCompleteDefinition())
@@ -2634,15 +2611,6 @@
Found = Tag->getDecl();
}
- if (D->getDescribedTemplate()) {
- if (auto *Template = dyn_cast<ClassTemplateDecl>(Found)) {
- Found = Template->getTemplatedDecl();
- } else {
- ConflictingDecls.push_back(FoundDecl);
- continue;
- }
- }
-
if (auto *FoundRecord = dyn_cast<RecordDecl>(Found)) {
// Do not emit false positive diagnostic in case of unnamed
// struct/union and in case of anonymous structs. Would be false
@@ -2838,8 +2806,7 @@
if (!LexicalDC->isFunctionOrMethod()) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
unsigned IDNS = Decl::IDNS_Ordinary;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
@@ -3005,9 +2972,9 @@
FunctionTemplateDecl *FromFT = D->getDescribedFunctionTemplate();
// If this is a function template specialization, then try to find the same
- // existing specialization in the "to" context. The localUncachedLookup
- // below will not find any specialization, but would find the primary
- // template; thus, we have to skip normal lookup in case of specializations.
+ // existing specialization in the "to" context. The lookup below will not
+ // find any specialization, but would find the primary template; thus, we
+ // have to skip normal lookup in case of specializations.
// FIXME handle member function templates (TK_MemberSpecialization) similarly?
if (D->getTemplatedKind() ==
FunctionDecl::TK_FunctionTemplateSpecialization) {
@@ -3025,20 +2992,11 @@
else if (!LexicalDC->isFunctionOrMethod()) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
unsigned IDNS = Decl::IDNS_Ordinary | Decl::IDNS_OrdinaryFriend;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
- // If template was found, look at the templated function.
- if (FromFT) {
- if (auto *Template = dyn_cast<FunctionTemplateDecl>(FoundDecl))
- FoundDecl = Template->getTemplatedDecl();
- else
- continue;
- }
-
if (auto *FoundFunction = dyn_cast<FunctionDecl>(FoundDecl)) {
if (FoundFunction->hasExternalFormalLinkage() &&
D->hasExternalFormalLinkage()) {
@@ -3299,8 +3257,7 @@
return ToD;
// Determine whether we've already imported this field.
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (FieldDecl *FoundField = dyn_cast<FieldDecl>(FoundDecl)) {
// For anonymous fields, match up by index.
@@ -3385,8 +3342,7 @@
return ToD;
// Determine whether we've already imported this field.
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (unsigned I = 0, N = FoundDecls.size(); I != N; ++I) {
if (auto *FoundField = dyn_cast<IndirectFieldDecl>(FoundDecls[I])) {
// For anonymous indirect fields, match up by index.
@@ -3454,7 +3410,7 @@
return std::move(Err);
// Determine whether we've already imported this decl.
- // FriendDecl is not a NamedDecl so we cannot use localUncachedLookup.
+ // FriendDecl is not a NamedDecl so we cannot use lookup.
auto *RD = cast<CXXRecordDecl>(DC);
FriendDecl *ImportedFriend = RD->getFirstFriend();
@@ -3532,8 +3488,7 @@
return ToD;
// Determine whether we've already imported this ivar
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (ObjCIvarDecl *FoundIvar = dyn_cast<ObjCIvarDecl>(FoundDecl)) {
if (Importer.IsStructurallyEquivalent(D->getType(),
@@ -3603,8 +3558,7 @@
if (D->isFileVarDecl()) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
unsigned IDNS = Decl::IDNS_Ordinary;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
@@ -3814,8 +3768,7 @@
if (ToD)
return ToD;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (auto *FoundMethod = dyn_cast<ObjCMethodDecl>(FoundDecl)) {
if (FoundMethod->isInstanceMethod() != D->isInstanceMethod())
@@ -4122,8 +4075,7 @@
return ToD;
ObjCProtocolDecl *MergeWithProtocol = nullptr;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_ObjCProtocol))
continue;
@@ -4548,8 +4500,7 @@
// Look for an existing interface with the same name.
ObjCInterfaceDecl *MergeWithIface = nullptr;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_Ordinary))
continue;
@@ -4724,8 +4675,7 @@
return ToD;
// Check whether we have already imported this property.
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (auto *FoundProp = dyn_cast<ObjCPropertyDecl>(FoundDecl)) {
// Check property types.
@@ -4995,8 +4945,7 @@
// We may already have a template of the same name; try to find and match it.
if (!DC->isFunctionOrMethod()) {
SmallVector<NamedDecl *, 4> ConflictingDecls;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_Ordinary |
Decl::IDNS_TagFriend))
@@ -5304,8 +5253,7 @@
assert(!DC->isFunctionOrMethod() &&
"Variable templates cannot be declared at function scope");
SmallVector<NamedDecl *, 4> ConflictingDecls;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(Decl::IDNS_Ordinary))
continue;
@@ -5537,8 +5485,7 @@
// type, and in the same context as the function we're importing.
if (!LexicalDC->isFunctionOrMethod()) {
unsigned IDNS = Decl::IDNS_Ordinary;
- SmallVector<NamedDecl *, 2> FoundDecls;
- DC->getRedeclContext()->localUncachedLookup(Name, FoundDecls);
+ auto FoundDecls = Importer.findDeclsInToCtx(DC, Name);
for (auto *FoundDecl : FoundDecls) {
if (!FoundDecl->isInIdentifierNamespace(IDNS))
continue;
@@ -7666,12 +7613,14 @@
ASTImporter::ASTImporter(ASTContext &ToContext, FileManager &ToFileManager,
ASTContext &FromContext, FileManager &FromFileManager,
- bool MinimalImport)
- : ToContext(ToContext), FromContext(FromContext),
+ bool MinimalImport,
+ ASTImporterLookupTable *LookupTable)
+ : LookupTable(LookupTable), ToContext(ToContext), FromContext(FromContext),
ToFileManager(ToFileManager), FromFileManager(FromFileManager),
Minimal(MinimalImport) {
- ImportedDecls[FromContext.getTranslationUnitDecl()]
- = ToContext.getTranslationUnitDecl();
+
+ ImportedDecls[FromContext.getTranslationUnitDecl()] =
+ ToContext.getTranslationUnitDecl();
}
ASTImporter::~ASTImporter() = default;
@@ -7682,6 +7631,58 @@
return make_error<ImportError>();
return ToT;
}
+
+Optional<unsigned> ASTImporter::getFieldIndex(Decl *F) {
+ assert(F && (isa<FieldDecl>(*F) || isa<IndirectFieldDecl>(*F)) &&
+ "Try to get field index for non-field.");
+
+ auto *Owner = dyn_cast<RecordDecl>(F->getDeclContext());
+ if (!Owner)
+ return None;
+
+ unsigned Index = 0;
+ for (const auto *D : Owner->decls()) {
+ if (D == F)
+ return Index;
+
+ if (isa<FieldDecl>(*D) || isa<IndirectFieldDecl>(*D))
+ ++Index;
+ }
+
+ llvm_unreachable("Field was not found in its parent context.");
+
+ return None;
+}
+
+ASTImporter::FoundDeclsTy
+ASTImporter::findDeclsInToCtx(DeclContext *DC, DeclarationName Name) {
+ // We search in the redecl context because of transparent contexts.
+ // E.g. a simple C language enum is a transparent context:
+ // enum E { A, B };
+ // Now if we had a global variable in the TU
+ // int A;
+ // then the enum constant 'A' and the variable 'A' violates ODR.
+ // We can diagnose this only if we search in the redecl context.
+ DeclContext *ReDC = DC->getRedeclContext();
+ if (LookupTable) {
+ ASTImporterLookupTable::LookupResult LookupResult =
+ LookupTable->lookup(ReDC, Name);
+ return FoundDeclsTy(LookupResult.begin(), LookupResult.end());
+ } else {
+ // FIXME Can we remove this kind of lookup?
+ // Or lldb really needs this C/C++ lookup?
+ FoundDeclsTy Result;
+ ReDC->localUncachedLookup(Name, Result);
+ return Result;
+ }
+}
+
+void ASTImporter::AddToLookupTable(Decl *ToD) {
+ if (LookupTable)
+ if (auto *ToND = dyn_cast<NamedDecl>(ToD))
+ LookupTable->add(ToND);
+}
+
QualType ASTImporter::Import(QualType FromT) {
if (FromT.isNull())
return {};
@@ -7774,6 +7775,11 @@
}
ToD = *ToDOrErr;
+ // Once the decl is connected to the existing declarations, i.e. when the
+ // redecl chain is properly set then we populate the lookup again.
+ // This way the primary context will be able to find all decls.
+ AddToLookupTable(ToD);
+
// Notify subclasses.
Imported(FromD, ToD);