[Lldb-commits] [lldb] Fix type lookup bug where wrong decl context was being used for a DIE. (PR #94846)

[Pavel Labath via lldb-commits]

================
@@ -0,0 +1,69 @@
+"""
+Test the SBModule and SBTarget type lookup APIs to find multiple types.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TypeFindFirstTestCase(TestBase):
+    def test_find_first_type(self):
+        """
+        Test SBTarget::FindTypes() and SBModule::FindTypes() APIs.
+
+        We had issues where our declaration context when finding types was
+        incorrectly calculated where a type in a namepace, and a type in a
+        function that was also in the same namespace would match a lookup. For
+        example:
+
+            namespace a {
+              struct Foo {
+                int foo;
+              };
+
+              unsigned foo() {
+                typedef unsigned Foo;
+                Foo foo = 12;
+                return foo;
+              }
+            } // namespace a
+
+
+        Previously LLDB would calculate the declaration context of "a::Foo"
+        correctly, but incorrectly calculate the declaration context of "Foo"
+        from within the foo() function as "a::Foo". Adding tests to ensure this
+        works correctly.
+        """
+        self.build()
+        target = self.createTestTarget()
+        exe_module = target.GetModuleAtIndex(0)
+        self.assertTrue(exe_module.IsValid())
+        # Test the SBTarget and SBModule APIs for FindFirstType
+        for api in [target, exe_module]:
+            # We should find the "a::Foo" but not the "Foo" type in the function
+            types = api.FindTypes("a::Foo")
+            self.assertEqual(types.GetSize(), 1)
+            type_str0 = str(types.GetTypeAtIndex(0))
+            self.assertIn('struct Foo', type_str0)
+
+            # When we search by type basename, we should find any type whose
+            # basename matches "Foo", so "a::Foo" and the "Foo" type in the
+            # function.
+            types = api.FindTypes("Foo")
+            self.assertEqual(types.GetSize(), 2)
+            type_str0 = str(types.GetTypeAtIndex(0))
+            type_str1 = str(types.GetTypeAtIndex(1))
+            # We don't know which order the types will come back as, so
+            if 'struct Foo' in type_str0:
+                self.assertIn('typedef Foo', type_str1)
+            else:
+                self.assertIn('struct Foo', type_str1)
+
+            # When we search by type basename with "::" prepended, we should
+            # only types in the root namespace which means only "Foo" type in
----------------
labath wrote:

I would say this is actually a bug, and it's one of the reasons why I said the two functions should be merged (as Michael alluded to in the other comment). I think that a better full name for this type would be something like `::a::foo()::Foo`
I did look into this briefly (before being preempted). Basically, I had this function follow DW_AT_specification links just like it was done for `GetDeclContext`, but I ran into two problems:
- this just gives you a name like `::a::foo::Foo` which, while probably better than `::Foo` is still not fully correct as the type `Foo` in `foo(void)` can be completely different from type `Foo` in `foo(something_else)`.
- it prevented us from accessing the type `Foo` in the expression evaluator, which currently "works" (scare quotes, because it only works if there are no conflicts) because we pretend the type to be global. To fix this we'd need to implement a dedicated method for injecting local types into the expression, just like we do with local variables. I don't think that's particularly hard, but it definitely takes more time that I had at the moment.

https://github.com/llvm/llvm-project/pull/94846