This is an archive of the discontinued LLVM Phabricator instance.

[lldb][NFC] Move Address and AddressRange functions out of Stream and let them take raw_ostream
ClosedPublic

Authored by teemperor on Dec 5 2019, 2:15 AM.

Details

Summary

Yet another step on the long road towards getting rid of lldb's Stream class.

We probably should just make this some kind of member of Address/AddressRange, but it seems quite often we just push
in random integers in there and this is just about getting rid of Stream and not improving arbitrary APIs.

I had to rename another DumpAddress function in FormatEntity that is dumping the content of an address to make Clang happy.

Diff Detail

Event Timeline

teemperor created this revision.Dec 5 2019, 2:15 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 5 2019, 2:15 AM
labath added a comment.Dec 5 2019, 2:32 AM

All our namespace names are snake_case, so this new namespace should be too. Though, honestly, I don't think this needs to be a namespace -- I'd probably just make the function names a bit more descriptive... Maybe DumpAddress/DumpAddressRange, to match the existing DumpRegisterValue/DumpDataExtractor ?

lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-basic/TestBasicVector.py
23–53 ↗(On Diff #232286)

huh ?

teemperor updated this revision to Diff 232301.Dec 5 2019, 3:42 AM
teemperor retitled this revision from [lldb][NFC] Move Address and AddressRange functions out of Stream into namespace and let them take raw_ostream to [lldb][NFC] Move Address and AddressRange functions out of Stream and let them take raw_ostream.
teemperor edited the summary of this revision. (Show Details)
  • Removed unrelated change to std-vector test.
  • Moved from namespace to Dump* prefix.
  • Renamed some function in FormatEntity that already had that name and made Clang very unhappy.
teemperor marked 2 inline comments as done.Dec 5 2019, 3:42 AM
teemperor added inline comments.
lldb/packages/Python/lldbsuite/test/commands/expression/import-std-module/vector-basic/TestBasicVector.py
23–53 ↗(On Diff #232286)

Just checking if you actually look at my patches :)

labath accepted this revision.Dec 5 2019, 3:57 AM

Let's ship it.

lldb/source/Utility/Stream.cpp
89

The "formatv" way of doing this would be via something like formatv("{0}0x{1:x}{2}", prefix, fmt_align(addr, AlignStyle::Right, addr_size*2, '0'), suffix), but that's quite a mouthful so I'd probably go for just s << prefix << format_hex(addr, 2+2*addr_size) << suffix;

This revision is now accepted and ready to land.Dec 5 2019, 3:57 AM
teemperor updated this revision to Diff 232322.Dec 5 2019, 5:20 AM
teemperor marked an inline comment as done.
  • Moved to format_hex.
  • Removed a now failing part of StreamTest that tested printing an address on a 160 bit system (which LLDB doesn't support.... yet).
This revision was automatically updated to reflect the committed changes.