aprantl added inline comments.

================
Comment at: lldb/source/Core/DumpRegisterValue.cpp:136-137
+  // See if we have made this type before and can reuse it.
+  CompilerType fields_type = ast->GetTypeForIdentifier<clang::CXXRecordDecl>(
+      ConstString(register_type_name.c_str()));
+
----------------
DavidSpickett wrote:
> aprantl wrote:
> > DavidSpickett wrote:
> > > aprantl wrote:
> > > > bulbazord wrote:
> > > > > This seems highly specific to C++... Let's try to find another way to 
> > > > > do this, ideally with `TypeSystem` instead of `TypeSystemClang` and 
> > > > > `clang::CXXRecordDecl`.
> > > > Are we saving a lot of code by going through the clang typesystem here, 
> > > > or would walking the bits and formatting the directly be roughly the 
> > > > same amount of code?
> > > Funny you should mention that.
> > > 
> > > I was initially doing that, but in the RFC it was suggested I use the 
> > > existing printers for C types 
> > > (https://discourse.llvm.org/t/rfc-showing-register-fields-in-lldb/64676/2).
> > >  There's also a hint that this would make assigning to specific fields 
> > > more easy (though that is a very far off goal).
> > > 
> > > This is what the first version looked like:
> > > ```
> > > (lldb) register read cpsr
> > >     cpsr = 0x60001000
> > > | N | Z | C | V | TCO | DIT | UAO | PAN | SS | IL | SSBS | BTYPE | D | A 
> > > | I | F | nRW | EL | SP |
> > > | 0 | 1 | 1 | 0 |  0  |  0  |  0  |  0  | 0  | 0  |  1   |   0   | 0 | 0 
> > > | 0 | 0 |  0  | 0  | 0  |
> > > ```
> > > Of course it would be refined based on line length and field size etc 
> > > etc. A lot of the things the type printers already do, which is why using 
> > > them looked like it would save some effort.
> > > 
> > > Some of that extra formatting code will get written anyway because I want 
> > > to eventually add a register command that will show you the register 
> > > layout. So table formatting is needed either way. That would produce 
> > > something like:
> > > ```
> > > (lldb) register info cpsr
> > > | 31 | 30 | ...
> > > | N  |  Z | ...
> > > ```
> > > 
> > > So overall no there's nothing preventing me from walking the bits. 
> > > Assuming you already have code to format text tables nicely, it would be 
> > > about the same amount of code.
> > > 
> > > I need to check if we have any generic table formatting code, or could 
> > > extract some from somewhere.
> > Sorry I wasn't aware of the history :-)
> Do you have an idea of how much work it would be to make TypeSystem do the 
> same things TypeSystemClang does? Maybe more to the point, would I be trying 
> to make TypeSystem do something it just isn't designed to do.
Fundamentally, creating new types is something so tightly coupled to a specific 
type system that it may not be a good idea to expose this through the generic 
TypeSystem interface.

We could design a generic interface that looks like 
```
CompilerType ty = CreateRecordType({{"field1", CompilerType1}, {"field2", ...}})
```
but here you explicitly want to control packing and bitfields, and so you will 
end up with something that most likely only TypeSystemClang can fulfill anyway.

So the better solution may be along the lines of a register formatting plugin 
that depends on TypeSystemClang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145580/new/

https://reviews.llvm.org/D145580

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to