diff --git a/ci/iwyu/mappings.imp b/ci/iwyu/mappings.imp index 97872205b..627e20127 100644 --- a/ci/iwyu/mappings.imp +++ b/ci/iwyu/mappings.imp @@ -110,7 +110,9 @@ { "symbol": ["nlohmann::json", "private", "", "public"] }, # pybind11 -{ "include": [ "", private, "", "public" ] }, +{ "include": [ "@", private, "", "public" ] }, +{ "include": [ "@\"pybind11/detail/.*.h\"", private, "\"pybind11/pybind11.h\"", "public" ] }, + { "symbol": ["pybind11", "private", "", "public"] }, { "symbol": ["pybind11", "private", "", "public"] }, diff --git a/cpp/mrc/CMakeLists.txt b/cpp/mrc/CMakeLists.txt index 5e570182c..fbebef5cf 100644 --- a/cpp/mrc/CMakeLists.txt +++ b/cpp/mrc/CMakeLists.txt @@ -154,6 +154,7 @@ add_library(libmrc src/public/runnable/types.cpp src/public/runtime/remote_descriptor.cpp src/public/utils/bytes_to_string.cpp + src/public/utils/string_utils.cpp src/public/utils/thread_utils.cpp src/public/utils/type_utils.cpp ) @@ -196,7 +197,7 @@ target_compile_definitions(libmrc $<$:MRC_ENABLE_BENCHMARKING> ) -if (MRC_ENABLE_CODECOV) +if(MRC_ENABLE_CODECOV) target_compile_definitions(libmrc INTERFACE "MRC_CODECOV_ENABLED") endif() @@ -206,7 +207,6 @@ set_target_properties(libmrc PROPERTIES OUTPUT_NAME ${PROJECT_NAME}) # ################################################################################################## # - install targets -------------------------------------------------------------------------------- - rapids_cmake_install_lib_dir(lib_dir) include(CPack) include(GNUInstallDirs) @@ -226,7 +226,6 @@ install( # ################################################################################################## # - subdirectories --------------------------------------------------------------------------------- - if(MRC_BUILD_TESTS) add_subdirectory(tests) @@ -239,7 +238,6 @@ endif() # ################################################################################################## # - install export --------------------------------------------------------------------------------- - set(doc_string [=[ Provide targets for mrc. diff --git a/cpp/mrc/include/mrc/utils/string_utils.hpp b/cpp/mrc/include/mrc/utils/string_utils.hpp index a835189f3..024ff8096 100644 --- a/cpp/mrc/include/mrc/utils/string_utils.hpp +++ b/cpp/mrc/include/mrc/utils/string_utils.hpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: Copyright (c) 2021-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-FileCopyrightText: Copyright (c) 2021-2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved. * SPDX-License-Identifier: Apache-2.0 * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -17,8 +17,23 @@ #pragma once -#include +// for ostringstream +#include // IWYU pragma: keep #include +#include // Concats multiple strings together using ostringstream. Use with MRC_CONCAT_STR("Start [" << my_int << "]") #define MRC_CONCAT_STR(strs) ((std::ostringstream&)(std::ostringstream() << strs)).str() + +namespace mrc { + +/** + * @brief Splits a string into an vector of strings based on a delimiter. + * + * @param str The string to split. + * @param delimiter The delimiter to split the string on. + * @return std::vector vector array of strings. + */ +std::vector split_string_to_vector(const std::string& str, const std::string& delimiter); + +} // namespace mrc diff --git a/cpp/mrc/src/internal/utils/parse_config.cpp b/cpp/mrc/src/internal/utils/parse_config.cpp index 7d49ce615..780739eb3 100644 --- a/cpp/mrc/src/internal/utils/parse_config.cpp +++ b/cpp/mrc/src/internal/utils/parse_config.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: Copyright (c) 2021-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-FileCopyrightText: Copyright (c) 2021-2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved. * SPDX-License-Identifier: Apache-2.0 * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -19,29 +19,15 @@ #include "./parse_ints.hpp" +#include "mrc/utils/string_utils.hpp" // for split_string_to_vector + #include #include // for uint32_t #include // for atoi -#include #include #include // for move -namespace { - -std::vector split_string_on(std::string str, char delim) -{ - std::vector tokens; - std::istringstream f(str); - std::string s; - while (std::getline(f, s, delim)) - { - tokens.push_back(s); - } - return tokens; -} -} // namespace - namespace mrc { ConfigurationMap parse_config(std::string config_str) @@ -50,9 +36,9 @@ ConfigurationMap parse_config(std::string config_str) bool left_wildcard = false; - for (const auto& entry : split_string_on(config_str, ';')) + for (const auto& entry : split_string_to_vector(config_str, ";")) { - auto tokens = split_string_on(entry, ':'); + auto tokens = split_string_to_vector(entry, ":"); int concurrency = 1; std::vector s; @@ -76,7 +62,7 @@ ConfigurationMap parse_config(std::string config_str) concurrency = std::atoi(tokens[1].c_str()); case 1: // parse segments - s = split_string_on(tokens[0], ','); + s = split_string_to_vector(tokens[0], ","); segments.insert(s.begin(), s.end()); break; @@ -86,7 +72,7 @@ ConfigurationMap parse_config(std::string config_str) "::;[repeated]"); } - config.push_back(std::make_tuple(std::move(segments), concurrency, std::move(groups))); + config.emplace_back(std::move(segments), concurrency, std::move(groups)); } return config; diff --git a/cpp/mrc/src/internal/utils/parse_ints.cpp b/cpp/mrc/src/internal/utils/parse_ints.cpp index 60c716982..c999339e2 100644 --- a/cpp/mrc/src/internal/utils/parse_ints.cpp +++ b/cpp/mrc/src/internal/utils/parse_ints.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: Copyright (c) 2021-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-FileCopyrightText: Copyright (c) 2021-2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved. * SPDX-License-Identifier: Apache-2.0 * * Licensed under the Apache License, Version 2.0 (the "License"); @@ -17,6 +17,8 @@ #include "./parse_ints.hpp" +#include "mrc/utils/string_utils.hpp" // for split_string_to_vector + #include #include @@ -31,17 +33,6 @@ int convert_string2_int(const std::string& str) return x; } -std::vector split_string_to_array(const std::string& str, char splitter) -{ - std::vector tokens; - std::stringstream ss(str); - std::string temp; - while (getline(ss, temp, splitter)) // split into new "lines" based on character - { - tokens.push_back(temp); - } - return tokens; -} } // namespace namespace mrc { @@ -49,10 +40,10 @@ namespace mrc { std::vector parse_ints(const std::string& data) { std::vector result; - std::vector tokens = split_string_to_array(data, ','); + std::vector tokens = split_string_to_vector(data, ","); for (auto& token : tokens) { - std::vector range = split_string_to_array(token, '-'); + std::vector range = split_string_to_vector(token, "-"); if (range.size() == 1) { result.push_back(convert_string2_int(range[0])); diff --git a/cpp/mrc/src/public/utils/string_utils.cpp b/cpp/mrc/src/public/utils/string_utils.cpp new file mode 100644 index 000000000..5ed572f4c --- /dev/null +++ b/cpp/mrc/src/public/utils/string_utils.cpp @@ -0,0 +1,36 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2021-2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "mrc/utils/string_utils.hpp" + +#include // for split +// We already have included we don't need these others, it is also the only public header +// with a definition for boost::is_any_of, so even if we replaced string.hpp with these others we would still need to +// include string.hpp or a detail/ header +// IWYU pragma: no_include +// IWYU pragma: no_include +// IWYU pragma: no_include + +namespace mrc { +std::vector split_string_to_vector(const std::string& str, const std::string& delimiter) +{ + std::vector results; + boost::split(results, str, boost::is_any_of(delimiter)); + return results; +} + +} // namespace mrc diff --git a/cpp/mrc/tests/CMakeLists.txt b/cpp/mrc/tests/CMakeLists.txt index 0e7eef64e..2d524caac 100644 --- a/cpp/mrc/tests/CMakeLists.txt +++ b/cpp/mrc/tests/CMakeLists.txt @@ -38,6 +38,7 @@ add_executable(test_mrc test_node.cpp test_pipeline.cpp test_segment.cpp + test_string_utils.cpp test_thread.cpp test_type_utils.cpp ) diff --git a/cpp/mrc/tests/test_string_utils.cpp b/cpp/mrc/tests/test_string_utils.cpp new file mode 100644 index 000000000..fbaf5b14e --- /dev/null +++ b/cpp/mrc/tests/test_string_utils.cpp @@ -0,0 +1,58 @@ +/* + * SPDX-FileCopyrightText: Copyright (c) 2024, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "./test_mrc.hpp" // IWYU pragma: associated + +#include "mrc/utils/string_utils.hpp" // for split_string_to_vector + +#include // for EXPECT_EQ + +#include +#include + +namespace mrc { + +TEST_CLASS(StringUtils); + +TEST_F(TestStringUtils, TestSplitStringToVector) +{ + struct TestValues + { + std::string str; + std::string delimiter; + std::vector expected_result; + }; + + std::vector values = { + {"Hello,World,!", ",", {"Hello", "World", "!"}}, + {"a/b/c", "/", {"a", "b", "c"}}, + {"/a/b/c", "/", {"", "a", "b", "c"}}, // leading delimeter + {"a/b/c/", "/", {"a", "b", "c", ""}}, // trailing delimeter + {"abcd", "/", {"abcd"}}, // no delimeter + {"", "/", {""}}, // empty string + {"/", "/", {"", ""}}, // single delimeter + {"//", "/", {"", "", ""}}, // duplicate delimeter + }; + + for (const auto& value : values) + { + auto result = mrc::split_string_to_vector(value.str, value.delimiter); + EXPECT_EQ(result, value.expected_result); + } +} + +} // namespace mrc diff --git a/python/mrc/_pymrc/include/pymrc/utilities/json_values.hpp b/python/mrc/_pymrc/include/pymrc/utilities/json_values.hpp index 25e852f2b..8c3db1aab 100644 --- a/python/mrc/_pymrc/include/pymrc/utilities/json_values.hpp +++ b/python/mrc/_pymrc/include/pymrc/utilities/json_values.hpp @@ -23,9 +23,11 @@ #include // for PYBIND11_EXPORT, pybind11::object, type_caster #include // for size_t -#include -// IWYU wants us to use the pybind11.h for the PYBIND11_EXPORT macro, but we already have it in pytypes.h -// IWYU pragma: no_include +#include // for map +#include // for string +#include // for move +// IWYU pragma: no_include +// IWYU pragma: no_include namespace mrc::pymrc { diff --git a/python/mrc/_pymrc/src/utilities/json_values.cpp b/python/mrc/_pymrc/src/utilities/json_values.cpp index 0a898e4d9..ebc8061f5 100644 --- a/python/mrc/_pymrc/src/utilities/json_values.cpp +++ b/python/mrc/_pymrc/src/utilities/json_values.cpp @@ -20,9 +20,8 @@ #include "pymrc/utilities/acquire_gil.hpp" #include "pymrc/utils.hpp" -#include "mrc/utils/string_utils.hpp" // for MRC_CONCAT_STR +#include "mrc/utils/string_utils.hpp" // for MRC_CONCAT_STR, split_string_to_array -#include // for split #include #include @@ -34,13 +33,6 @@ #include // for move #include // for vector -// We already have included we don't need these others, it is also the only public header -// with a definition for boost::is_any_of, so even if we replaced string.hpp with these others we would still need to -// include string.hpp or a detail/ header -// IWYU pragma: no_include -// IWYU pragma: no_include -// IWYU pragma: no_include - namespace py = pybind11; using namespace std::string_literals; @@ -48,9 +40,7 @@ namespace { std::vector split_path(const std::string& path) { - std::vector path_parts; - boost::split(path_parts, path, boost::is_any_of("/")); - return path_parts; + return mrc::split_string_to_vector(path, "/"s); } struct PyFoundObject