[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)

[Zequan Wu via lldb-commits]

ZequanWu wrote:

> > > How does this patch fit with the:
> > > ```
> > >    -p ( --skip-pointers )
> > >         Don't use this format for pointers-to-type objects.
> > > 
> > >    -r ( --skip-references )
> > >         Don't use this format for references-to-type objects.
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > settings in `type <formatter> add`? It seems like we let the formatters control whether they should also apply to pointers and references to those types. Shouldn't that be what controls whether formatters also apply to pointers?
> > > And if a formatter says it can handle pointers, it's up to the formatter to do that right.
> > 
> > 
> > This has no impact for reference. For pointers, if `-p` is set in `type <formatter> add`, the new formatter will apply only the type T and this patch has no impact for that. This patch changes the behaviour when `-p` is not set. Currently, lldb apply the new formatter to `T`, `T*`, `T**` and so on if `-p` is not set. This change let lldb only apply the formatter to `T` and `T*`.
> 
> If you write the formatter knowing that you've asked for a match against all the pointers to this type, it's up to the formatter to not assume it's only being passed T or T*. I still don't see why we should be cutting this off in lldb. If I have a std::vector variable, I want to see its size in the variable printing, regardless of whether the handle to it is a pointer, or a pointer to a pointer. It seems artificial to cut that off for me.

The reason that we should only dereference once is that many formatters relies on `ValueObject::GetChildMemberWithName` to calculate the synthetic children or summary strings. For C/C++, that function eventually calls `TypeSystemClang::GetIndexOfChildMemberWithName` which will only dereference pointer type once. This will cause inconsistent result when printing `T*` and `T**` if they share the same data formatters for `T`.

For example, when using the builtin libc++ std::vector formatter, lldb shows inconsistent size count for `std::vector<int> *` and `std::vector<int> **` (if you use libstd++ std::vector, you can still see the inconsistence):
```
Process 2570716 launched: '/tmp/a.out' (x86_64)
Process 2570716 stopped
* thread #1, name = 'a.out', stop reason = breakpoint 1.1
    frame #0: 0x000055555555523e a.out`main at main.cpp:7:3
   4      std::vector<int> v = {1,2};
   5      auto vp = &v;
   6      auto vpp = &vp;
-> 7      return 0;
   8    }
(lldb) v vp vpp
(std::vector<int> *) vp = 0x00007fffffffd520 size=2
(std::vector<int> **) vpp = 0x00007fffffffd500 size=1
```

> If anything, we should allow --skip-pointers 1 for formatter writers that only want to handle one level of pointer to the type.

If we really want to have the formatter apply to all pointer types (no matter how many layers there are), it can be achieved with regex like `type summary add -x -s "MyInt" "int[ ]*[\*]*"`. 

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