Skip to content

Commit 6708610

Browse files
[SYCL] Fix regression introduced in #11517 (#12002)
This patch fixes a regression caused by refactoring of `split_string` function in #11517. The regression is the following: the delimiter is a space, input string is `"A B C "` - which has a space at the end. Before #11517 `split_string` returned `{"A", "B", "C"}` but with #11517 it returns `{"A", "B", "C", ""}`. This regression affects multiple CTS tests, verified the fix locally, and also unit tests for `split_string` function were added to this patch. This log shows that one of the unit tests failed before applying the fix: https://github.com/intel/llvm/actions/runs/6974175043/job/18979466747#step:11:372. With the fix it passes. Fixes #11996
1 parent 88f1d0a commit 6708610

File tree

2 files changed

+41
-10
lines changed

2 files changed

+41
-10
lines changed

sycl/source/detail/common.cpp

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,18 +72,24 @@ const char *stringifyErrorCode(pi_int32 error) {
7272
}
7373

7474
std::vector<std::string> split_string(const std::string &str, char delimeter) {
75-
std::vector<std::string> result;
76-
size_t beg = 0;
77-
size_t end = 0;
78-
while ((end = str.find(delimeter, beg)) != std::string::npos) {
79-
result.push_back(str.substr(beg, end - beg));
80-
beg = end + 1;
75+
std::vector<std::string> Result;
76+
size_t Start = 0;
77+
size_t End = 0;
78+
while ((End = str.find(delimeter, Start)) != std::string::npos) {
79+
Result.push_back(str.substr(Start, End - Start));
80+
Start = End + 1;
8181
}
82-
end = str.find('\0');
83-
if (beg < end) {
84-
result.push_back(str.substr(beg, end - beg));
82+
// Get the last substring and ignore the null character so we wouldn't get
83+
// double null characters \0\0 at the end of the substring
84+
End = str.find('\0');
85+
if (Start < End) {
86+
std::string LastSubStr(str.substr(Start, End - Start));
87+
// In case str has a delimeter at the end, the substring will be empty, so
88+
// we shouldn't add it to the final vector
89+
if (!LastSubStr.empty())
90+
Result.push_back(LastSubStr);
8591
}
86-
return result;
92+
return Result;
8793
}
8894

8995
} // namespace detail

sycl/unittests/kernel-and-program/DeviceInfo.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,3 +235,28 @@ TEST_F(DeviceInfoNegativeTest, TestAspectNotSupported) {
235235
EXPECT_EQ(Dev.has(aspect::ext_intel_memory_clock_rate), false);
236236
EXPECT_EQ(Dev.has(aspect::ext_intel_memory_bus_width), false);
237237
}
238+
239+
TEST_F(DeviceInfoTest, SplitStringDelimeterSpace) {
240+
std::string InputString("V1 V2 V3");
241+
std::vector<std::string> Expected{"V1", "V2", "V3"};
242+
EXPECT_EQ(detail::split_string(InputString, ' '), Expected);
243+
}
244+
245+
TEST_F(DeviceInfoTest, SplitStringDelimeterSpaceAtTheEnd) {
246+
std::string InputString("V1 V2 V3 ");
247+
std::vector<std::string> Expected{"V1", "V2", "V3"};
248+
EXPECT_EQ(detail::split_string(InputString, ' '), Expected);
249+
}
250+
251+
TEST_F(DeviceInfoTest, SplitStringDelimeterSemicolon) {
252+
std::string InputString("V1;V2;V3");
253+
std::vector<std::string> Expected{"V1", "V2", "V3"};
254+
EXPECT_EQ(detail::split_string(InputString, ';'), Expected);
255+
}
256+
257+
TEST_F(DeviceInfoTest, SplitStringCheckNoDoubleNullCharacters) {
258+
std::string InputString("V1;V23");
259+
std::vector<std::string> Result = detail::split_string(InputString, ';');
260+
EXPECT_EQ(Result[0].length(), (unsigned)2);
261+
EXPECT_EQ(Result[1].length(), (unsigned)3);
262+
}

0 commit comments

Comments
 (0)