DavidSpickett wrote:

> For fields and enums that was very easy, but we want to add support for 
> structs and unions soon.

It's pretty easy for unions too - 
https://github.com/llvm/llvm-project/pull/196032/changes/4d5ac3881496f824b51ff60ae193f59d37ad463a.
 At least when all union members are other RegisterTypes, I did not implement 
union of basic types like int/float.

(I see what you mean though, easy at first doesn't mean easy long term or free 
of tech debt, much like manually dumping the values could have been "easy" but 
would quickly get out of hand)

> My idea is to avoid making our own custom type system like types and have the 
> RegisterType make its own CompilerType using clang AST types much like DWARF 
> is converted to AST types.

What I have now is `c++ representation of XML types` -> `XML` -> `c++ 
representation of types` -> `CompilerType` so if I could remove a few layers 
that'd be great.

I have a few concerns about doing that:
* I think we already link in CompilerType into lldb-server, but I'm not sure we 
use it and whether we'd need to link in a TypeSystem to use it. If we have 
already paid that cost / are ok paying it, fine.
* Whether XML to CompilerType would end up being lossy for some types, 
specifically whether exact bit layouts can be preserved. In 
https://github.com/llvm/llvm-project/pull/189590 I was able to provide the 
layout via. a callback but I think that's only called when you go to dump the 
value. So if we had a CompilerType for a bitfiield struct (aka `flags`), we 
would need to have some side data that has the exact layout. Though the class 
would still be more compact than it is now, CompilerType + list of name/range 
pairs.

> We can easily make a ValueObjectRegister from and RegisterInfo and have it be 
> able to use the ValueObject code to display iteself instead of the custom 
> DumpRegisterValue.cpp and DumpRegisterInfo.cpp.

I don't think that that much of DumpRegisterValue would be removed by doing 
this, unless you're saying we could move the whole of 
`lldb_private::DumpRegisterValue` into the ValueObject which would be a more 
logical place for it.

(as I think the raw value is always going to have its uses)

I'm not sure if you are suggesting that we use the normal type printing for 
register info, or just put the custom formatting logic into the ValueObjects, 
but I do want to be clear that at least for `flags` I do not want  to print the 
default C type for , as it doesn't show the bit ranges which are often useful 
for low level work.

For example:
```
(lldb) register info por
       Name: por
       Size: 8 bytes (64 bits)
    In sets: Permission Overlay Registers (index 7)

| 63-60  | 59-56  | 55-52  | 51-48  | 47-44  | 43-40  | 39-36 | 35-32 | 31-28 | 
27-24 | 23-20 | 19-16 | 15-12 | 11-8  |  7-4  |  3-0  |
|--------|--------|--------|--------|--------|--------|-------|-------|-------|-------|-------|-------|-------|-------|-------|-------|
| Perm15 | Perm14 | Perm13 | Perm12 | Perm11 | Perm10 | Perm9 | Perm8 | Perm7 | 
Perm6 | Perm5 | Perm4 | Perm3 | Perm2 | Perm1 | Perm0 |

Perm15: 0 = No Access, 1 = Read, 2 = Execute, 3 = Read, Execute, 4 = Write, 5 = 
Write, Read, 6 = Write, Execute, 7 = Read, Write, Execute
<...>
Perm0: 0 = No Access, 1 = Read, 2 = Execute, 3 = Read, Execute, 4 = Write, 5 = 
Write, Read, 6 = Write, Execute, 7 = Read, Write, Execute
```
Yes, it does print the same enum 15 times, once for each field that uses it 
:rofl: This is a good sign that my manual printing code, while I think it has 
good intentions, is going to fall apart when we start nesting types.

So the extra bits we need are the field enums, which can be a pre or post print 
thing, and the bit positions. Perhaps we can extend the normal printing with 
annotations like:
```
struct por {
  uint64_t Perm15:4; // [63-60]
<...>
```
For `vector` and `union` I expect the default path would be fine. Laying it all 
out is a challenge, so reusing the existing code would really help with that, 
as it has done for value printing.

I also want one day (:unicorn: ) to add descriptions for registers and register 
`flags` fields. The former is easy, it's just header text, the latter might get 
awkward but it's just a future thought so I won't make that a blocker here.

If we really found that we had two audiences:
* People who want the C type in C form, or at least a "compact" form
* People who want the equivalent of the architecture manual, with text and 
tables and so on

And we can't do a good job for both in one command, `register type` could be 
the compact one, or we can tell people how to `expr typeof($register)` to see 
the raw C type when they need to make expressions.

So in summary:
* Sounds great if we can keep `register info` in a useful format.
* I do not have the time at the moment to implement the `CompilerType` approach 
myself.
* If there were a prototype where value printing worked, I will make the time 
to deal with the `register info` part (since I am the one most concerned with 
it).

https://github.com/llvm/llvm-project/pull/196960
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to