Michael137 created this revision.
Michael137 added reviewers: aprantl, labath.
Herald added a project: All.
Michael137 updated this revision to Diff 510478.
Michael137 added a comment.
Michael137 updated this revision to Diff 510482.
Michael137 edited the summary of this revision.
Michael137 updated this revision to Diff 510489.
Michael137 published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
- Add docs
Michael137 added a comment.
- Merge into single API
Michael137 added a comment.
- Adjust test-case. Add FIXME
**Summary**
In a program such as:
namespace A {
namespace B {
struct Bar {};
}
}
namespace B {
struct Foo {};
}
...LLDB would run into issues such as:
(lldb) expr ::B::Foo f
error: expression failed to parse:
error: <user expression 0>:1:6: no type named 'Foo' in namespace 'A::B'
::B::Foo f
~~~~~^
This is because the `SymbolFileDWARF::FindNamespace` implementation
will return *any* namespace it finds if the `parent_decl_ctx` provided
is empty. In `FindExternalVisibleDecls` we use this API to find the
namespace that symbol `B` refers to. If `A::B` happened to be the one
that `SymbolFileDWARF::FindNamespace` looked at first, we would try
to find `struct Foo` in `A::B`. Hence the error.
This patch proposes a new flag to `SymbolFileDWARF::FindNamespace`
allows us to only consider top-level namespaces in our search, which is what
`FindExternalVisibleDecls` is attempting anyway; it just never
accounted for multiple namespaces of the same name.
**Testing**
- Added API test-case
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D147436
Files:
lldb/include/lldb/Symbol/SymbolFile.h
lldb/include/lldb/Symbol/SymbolFileOnDemand.h
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
lldb/source/Symbol/SymbolFileOnDemand.cpp
lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
lldb/test/API/lang/cpp/namespace/TestNamespace.py
lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
lldb/test/API/lang/cpp/namespace/main.cpp
Index: lldb/test/API/lang/cpp/namespace/main.cpp
===================================================================
--- lldb/test/API/lang/cpp/namespace/main.cpp
+++ lldb/test/API/lang/cpp/namespace/main.cpp
@@ -102,6 +102,18 @@
return myfunc2(3) + j + i + a + 2 + anon_uint + a_uint + b_uint + y_uint; // Set break point at this line.
}
+namespace B {
+struct Bar {
+ int x() { return 42; }
+};
+} // namespace B
+
+namespace A::B {
+struct Bar {
+ int y() { return 137; }
+};
+} // namespace A::B
+
int
main (int argc, char const *argv[])
{
@@ -112,5 +124,7 @@
A::B::test_lookup_at_nested_ns_scope_after_using();
test_lookup_before_using_directive();
test_lookup_after_using_directive();
- return Foo::myfunc(12);
+ ::B::Bar bb;
+ A::B::Bar ab;
+ return Foo::myfunc(12) + bb.x() + ab.y();
}
Index: lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
===================================================================
--- lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespaceLookup.py
@@ -159,8 +159,8 @@
self.runToBkpt("continue")
# Evaluate func(10) - should call A::func(int)
self.expect_expr("func(10)", result_type="int", result_value="13")
- # Evaluate B::func() - should call B::func()
- self.expect_expr("B::func()", result_type="int", result_value="4")
+ # FIXME: under C++ rules this should call A::B::func() since we're at namespace scope of 'A'.
+ self.expect("expr B::func()", error=True, substrs=["no member named 'func' in namespace 'B'"])
# Evaluate func() - should call A::func()
self.expect_expr("func()", result_type="int", result_value="3")
@@ -187,14 +187,14 @@
# Continue to BP_before_using_directive at global scope before using
# declaration
self.runToBkpt("continue")
- # Evaluate B::func() - should call B::func()
- self.expect_expr("B::func()", result_type="int", result_value="4")
+ # Evaluate B::func() - should error out since there is no definition for it
+ self.expect("expr B::func()", error=True, substrs=["no member named 'func' in namespace 'B'"])
# Continue to BP_after_using_directive at global scope after using
# declaration
self.runToBkpt("continue")
- # Evaluate B::func() - should call B::func()
- self.expect_expr("B::func()", result_type="int", result_value="4")
+ # FIXME: under C++ rules this should call A::B::func() since we imported namespace 'A'
+ self.expect("expr B::func()", error=True, substrs=["no member named 'func' in namespace 'B'"])
@expectedFailure("lldb scope lookup of functions bugs")
def test_function_scope_lookup_with_run_command(self):
Index: lldb/test/API/lang/cpp/namespace/TestNamespace.py
===================================================================
--- lldb/test/API/lang/cpp/namespace/TestNamespace.py
+++ lldb/test/API/lang/cpp/namespace/TestNamespace.py
@@ -225,3 +225,6 @@
self.expect("expression variadic_sum", patterns=[
'\(anonymous namespace\)::variadic_sum\(int, ...\)'])
+
+ self.expect_expr("::B::Bar b; b.x()", result_type="int", result_value="42")
+ self.expect_expr("A::B::Bar b; b.y()", result_type="int", result_value="137")
Index: lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
===================================================================
--- lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
+++ lldb/test/API/commands/expression/inline-namespace/TestInlineNamespace.py
@@ -46,7 +46,7 @@
self.expect("expr A::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A'"])
self.expect("expr A::B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
- self.expect("expr B::other_var", error=True, substrs=["no member named 'other_var' in namespace 'A::B'"])
+ self.expect("expr B::other_var", error=True, substrs=["use of undeclared identifier 'B'"])
# 'frame variable' can correctly distinguish between A::B::global_var and A::global_var
gvars = self.target().FindGlobalVariables("A::global_var", 10)
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===================================================================
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -483,13 +483,16 @@
CompilerDeclContext
SymbolFileOnDemand::FindNamespace(ConstString name,
- const CompilerDeclContext &parent_decl_ctx) {
+ const CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces) {
if (!m_debug_info_enabled) {
LLDB_LOG(GetLog(), "[{0}] {1}({2}) is skipped", GetSymbolFileName(),
__FUNCTION__, name);
- return SymbolFile::FindNamespace(name, parent_decl_ctx);
+ return SymbolFile::FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
}
- return m_sym_file_impl->FindNamespace(name, parent_decl_ctx);
+ return m_sym_file_impl->FindNamespace(name, parent_decl_ctx,
+ only_root_namespaces);
}
std::vector<std::unique_ptr<lldb_private::CallEdge>>
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===================================================================
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -157,9 +157,10 @@
llvm::Expected<lldb::TypeSystemSP>
GetTypeSystemForLanguage(lldb::LanguageType language) override;
- lldb_private::CompilerDeclContext FindNamespace(
- lldb_private::ConstString name,
- const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+ lldb_private::CompilerDeclContext
+ FindNamespace(lldb_private::ConstString name,
+ const lldb_private::CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces) override;
llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -1697,7 +1697,7 @@
lldb_private::CompilerDeclContext
SymbolFilePDB::FindNamespace(lldb_private::ConstString name,
- const CompilerDeclContext &parent_decl_ctx) {
+ const CompilerDeclContext &parent_decl_ctx, bool) {
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
auto type_system_or_err =
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
===================================================================
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
@@ -152,9 +152,9 @@
llvm::Expected<lldb::TypeSystemSP>
GetTypeSystemForLanguage(lldb::LanguageType language) override;
- CompilerDeclContext
- FindNamespace(ConstString name,
- const CompilerDeclContext &parent_decl_ctx) override;
+ CompilerDeclContext FindNamespace(ConstString name,
+ const CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces) override;
llvm::StringRef GetPluginName() override { return GetPluginNameStatic(); }
Index: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -2133,9 +2133,8 @@
TypeClass type_mask,
lldb_private::TypeList &type_list) {}
-CompilerDeclContext
-SymbolFileNativePDB::FindNamespace(ConstString name,
- const CompilerDeclContext &parent_decl_ctx) {
+CompilerDeclContext SymbolFileNativePDB::FindNamespace(
+ ConstString name, const CompilerDeclContext &parent_decl_ctx, bool) {
return {};
}
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
@@ -136,9 +136,10 @@
lldb_private::LanguageSet languages,
llvm::DenseSet<lldb_private::SymbolFile *> &searched_symbol_files,
lldb_private::TypeMap &types) override;
- lldb_private::CompilerDeclContext FindNamespace(
- lldb_private::ConstString name,
- const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+ lldb_private::CompilerDeclContext
+ FindNamespace(lldb_private::ConstString name,
+ const lldb_private::CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces) override;
void GetTypes(lldb_private::SymbolContextScope *sc_scope,
lldb::TypeClass type_mask,
lldb_private::TypeList &type_list) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -1256,13 +1256,14 @@
}
CompilerDeclContext SymbolFileDWARFDebugMap::FindNamespace(
- lldb_private::ConstString name,
- const CompilerDeclContext &parent_decl_ctx) {
+ lldb_private::ConstString name, const CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces) {
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
CompilerDeclContext matching_namespace;
ForEachSymbolFile([&](SymbolFileDWARF *oso_dwarf) -> bool {
- matching_namespace = oso_dwarf->FindNamespace(name, parent_decl_ctx);
+ matching_namespace =
+ oso_dwarf->FindNamespace(name, parent_decl_ctx, only_root_namespaces);
return (bool)matching_namespace;
});
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -214,9 +214,18 @@
llvm::Expected<lldb::TypeSystemSP>
GetTypeSystemForLanguage(lldb::LanguageType language) override;
- lldb_private::CompilerDeclContext FindNamespace(
- lldb_private::ConstString name,
- const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+ /// Checks the DWARF index for a namespace of name \ref name
+ /// and whose parent context is \ref parent_decl_ctx.
+ ///
+ /// If \code{.cpp} !parent_decl_ctx.IsValid() \endcode
+ /// then this function will consider all namespaces that
+ /// match the name. If \ref only_root_namespaces is
+ /// true, only consider in the search those DIEs that
+ /// represent top-level namespaces.
+ lldb_private::CompilerDeclContext
+ FindNamespace(lldb_private::ConstString name,
+ const lldb_private::CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces) override;
void PreloadSymbols() override;
@@ -274,7 +283,7 @@
static bool
DIEInDeclContext(const lldb_private::CompilerDeclContext &parent_decl_ctx,
- const DWARFDIE &die);
+ const DWARFDIE &die, bool only_root_namespaces = false);
std::vector<std::unique_ptr<lldb_private::CallEdge>>
ParseCallEdgesInFunction(lldb_private::UserID func_id) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2324,12 +2324,17 @@
}
bool SymbolFileDWARF::DIEInDeclContext(const CompilerDeclContext &decl_ctx,
- const DWARFDIE &die) {
+ const DWARFDIE &die,
+ bool only_root_namespaces) {
// If we have no parent decl context to match this DIE matches, and if the
// parent decl context isn't valid, we aren't trying to look for any
// particular decl context so any die matches.
+ //
+ // But if we are only checking root decl contexts, confirm that the
+ // 'die' is a top-level context.
if (!decl_ctx.IsValid())
- return true;
+ return !only_root_namespaces ||
+ die.GetParent().Tag() == dwarf::DW_TAG_compile_unit;
if (die) {
if (DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU())) {
@@ -2632,7 +2637,8 @@
CompilerDeclContext
SymbolFileDWARF::FindNamespace(ConstString name,
- const CompilerDeclContext &parent_decl_ctx) {
+ const CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces) {
std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
Log *log = GetLog(DWARFLog::Lookups);
@@ -2648,7 +2654,7 @@
return namespace_decl_ctx;
m_index->GetNamespaces(name, [&](DWARFDIE die) {
- if (!DIEInDeclContext(parent_decl_ctx, die))
+ if (!DIEInDeclContext(parent_decl_ctx, die, only_root_namespaces))
return true; // The containing decl contexts don't match
DWARFASTParser *dwarf_ast = GetDWARFParser(*die.GetCU());
Index: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
===================================================================
--- lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
+++ lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
@@ -134,9 +134,9 @@
llvm::inconvertibleErrorCode());
}
- CompilerDeclContext
- FindNamespace(ConstString name,
- const CompilerDeclContext &parent_decl_ctx) override {
+ CompilerDeclContext FindNamespace(ConstString name,
+ const CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces) override {
return CompilerDeclContext();
}
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===================================================================
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -700,7 +700,8 @@
if (!symbol_file)
continue;
- found_namespace_decl = symbol_file->FindNamespace(name, namespace_decl);
+ found_namespace_decl = symbol_file->FindNamespace(
+ name, namespace_decl, /* only root namespaces */ true);
if (found_namespace_decl) {
context.m_namespace_map->push_back(
@@ -1673,8 +1674,8 @@
if (!symbol_file)
continue;
- found_namespace_decl =
- symbol_file->FindNamespace(name, null_namespace_decl);
+ found_namespace_decl = symbol_file->FindNamespace(
+ name, null_namespace_decl, /* only root namespaces */ true);
if (!found_namespace_decl)
continue;
Index: lldb/include/lldb/Symbol/SymbolFileOnDemand.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolFileOnDemand.h
+++ lldb/include/lldb/Symbol/SymbolFileOnDemand.h
@@ -171,9 +171,10 @@
llvm::Expected<lldb::TypeSystemSP>
GetTypeSystemForLanguage(lldb::LanguageType language) override;
- lldb_private::CompilerDeclContext FindNamespace(
- lldb_private::ConstString name,
- const lldb_private::CompilerDeclContext &parent_decl_ctx) override;
+ lldb_private::CompilerDeclContext
+ FindNamespace(lldb_private::ConstString name,
+ const lldb_private::CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces) override;
std::vector<std::unique_ptr<lldb_private::CallEdge>>
ParseCallEdgesInFunction(UserID func_id) override;
Index: lldb/include/lldb/Symbol/SymbolFile.h
===================================================================
--- lldb/include/lldb/Symbol/SymbolFile.h
+++ lldb/include/lldb/Symbol/SymbolFile.h
@@ -327,7 +327,8 @@
GetTypeSystemForLanguage(lldb::LanguageType language) = 0;
virtual CompilerDeclContext
- FindNamespace(ConstString name, const CompilerDeclContext &parent_decl_ctx) {
+ FindNamespace(ConstString name, const CompilerDeclContext &parent_decl_ctx,
+ bool only_root_namespaces = false) {
return CompilerDeclContext();
}
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
