Author: Michał Górny Date: 2021-10-22T12:27:46+02:00 New Revision: 66e06cc8cba3c39c760082a8ed469b5292f9ee67
URL: https://github.com/llvm/llvm-project/commit/66e06cc8cba3c39c760082a8ed469b5292f9ee67 DIFF: https://github.com/llvm/llvm-project/commit/66e06cc8cba3c39c760082a8ed469b5292f9ee67.diff LOG: [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions Optimize the iterator comparison logic to compare Current.data() pointers. Use std::tie for assignments from std::pair. Replace the custom class with a function returning iterator_range. Differential Revision: https://reviews.llvm.org/D110535 Added: Modified: lldb/source/Host/common/File.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp llvm/include/llvm/ADT/StringExtras.h llvm/lib/IR/DataLayout.cpp llvm/unittests/ADT/StringExtrasTest.cpp Removed: ################################################################################ diff --git a/lldb/source/Host/common/File.cpp b/lldb/source/Host/common/File.cpp index 1c8a0ab7f83e1..5ad5ae6bb2192 100644 --- a/lldb/source/Host/common/File.cpp +++ b/lldb/source/Host/common/File.cpp @@ -772,7 +772,7 @@ mode_t File::ConvertOpenOptionsForPOSIXOpen(OpenOptions open_options) { llvm::Expected<SerialPort::Options> SerialPort::OptionsFromURL(llvm::StringRef urlqs) { SerialPort::Options serial_options; - for (llvm::StringRef x : llvm::Split(urlqs, '&')) { + for (llvm::StringRef x : llvm::split(urlqs, '&')) { if (x.consume_front("baud=")) { unsigned int baud_rate; if (!llvm::to_integer(x, baud_rate, 10)) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp index c68fcc1789d80..9550cb53e9e83 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp @@ -355,7 +355,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() { // configuration of the transport before attaching/launching the process. m_qSupported_response = response.GetStringRef().str(); - for (llvm::StringRef x : llvm::Split(response.GetStringRef(), ';')) { + for (llvm::StringRef x : llvm::split(response.GetStringRef(), ';')) { if (x == "qXfer:auxv:read+") m_supports_qXfer_auxv_read = eLazyBoolYes; else if (x == "qXfer:libraries-svr4:read+") @@ -1609,7 +1609,7 @@ Status GDBRemoteCommunicationClient::GetMemoryRegionInfo( error.SetErrorString(error_string.c_str()); } else if (name.equals("dirty-pages")) { std::vector<addr_t> dirty_page_list; - for (llvm::StringRef x : llvm::Split(value, ',')) { + for (llvm::StringRef x : llvm::split(value, ',')) { addr_t page; x.consume_front("0x"); if (llvm::to_integer(x, page, 16)) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp index 8a86734b3da9e..2080e09da3c2a 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp @@ -3637,7 +3637,7 @@ GDBRemoteCommunicationServerLLGS::Handle_qSaveCore( StringRef packet_str{packet.GetStringRef()}; assert(packet_str.startswith("qSaveCore")); if (packet_str.consume_front("qSaveCore;")) { - for (auto x : llvm::Split(packet_str, ';')) { + for (auto x : llvm::split(packet_str, ';')) { if (x.consume_front("path-hint:")) StringExtractor(x).GetHexByteString(path_hint); else diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index 3c2476c6b730e..92e1e57f84260 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -375,7 +375,7 @@ static size_t SplitCommaSeparatedRegisterNumberString( const llvm::StringRef &comma_separated_register_numbers, std::vector<uint32_t> ®nums, int base) { regnums.clear(); - for (llvm::StringRef x : llvm::Split(comma_separated_register_numbers, ',')) { + for (llvm::StringRef x : llvm::split(comma_separated_register_numbers, ',')) { uint32_t reg; if (llvm::to_integer(x, reg, base)) regnums.push_back(reg); @@ -1405,7 +1405,7 @@ size_t ProcessGDBRemote::UpdateThreadIDsFromStopReplyThreadsValue( size_t ProcessGDBRemote::UpdateThreadPCsFromStopReplyThreadsValue( llvm::StringRef value) { m_thread_pcs.clear(); - for (llvm::StringRef x : llvm::Split(value, ',')) { + for (llvm::StringRef x : llvm::split(value, ',')) { lldb::addr_t pc; if (llvm::to_integer(x, pc, 16)) m_thread_pcs.push_back(pc); @@ -4973,7 +4973,7 @@ llvm::Expected<bool> ProcessGDBRemote::SaveCore(llvm::StringRef outfile) { std::string path; // process the response - for (auto x : llvm::Split(response.GetStringRef(), ';')) { + for (auto x : llvm::split(response.GetStringRef(), ';')) { if (x.consume_front("core-path:")) StringExtractor(x).GetHexByteString(path); } diff --git a/llvm/include/llvm/ADT/StringExtras.h b/llvm/include/llvm/ADT/StringExtras.h index 41dc7e291916d..0c2868040a44a 100644 --- a/llvm/include/llvm/ADT/StringExtras.h +++ b/llvm/include/llvm/ADT/StringExtras.h @@ -505,6 +505,7 @@ class ListSeparator { class SplittingIterator : public iterator_facade_base<SplittingIterator, std::forward_iterator_tag, StringRef> { + char SeparatorStorage; StringRef Current; StringRef Next; StringRef Separator; @@ -515,8 +516,35 @@ class SplittingIterator ++*this; } + SplittingIterator(StringRef Str, char Separator) + : SeparatorStorage(Separator), Next(Str), + Separator(&SeparatorStorage, 1) { + ++*this; + } + + SplittingIterator(const SplittingIterator &R) + : SeparatorStorage(R.SeparatorStorage), Current(R.Current), Next(R.Next), + Separator(R.Separator) { + if (R.Separator.data() == &R.SeparatorStorage) + Separator = StringRef(&SeparatorStorage, 1); + } + + SplittingIterator &operator=(const SplittingIterator &R) { + if (this == &R) + return *this; + + SeparatorStorage = R.SeparatorStorage; + Current = R.Current; + Next = R.Next; + Separator = R.Separator; + if (R.Separator.data() == &R.SeparatorStorage) + Separator = StringRef(&SeparatorStorage, 1); + return *this; + } + bool operator==(const SplittingIterator &R) const { - return Current == R.Current && Next == R.Next && Separator == R.Separator; + assert(Separator == R.Separator); + return Current.data() == R.Current.data(); } const StringRef &operator*() const { return Current; } @@ -524,9 +552,7 @@ class SplittingIterator StringRef &operator*() { return Current; } SplittingIterator &operator++() { - std::pair<StringRef, StringRef> Res = Next.split(Separator); - Current = Res.first; - Next = Res.second; + std::tie(Current, Next) = Next.split(Separator); return *this; } }; @@ -536,26 +562,21 @@ class SplittingIterator /// over separated strings like so: /// /// \code -/// for (StringRef x : llvm::Split("foo,bar,baz", ',')) +/// for (StringRef x : llvm::split("foo,bar,baz", ",")) /// ...; /// \end /// /// Note that the passed string must remain valid throuhgout lifetime /// of the iterators. -class Split { - StringRef Str; - std::string SeparatorStr; - -public: - Split(StringRef NewStr, StringRef Separator) - : Str(NewStr), SeparatorStr(Separator) {} - Split(StringRef NewStr, char Separator) - : Str(NewStr), SeparatorStr(1, Separator) {} - - SplittingIterator begin() { return SplittingIterator(Str, SeparatorStr); } +inline iterator_range<SplittingIterator> split(StringRef Str, StringRef Separator) { + return {SplittingIterator(Str, Separator), + SplittingIterator(StringRef(), Separator)}; +} - SplittingIterator end() { return SplittingIterator("", SeparatorStr); } -}; +inline iterator_range<SplittingIterator> split(StringRef Str, char Separator) { + return {SplittingIterator(Str, Separator), + SplittingIterator(StringRef(), Separator)}; +} } // end namespace llvm diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp index e50423d07a714..5edff7a741362 100644 --- a/llvm/lib/IR/DataLayout.cpp +++ b/llvm/lib/IR/DataLayout.cpp @@ -260,12 +260,12 @@ Error DataLayout::parseSpecifier(StringRef Desc) { while (!Desc.empty()) { // Split at '-'. std::pair<StringRef, StringRef> Split; - if (Error Err = split(Desc, '-', Split)) + if (Error Err = ::split(Desc, '-', Split)) return Err; Desc = Split.second; // Split at ':'. - if (Error Err = split(Split.first, ':', Split)) + if (Error Err = ::split(Split.first, ':', Split)) return Err; // Aliases used below. @@ -274,7 +274,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) { if (Tok == "ni") { do { - if (Error Err = split(Rest, ':', Split)) + if (Error Err = ::split(Rest, ':', Split)) return Err; Rest = Split.second; unsigned AS; @@ -315,7 +315,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) { if (Rest.empty()) return reportError( "Missing size specification for pointer in datalayout string"); - if (Error Err = split(Rest, ':', Split)) + if (Error Err = ::split(Rest, ':', Split)) return Err; unsigned PointerMemSize; if (Error Err = getIntInBytes(Tok, PointerMemSize)) @@ -327,7 +327,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) { if (Rest.empty()) return reportError( "Missing alignment specification for pointer in datalayout string"); - if (Error Err = split(Rest, ':', Split)) + if (Error Err = ::split(Rest, ':', Split)) return Err; unsigned PointerABIAlign; if (Error Err = getIntInBytes(Tok, PointerABIAlign)) @@ -342,7 +342,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) { // Preferred alignment. unsigned PointerPrefAlign = PointerABIAlign; if (!Rest.empty()) { - if (Error Err = split(Rest, ':', Split)) + if (Error Err = ::split(Rest, ':', Split)) return Err; if (Error Err = getIntInBytes(Tok, PointerPrefAlign)) return Err; @@ -352,7 +352,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) { // Now read the index. It is the second optional parameter here. if (!Rest.empty()) { - if (Error Err = split(Rest, ':', Split)) + if (Error Err = ::split(Rest, ':', Split)) return Err; if (Error Err = getIntInBytes(Tok, IndexSize)) return Err; @@ -393,7 +393,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) { if (Rest.empty()) return reportError( "Missing alignment specification in datalayout string"); - if (Error Err = split(Rest, ':', Split)) + if (Error Err = ::split(Rest, ':', Split)) return Err; unsigned ABIAlign; if (Error Err = getIntInBytes(Tok, ABIAlign)) @@ -410,7 +410,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) { // Preferred alignment. unsigned PrefAlign = ABIAlign; if (!Rest.empty()) { - if (Error Err = split(Rest, ':', Split)) + if (Error Err = ::split(Rest, ':', Split)) return Err; if (Error Err = getIntInBytes(Tok, PrefAlign)) return Err; @@ -439,7 +439,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) { LegalIntWidths.push_back(Width); if (Rest.empty()) break; - if (Error Err = split(Rest, ':', Split)) + if (Error Err = ::split(Rest, ':', Split)) return Err; } break; diff --git a/llvm/unittests/ADT/StringExtrasTest.cpp b/llvm/unittests/ADT/StringExtrasTest.cpp index 35d3535ec2683..20437f9fbbb39 100644 --- a/llvm/unittests/ADT/StringExtrasTest.cpp +++ b/llvm/unittests/ADT/StringExtrasTest.cpp @@ -8,6 +8,7 @@ #include "llvm/ADT/StringExtras.h" #include "llvm/Support/raw_ostream.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" using namespace llvm; @@ -276,7 +277,7 @@ TEST(StringExtrasTest, toStringAPSInt) { } TEST(StringExtrasTest, splitStringRef) { - auto Spl = Split("foo<=>bar<=><=>baz", "<=>"); + auto Spl = split("foo<=>bar<=><=>baz", "<=>"); auto It = Spl.begin(); auto End = Spl.end(); @@ -291,8 +292,15 @@ TEST(StringExtrasTest, splitStringRef) { ASSERT_EQ(++It, End); } -TEST(StringExtrasTest, splItChar) { - auto Spl = Split("foo,bar,,baz", ','); +TEST(StringExtrasTest, splitStringRefForLoop) { + llvm::SmallVector<StringRef, 4> Result; + for (StringRef x : split("foo<=>bar<=><=>baz", "<=>")) + Result.push_back(x); + EXPECT_THAT(Result, testing::ElementsAre("foo", "bar", "", "baz")); +} + +TEST(StringExtrasTest, splitChar) { + auto Spl = split("foo,bar,,baz", ','); auto It = Spl.begin(); auto End = Spl.end(); @@ -306,3 +314,10 @@ TEST(StringExtrasTest, splItChar) { EXPECT_EQ(*It, StringRef("baz")); ASSERT_EQ(++It, End); } + +TEST(StringExtrasTest, splitCharForLoop) { + llvm::SmallVector<StringRef, 4> Result; + for (StringRef x : split("foo,bar,,baz", ',')) + Result.push_back(x); + EXPECT_THAT(Result, testing::ElementsAre("foo", "bar", "", "baz")); +} _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
