philnik added inline comments.

================
Comment at: libcxx/utils/gdb/libcxx/printers.py:192
 
 class StdStringPrinter(object):
     """Print a std::string."""
----------------
dblaikie wrote:
> philnik wrote:
> > jgorbe wrote:
> > > Mordante wrote:
> > > > philnik wrote:
> > > > > Mordante wrote:
> > > > > > philnik wrote:
> > > > > > > Mordante wrote:
> > > > > > > > Does this also break the LLDB pretty printer?
> > > > > > > Probably. Would be nice to have a test runner for that.
> > > > > > I already planned to look into that, D97044#3440904 ;-)
> > > > > Do you know where I would have to look to know what the LLDB pretty 
> > > > > printers do?
> > > > Unfortunately no. @jingham seems to be the Data formatter code owner.
> > > There was a recent lldb change fixing prettyprinters after a similar 
> > > change to string: 
> > > https://github.com/llvm/llvm-project/commit/45428412fd7c9900d3d6ac9803aa7dcf6adfa6fe
> > > 
> > > If the gdb prettyprinter needed fixing for this change, chances are that 
> > > lldb will need a similar update too.
> > Could someone from #lldb help me figure out what to change in the pretty 
> > printers? I looked at the file, but I don't really understand how it works 
> > and TBH I don't really feel like spending a lot of time figuring it out. If 
> > nobody says anything I'll land this in a week.
> > 
> > As a side note: it would be really nice if there were a few more comments 
> > inside `LibCxx.cpp` to explain what happens there. That would make fixing 
> > the pretty printer a lot easier. The code is probably not very hard (at 
> > least it doesn't look like it), but I am looking for a few things that I 
> > can't find and I have no idea what some of the things mean.
> Looks like something around 
> https://github.com/llvm/llvm-project/blob/2e6ac54cf48aa04f7b05c382c33135b16d3f01ea/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp#L597
>  (& the similar masking in the `else` block a few lines down) - I guess a 
> specific lookup for the new field would be needed, rather than the bitmasking.
Yes, but what do the numbers in `size_mode_locations` mean? Why is there no 
checking if it's big or little endian? Is it unsupported maybe? Does it work 
because of something else? Is there a reason that `g_data_name` exists instead 
of comparing directly? Should I add another layout style or should I just 
update the code for the new layout?
I don't know anything about the LLDB codebase, so I don't understand the code 
and I don't know how I should change it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123580

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

Reply via email to