[lldb] Make SBType::FindDirectNestedType work with expression ASTs #89183
Conversation
|
@llvm/pr-subscribers-lldb Author: Pavel Labath (labath) ChangesThe types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once). Full diff: https://github.com/llvm/llvm-project/pull/89183.diff 5 Files Affected:
diff --git a/lldb/include/lldb/Symbol/TypeSystem.h b/lldb/include/lldb/Symbol/TypeSystem.h
index f647fcbf1636ea..3a927d313b823d 100644
--- a/lldb/include/lldb/Symbol/TypeSystem.h
+++ b/lldb/include/lldb/Symbol/TypeSystem.h
@@ -28,6 +28,7 @@
#include "lldb/Symbol/CompilerDeclContext.h"
#include "lldb/Symbol/Type.h"
#include "lldb/lldb-private.h"
+#include "lldb/lldb-types.h"
class PDBASTParser;
@@ -363,6 +364,12 @@ class TypeSystem : public PluginInterface,
lldb::opaque_compiler_type_t type, llvm::StringRef name,
bool omit_empty_base_classes, std::vector<uint32_t> &child_indexes) = 0;
+ virtual CompilerType
+ GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type,
+ llvm::StringRef name) {
+ return CompilerType();
+ }
+
virtual bool IsTemplateType(lldb::opaque_compiler_type_t type);
virtual size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index be0ddb06f82c18..2621f682011b41 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -6997,6 +6997,36 @@ TypeSystemClang::GetIndexOfChildWithName(lldb::opaque_compiler_type_t type,
return UINT32_MAX;
}
+CompilerType
+TypeSystemClang::GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type,
+ llvm::StringRef name) {
+ if (!type || name.empty())
+ return CompilerType();
+
+ clang::QualType qual_type = RemoveWrappingTypes(GetCanonicalQualType(type));
+ const clang::Type::TypeClass type_class = qual_type->getTypeClass();
+
+ switch (type_class) {
+ case clang::Type::Record: {
+ if (!GetCompleteType(type))
+ return CompilerType();
+ const clang::RecordType *record_type =
+ llvm::cast<clang::RecordType>(qual_type.getTypePtr());
+ const clang::RecordDecl *record_decl = record_type->getDecl();
+
+ clang::DeclarationName decl_name(&getASTContext().Idents.get(name));
+ for (NamedDecl *decl : record_decl->lookup(decl_name)) {
+ if (auto *tag_decl = dyn_cast<clang::TagDecl>(decl))
+ return GetType(getASTContext().getTagDeclType(tag_decl));
+ }
+ break;
+ }
+ default:
+ break;
+ }
+ return CompilerType();
+}
+
bool TypeSystemClang::IsTemplateType(lldb::opaque_compiler_type_t type) {
if (!type)
return false;
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
index 05c303baa41640..cfe5327e14f823 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
@@ -897,6 +897,10 @@ class TypeSystemClang : public TypeSystem {
bool omit_empty_base_classes,
std::vector<uint32_t> &child_indexes) override;
+ CompilerType
+ GetDirectNestedTypeWithName(lldb::opaque_compiler_type_t type,
+ llvm::StringRef name) override;
+
bool IsTemplateType(lldb::opaque_compiler_type_t type) override;
size_t GetNumTemplateArguments(lldb::opaque_compiler_type_t type,
diff --git a/lldb/source/Symbol/Type.cpp b/lldb/source/Symbol/Type.cpp
index 44a24d7178f562..5c49c4726157fb 100644
--- a/lldb/source/Symbol/Type.cpp
+++ b/lldb/source/Symbol/Type.cpp
@@ -1177,20 +1177,7 @@ CompilerType TypeImpl::FindDirectNestedType(llvm::StringRef name) {
if (name.empty())
return CompilerType();
auto type_system = GetTypeSystem(/*prefer_dynamic*/ false);
- auto *symbol_file = type_system->GetSymbolFile();
- if (!symbol_file)
- return CompilerType();
- auto decl_context = type_system->GetCompilerDeclContextForType(m_static_type);
- if (!decl_context.IsValid())
- return CompilerType();
- TypeQuery query(decl_context, ConstString(name),
- TypeQueryOptions::e_find_one);
- TypeResults results;
- symbol_file->FindTypes(query, results);
- TypeSP type_sp = results.GetFirstType();
- if (type_sp)
- return type_sp->GetFullCompilerType();
- return CompilerType();
+ return type_system->GetDirectNestedTypeWithName(m_static_type.GetOpaqueQualType(), name);
}
bool TypeMemberFunctionImpl::IsValid() {
diff --git a/lldb/test/API/python_api/type/TestTypeList.py b/lldb/test/API/python_api/type/TestTypeList.py
index eba5e17355c3f8..b27842eb3be87a 100644
--- a/lldb/test/API/python_api/type/TestTypeList.py
+++ b/lldb/test/API/python_api/type/TestTypeList.py
@@ -18,6 +18,21 @@ def setUp(self):
self.source = "main.cpp"
self.line = line_number(self.source, "// Break at this line")
+ def _find_nested_type_in_Task_pointer(self, pointer_type):
+ self.assertTrue(pointer_type)
+ self.DebugSBType(pointer_type)
+ pointer_info_type = pointer_type.template_args[1]
+ self.assertTrue(pointer_info_type)
+ self.DebugSBType(pointer_info_type)
+
+ pointer_masks1_type = pointer_info_type.FindDirectNestedType("Masks1")
+ self.assertTrue(pointer_masks1_type)
+ self.DebugSBType(pointer_masks1_type)
+
+ pointer_masks2_type = pointer_info_type.FindDirectNestedType("Masks2")
+ self.assertTrue(pointer_masks2_type)
+ self.DebugSBType(pointer_masks2_type)
+
@skipIf(compiler="clang", compiler_version=["<", "17.0"])
def test(self):
"""Exercise SBType and SBTypeList API."""
@@ -151,22 +166,10 @@ def test(self):
invalid_type = task_type.FindDirectNestedType(None)
self.assertFalse(invalid_type)
- # Check that FindDirectNestedType works with types from AST
- pointer = frame0.FindVariable("pointer")
- pointer_type = pointer.GetType()
- self.assertTrue(pointer_type)
- self.DebugSBType(pointer_type)
- pointer_info_type = pointer_type.template_args[1]
- self.assertTrue(pointer_info_type)
- self.DebugSBType(pointer_info_type)
-
- pointer_masks1_type = pointer_info_type.FindDirectNestedType("Masks1")
- self.assertTrue(pointer_masks1_type)
- self.DebugSBType(pointer_masks1_type)
-
- pointer_masks2_type = pointer_info_type.FindDirectNestedType("Masks2")
- self.assertTrue(pointer_masks2_type)
- self.DebugSBType(pointer_masks2_type)
+ # Check that FindDirectNestedType works with types from module and
+ # expression ASTs.
+ self._find_nested_type_in_Task_pointer(frame0.FindVariable("pointer").GetType())
+ self._find_nested_type_in_Task_pointer(frame0.EvaluateExpression("pointer").GetType())
# We'll now get the child member 'id' from 'task_head'.
id = task_head.GetChildMemberWithName("id")
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
✅ With the latest revision this PR passed the Python code formatter. |
|
In terms of #68705, I think this will fix the issue where the data formatter works on One thing I'd like to get feedback on is whether this should be looking into base classes. The discussion on the original PR focuses on lexically nested types, but going through base classes (following usual language rules) is another form of nesting. Both the existing and the new implementations don't do that, but I think it would be more natural to do it. Of course, then the function can definitely return more than one result... |
The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once).
What you're describing here is in line with member lookup described in the Standard. While it might be a useful facility, it's at odds with the intent of |
I don't expect the simple Re. looking at base classes, don't have a strong opinion on it. There's some precedent for looking at base classes already in |
Agreed. This should be the same lookup that clang does every time is sees a
That's makes sense. Even if it was not the intention, the API makes this the only reasonable implementation. I have no use for the base-class traversal (may change in the future). If there's is a need for it, we can add a different API. |
|
@Michael137 suggested that I check that performance of |
Cool. Thanks for confirming that. |
…lvm#89183) The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once).
The types we get out of expressions will not have an associated symbol file, so the current method of looking up the type will fail. Instead, I plumb the query through the TypeSystem class. This correctly finds the type in both cases (importing it into the expression AST if needed). I haven't measured, but it should also be more efficient than doing a type lookup (at least, after the type has already been found once).