[llvm] 66e06cc - [llvm] [ADT] Update llvm::Split() per Pavel Labath's suggestions
Michał Górny via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 22 03:28:01 PDT 2021
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"));
+}
More information about the llvm-commits
mailing list