[Lldb-commits] [lldb] Make only one function that needs to be implemented when searching for types (PR #74786)

[Greg Clayton via lldb-commits]

clayborg wrote:

So I traced down why things used to work and why they don't work now. The issue is we are being too accurate with our type lookups now. For my reduced example I had this code:
```
struct A {
  struct B {
    static int f;
  };
};
```
What would happen when you type "p A::B::f" was this:
We would look for 'A' in the root namespace and we would find it, the record decl looks like:
```
CXXRecordDecl 0x105504e08 <<invalid sloc>> <invalid sloc> struct A definition
`-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
  |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
  |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
  |-MoveConstructor exists simple trivial needs_implicit
  |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
  |-MoveAssignment exists simple trivial needs_implicit
  `-Destructor simple irrelevant trivial needs_implicit
```
We would look for 'B' in the root namespace, but since type lookups used to realize all basename matches (where 'B' would match 'A::B') and then we would throw away results that didn't match, but we did end up converting the A::B and it now exists in the 'A' CXXRecordDecl, but the compiler isn't looking for it there, so the expression fails with:
```
error: <user expression 0>:1:4: no member named 'B' in 'A'
    1 | A::B::f
      | ~~~^
```

When you run the expression the second time, the "struct A" now has a forward declaration for "A::B" in the CXXRecordDecl. So the second expression would look for 'A' in the root namespace and we would find it, the record decl 
 now looks like:
```
CXXRecordDecl 0x156b78a08 <<invalid sloc>> <invalid sloc> struct A definition
|-DefinitionData pass_in_registers empty aggregate standard_layout trivially_copyable pod trivial literal has_constexpr_non_copy_move_ctor can_const_default_init
| |-DefaultConstructor exists trivial constexpr needs_implicit defaulted_is_constexpr
| |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveConstructor exists simple trivial needs_implicit
| |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
| |-MoveAssignment exists simple trivial needs_implicit
| `-Destructor simple irrelevant trivial needs_implicit
`-CXXRecordDecl 0x156b78b58 <<invalid sloc>> <invalid sloc> <undeserialized declarations> struct B
```
Note the last line is our definition for 'A::B' that was not there before, but now since it is there, the compiler will find it.

With the current upstream we run and we find "A" and have the same CXXRecordDecl as the first expression in the old LLDB, but now we will ask for "B" in the root namespace and we won't find anything because we are smart enough to  _not_ create every type whose basename matches "B", so we won't create "B", so it won't get added to A's CXXRecordDecl that this causes the expression to never be able to succeed.

So I am surprised to see that the compiler is asking for "B" in the root namespace, and not asking us to find "B" in the "A" decl context. 

But the real issue might be that LLDB, which is acting like a precompiled header, and when asked us to find 'A', we return a forward declaration that is marked as "can be completed lazily via the precompiled header (external AST source) interface. And when we are asked to complete the 'A' type, it might expect us to add all contained types, like "struct A::B". We are not doing that right now, but maybe that is how the actual precompiled header support does things.

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