teemperor created this revision.
teemperor added a reviewer: shafik.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.
With this patch LLDB starts querying the ExternalASTMerger when we look up a
namespace in the translation unit context. Because
Clang's FindExternalVisibleDecls is returning a true/false on Lookup and LLDB
instead uses a list of decls, this also adds some
glue code that passes this bool back to the caller when we have
modern-type-lookup activated. We can't easily emulate
LLDB's API as Clang doesn't directly tell us what declarations it found in the
DeclContext and querying the decl context for potential
declarations could have side-effects (like modifying the cached lookup in some
way).
Note that this doesn't change any code for normal LLDB as we continue to ignore
that return value in normal LLDB (mostly because
I don't know if there is a reason this was implemented this way).
The bigger change in ClangExpressionDeclMap.cpp is necessary as currently never
query the ExternalASTSource when we don't
have a NamespaceMap (which we never have, as that's a non-modern-type-lookup
thing). So instead we just ignore the NameSpaceMap
and continue to the actual ExternalASTSource in this case.
Repository:
rLLDB LLDB
https://reviews.llvm.org/D68464
Files:
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/TestNamespaceWithModernTypeLookup.py
lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/main.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
@@ -279,7 +279,7 @@
///
/// \return
/// True on success; false otherwise.
- void FindExternalVisibleDecls(NameSearchContext &context) override;
+ bool FindExternalVisibleDecls(NameSearchContext &context) override;
/// Find all entities matching a given name in a given module/namespace,
/// using a NameSearchContext to make Decls for them.
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -683,7 +683,7 @@
// Interface for ClangASTSource
-void ClangExpressionDeclMap::FindExternalVisibleDecls(
+bool ClangExpressionDeclMap::FindExternalVisibleDecls(
NameSearchContext &context) {
assert(m_ast_context);
@@ -696,7 +696,7 @@
if (GetImportInProgress()) {
if (log && log->GetVerbose())
LLDB_LOGF(log, "Ignoring a query during an import");
- return;
+ return false;
}
static unsigned int invocation_id = 0;
@@ -732,32 +732,34 @@
context.m_decl_context)));
FindExternalVisibleDecls(context, lldb::ModuleSP(), compiler_decl_ctx,
current_id);
- return;
+ return context.m_decls.size() != 0;
}
- ClangASTImporter::NamespaceMapSP namespace_map =
- m_ast_importer_sp
- ? m_ast_importer_sp->GetNamespaceMap(namespace_context)
- : ClangASTImporter::NamespaceMapSP();
+ // Only do this when we have a ClangASTImporter.
+ if (m_ast_importer_sp) {
+ ClangASTImporter::NamespaceMapSP namespace_map =
+ m_ast_importer_sp->GetNamespaceMap(namespace_context);
- if (!namespace_map)
- return;
+ if (!namespace_map)
+ return context.m_decls.size() != 0;
- if (log && log->GetVerbose())
- log->Printf(" CEDM::FEVD[%u] Inspecting (NamespaceMap*)%p (%d entries)",
- current_id, static_cast<void *>(namespace_map.get()),
- (int)namespace_map->size());
+ if (log && log->GetVerbose())
+ log->Printf(" CEDM::FEVD[%u] Inspecting (NamespaceMap*)%p (%d entries)",
+ current_id, static_cast<void *>(namespace_map.get()),
+ (int)namespace_map->size());
- for (ClangASTImporter::NamespaceMap::iterator i = namespace_map->begin(),
- e = namespace_map->end();
- i != e; ++i) {
- if (log)
- log->Printf(" CEDM::FEVD[%u] Searching namespace %s in module %s",
- current_id, i->second.GetName().AsCString(),
- i->first->GetFileSpec().GetFilename().GetCString());
+ for (ClangASTImporter::NamespaceMap::iterator i = namespace_map->begin(),
+ e = namespace_map->end();
+ i != e; ++i) {
+ if (log)
+ log->Printf(" CEDM::FEVD[%u] Searching namespace %s in module %s",
+ current_id, i->second.GetName().AsCString(),
+ i->first->GetFileSpec().GetFilename().GetCString());
- FindExternalVisibleDecls(context, i->first, i->second, current_id);
+ FindExternalVisibleDecls(context, i->first, i->second, current_id);
+ }
}
+
} else if (isa<TranslationUnitDecl>(context.m_decl_context)) {
CompilerDeclContext namespace_decl;
@@ -768,7 +770,7 @@
current_id);
}
- ClangASTSource::FindExternalVisibleDecls(context);
+ return ClangASTSource::FindExternalVisibleDecls(context);
}
void ClangExpressionDeclMap::FindExternalVisibleDecls(
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -191,7 +191,7 @@
///
/// \param[in] context
/// The NameSearchContext to use when filing results.
- virtual void FindExternalVisibleDecls(NameSearchContext &context);
+ virtual bool FindExternalVisibleDecls(NameSearchContext &context);
clang::Sema *getSema();
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -259,10 +259,17 @@
llvm::SmallVector<NamedDecl *, 4> name_decls;
NameSearchContext name_search_context(*this, name_decls, clang_decl_name,
decl_ctx);
- FindExternalVisibleDecls(name_search_context);
+ bool success = FindExternalVisibleDecls(name_search_context);
SetExternalVisibleDeclsForName(decl_ctx, clang_decl_name, name_decls);
// --g_depth;
m_active_lookups.erase(uniqued_const_decl_name);
+ // The merger properly implements FindExternalVisibleDecls and returns
+ // true/false when it found something. There is still some non-modern-type
+ // lookup code in FindExternalVisibleDecls that could return a wrong value
+ // for success, so let's not trust the return value when we don't use
+ // modern-type-lookup.
+ if (HasMerger())
+ return success || (name_decls.size() != 0);
return (name_decls.size() != 0);
}
@@ -680,7 +687,7 @@
return;
}
-void ClangASTSource::FindExternalVisibleDecls(NameSearchContext &context) {
+bool ClangASTSource::FindExternalVisibleDecls(NameSearchContext &context) {
assert(m_ast_context);
ClangASTMetrics::RegisterVisibleQuery();
@@ -715,8 +722,7 @@
name.GetCString(), context.m_decl_context->getDeclKindName());
}
- if (HasMerger() && !isa<TranslationUnitDecl>(context.m_decl_context)
- /* possibly handle NamespaceDecls here? */) {
+ if (HasMerger()) {
if (auto *interface_decl =
dyn_cast<ObjCInterfaceDecl>(context.m_decl_context)) {
ObjCInterfaceDecl *complete_iface_decl =
@@ -729,9 +735,9 @@
}
}
- GetMergerUnchecked().FindExternalVisibleDeclsByName(context.m_decl_context,
- context.m_decl_name);
- return; // otherwise we may need to fall back
+ if (GetMergerUnchecked().FindExternalVisibleDeclsByName(
+ context.m_decl_context, context.m_decl_name))
+ return true;
}
context.m_namespace_map = std::make_shared<ClangASTImporter::NamespaceMap>();
@@ -747,7 +753,7 @@
static_cast<int>(namespace_map->size()));
if (!namespace_map)
- return;
+ return false;
for (ClangASTImporter::NamespaceMap::iterator i = namespace_map->begin(),
e = namespace_map->end();
@@ -762,7 +768,7 @@
FindObjCPropertyAndIvarDecls(context);
} else if (!isa<TranslationUnitDecl>(context.m_decl_context)) {
// we shouldn't be getting FindExternalVisibleDecls calls for these
- return;
+ return false;
} else {
CompilerDeclContext namespace_decl;
@@ -785,6 +791,7 @@
if (clang_namespace_decl)
clang_namespace_decl->setHasExternalVisibleStorage();
}
+ return false;
}
clang::Sema *ClangASTSource::getSema() {
Index: lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/main.cpp
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/main.cpp
@@ -0,0 +1,5 @@
+namespace A { int i = 42; }
+
+int main(int argc, char **argv) {
+ return A::i; // Set break point at this line.
+}
Index: lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/TestNamespaceWithModernTypeLookup.py
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/TestNamespaceWithModernTypeLookup.py
@@ -0,0 +1,19 @@
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class NamespaceWithModernTypeLookup(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ def test(self):
+ self.build()
+
+ # Activate modern-type-lookup.
+ self.runCmd("settings set target.experimental.use-modern-type-lookup true")
+
+ lldbutil.run_to_source_breakpoint(self,
+ "// Set break point at this line.", lldb.SBFileSpec("main.cpp"))
+
+ # Test lookup in namespace.
+ self.expect("expr A::i", substrs=["(int) ", " = 42\n"])
Index: lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/Makefile
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/modern-type-lookup/namespace-lookup/Makefile
@@ -0,0 +1,2 @@
+CXX_SOURCES := main.cpp
+include Makefile.rules
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
