[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:

> So I don't know of anyone that would use the function name when asking for a type, even if that type was defined in a function.

It's certainly not valid C++, if that's what you mean (c++ just doesn't give you the option of referring to a local type from outside of the function), but I don't think this kind of qualification is a revolutionary concept. For example if the aforementioned local struct had a constructor, it's mangled name would be `_ZZN1a3fooEvEN3FooC1Ev `, that is `a::foo()::Foo::Foo()`. Gdb (AFAICT) does not support these kinds of lookups, but it also does not return the type for query like `::Foo`. (instead gdb supports searching for a type in a specific block/function, which is kind of similar to this)

> To make the expression parser work, we should be able to find all of the types using a context like just "Foo", where we would find both `::Foo` and the `Foo` from `a::foo()`, but sort the results based on the CompilerDeclContext of each type that was returned. If we are evaluating an expression in `a::foo()`, the `Foo` type from that function could tell us the CompilerDeclContext it was defined in and we would find the closest match to where the expression is being run.

This approach has the problem that a type (or really, anything) defined with the same name at a class level would take precedence over a local declaration.

For example with code like:
```
struct Foo { int one; }
class Class {
  struct Foo { int two; }
  void Method() {
    struct Foo { int three; }
  }
};
```
if we were stopped in `Class::Method`, then the name `Foo` should refer to the third definition with that name, but I believe this will end up using the second one because clang will see the class-level declaration first and will not bother asking for other possible definitions. I think we used to use the same approach for local variables, but then we changed the implementation because of the same shadowing problem.

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