This is an archive of the discontinued LLVM Phabricator instance.

[lldb][NFCI] Minor cleanups to StructuredData::GetObjectForDotSeparatedPath
ClosedPublic

Authored by bulbazord on Jul 5 2023, 11:23 AM.

Details

Summary

This accomplishes a few minor things:

  • Removed unnecessary uses of this->
  • Removed an unnecessary std::string allocation.
  • Removed some nesting to improve readability using early returns where it makes sense.
  • Replaced strtoul with llvm::to_integer which avoids another std::string allocation.
  • Removed braces from single statement conditions, removed else-after-returns.

Diff Detail

Event Timeline

bulbazord created this revision.Jul 5 2023, 11:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 5 2023, 11:23 AM
bulbazord requested review of this revision.Jul 5 2023, 11:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 5 2023, 11:23 AM
fdeazeve added inline comments.Jul 5 2023, 11:27 AM
lldb/source/Utility/StructuredData.cpp
120

Shouldn't we remove this else?

bulbazord added inline comments.Jul 5 2023, 11:29 AM
lldb/source/Utility/StructuredData.cpp
120

Ah, the first if does return no matter what, so yeah this else isn't necessary.

bulbazord updated this revision to Diff 537451.Jul 5 2023, 11:37 AM

Remove unneeded else

fdeazeve accepted this revision.Jul 5 2023, 11:38 AM

LGTM! Thanks for improving this

This revision is now accepted and ready to land.Jul 5 2023, 11:38 AM
bulbazord marked an inline comment as done.Jul 5 2023, 11:44 AM
mib accepted this revision.Jul 5 2023, 1:02 PM

LGTM with nits.

lldb/source/Utility/StructuredData.cpp
123–124
132
bulbazord added inline comments.Jul 5 2023, 1:04 PM
lldb/source/Utility/StructuredData.cpp
123–124

Good catch.

bulbazord updated this revision to Diff 537523.Jul 5 2023, 3:12 PM

Address feedback from @mib

bulbazord marked 2 inline comments as done.Jul 5 2023, 3:13 PM