From d6f2a7ab4b6cbb6648928c5da6cd0bf845025126 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 29 Nov 2022 17:32:41 -0500 Subject: cmStringCommand: remove use of cmCatViews() --- Source/cmStringCommand.cxx | 64 +++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/Source/cmStringCommand.cxx b/Source/cmStringCommand.cxx index c12d1fe..5a64588 100644 --- a/Source/cmStringCommand.cxx +++ b/Source/cmStringCommand.cxx @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -950,9 +949,9 @@ struct Args : cmRange::const_iterator> class json_error : public std::runtime_error { public: - json_error(std::initializer_list message, + json_error(const std::string& message, cm::optional errorPath = cm::nullopt) - : std::runtime_error(cmCatViews(message)) + : std::runtime_error(message) , ErrorPath{ std::move(errorPath) // NOLINT(performance-move-const-arg) } @@ -964,7 +963,7 @@ public: const std::string& Args::PopFront(cm::string_view error) { if (this->empty()) { - throw json_error({ error }); + throw json_error(std::string(error)); } const std::string& res = *this->begin(); this->advance(1); @@ -974,7 +973,7 @@ const std::string& Args::PopFront(cm::string_view error) const std::string& Args::PopBack(cm::string_view error) { if (this->empty()) { - throw json_error({ error }); + throw json_error(std::string(error)); } const std::string& res = *(this->end() - 1); this->retreat(1); @@ -999,7 +998,7 @@ cm::string_view JsonTypeToString(Json::ValueType type) case Json::ValueType::objectValue: return "OBJECT"_s; } - throw json_error({ "invalid JSON type found"_s }); + throw json_error("invalid JSON type found"); } int ParseIndex( @@ -1008,14 +1007,14 @@ int ParseIndex( { unsigned long lindex; if (!cmStrToULong(str, &lindex)) { - throw json_error({ "expected an array index, got: '"_s, str, "'"_s }, + throw json_error(cmStrCat("expected an array index, got: '"_s, str, "'"_s), progress); } Json::ArrayIndex index = static_cast(lindex); if (index >= max) { cmAlphaNum sizeStr{ max }; - throw json_error({ "expected an index less than "_s, sizeStr.View(), - " got '"_s, str, "'"_s }, + throw json_error(cmStrCat("expected an index less than "_s, sizeStr.View(), + " got '"_s, str, "'"_s), progress); } return index; @@ -1036,16 +1035,16 @@ Json::Value& ResolvePath(Json::Value& json, Args path) } else if (search->isObject()) { if (!search->isMember(field)) { const auto progressStr = cmJoin(progress, " "_s); - throw json_error({ "member '"_s, progressStr, "' not found"_s }, + throw json_error(cmStrCat("member '"_s, progressStr, "' not found"_s), progress); } search = &(*search)[field]; } else { const auto progressStr = cmJoin(progress, " "_s); throw json_error( - { "invalid path '"_s, progressStr, - "', need element of OBJECT or ARRAY type to lookup '"_s, field, - "' got "_s, JsonTypeToString(search->type()) }, + cmStrCat("invalid path '"_s, progressStr, + "', need element of OBJECT or ARRAY type to lookup '"_s, + field, "' got "_s, JsonTypeToString(search->type())), progress); } } @@ -1061,7 +1060,7 @@ Json::Value ReadJson(const std::string& jsonstr) std::string error; if (!jsonReader->parse(jsonstr.data(), jsonstr.data() + jsonstr.size(), &json, &error)) { - throw json_error({ "failed parsing json string: "_s, error }); + throw json_error(cmStrCat("failed parsing json string: "_s, error)); } return json; } @@ -1101,9 +1100,9 @@ bool HandleJSONCommand(std::vector const& arguments, mode != "LENGTH"_s && mode != "REMOVE"_s && mode != "SET"_s && mode != "EQUAL"_s) { throw json_error( - { "got an invalid mode '"_s, mode, - "', expected one of GET, TYPE, MEMBER, LENGTH, REMOVE, SET, " - " EQUAL"_s }); + cmStrCat("got an invalid mode '"_s, mode, + "', expected one of GET, TYPE, MEMBER, LENGTH, REMOVE, SET, " + " EQUAL"_s)); } const auto& jsonstr = args.PopFront("missing json string argument"_s); @@ -1127,10 +1126,11 @@ bool HandleJSONCommand(std::vector const& arguments, const auto& indexStr = args.PopBack("missing member index"_s); const auto& value = ResolvePath(json, args); if (!value.isObject()) { - throw json_error({ "MEMBER needs to be called with an element of " - "type OBJECT, got "_s, - JsonTypeToString(value.type()) }, - args); + throw json_error( + cmStrCat("MEMBER needs to be called with an element of " + "type OBJECT, got "_s, + JsonTypeToString(value.type())), + args); } const auto index = ParseIndex( indexStr, Args{ args.begin(), args.end() + 1 }, value.size()); @@ -1140,9 +1140,9 @@ bool HandleJSONCommand(std::vector const& arguments, } else if (mode == "LENGTH"_s) { const auto& value = ResolvePath(json, args); if (!value.isArray() && !value.isObject()) { - throw json_error({ "LENGTH needs to be called with an " - "element of type ARRAY or OBJECT, got "_s, - JsonTypeToString(value.type()) }, + throw json_error(cmStrCat("LENGTH needs to be called with an " + "element of type ARRAY or OBJECT, got "_s, + JsonTypeToString(value.type())), args); } @@ -1165,9 +1165,9 @@ bool HandleJSONCommand(std::vector const& arguments, value.removeMember(toRemove, &removed); } else { - throw json_error({ "REMOVE needs to be called with an " - "element of type ARRAY or OBJECT, got "_s, - JsonTypeToString(value.type()) }, + throw json_error(cmStrCat("REMOVE needs to be called with an " + "element of type ARRAY or OBJECT, got "_s, + JsonTypeToString(value.type())), args); } makefile.AddDefinition(*outputVariable, WriteJson(json)); @@ -1189,9 +1189,9 @@ bool HandleJSONCommand(std::vector const& arguments, value.append(newValue); } } else { - throw json_error({ "SET needs to be called with an " - "element of type OBJECT or ARRAY, got "_s, - JsonTypeToString(value.type()) }); + throw json_error(cmStrCat("SET needs to be called with an " + "element of type OBJECT or ARRAY, got "_s, + JsonTypeToString(value.type()))); } makefile.AddDefinition(*outputVariable, WriteJson(json)); @@ -1207,7 +1207,7 @@ bool HandleJSONCommand(std::vector const& arguments, if (outputVariable && e.ErrorPath) { const auto errorPath = cmJoin(*e.ErrorPath, "-"); makefile.AddDefinition(*outputVariable, - cmCatViews({ errorPath, "-NOTFOUND"_s })); + cmStrCat(errorPath, "-NOTFOUND"_s)); } else if (outputVariable) { makefile.AddDefinition(*outputVariable, "NOTFOUND"_s); } @@ -1215,7 +1215,7 @@ bool HandleJSONCommand(std::vector const& arguments, if (errorVariable) { makefile.AddDefinition(*errorVariable, e.what()); } else { - status.SetError(cmCatViews({ "sub-command JSON "_s, e.what(), "."_s })); + status.SetError(cmStrCat("sub-command JSON "_s, e.what(), "."_s)); success = false; } } -- cgit v0.12 From beba50bd61d32ea68acffca67a48bd7e81e6e097 Mon Sep 17 00:00:00 2001 From: Kyle Edwards Date: Tue, 29 Nov 2022 16:59:24 -0500 Subject: cmStrCat(): optimize when first argument is an rvalue string --- Source/cmStringAlgorithms.cxx | 18 ++++++++++---- Source/cmStringAlgorithms.h | 43 +++++++++++++++++++++++++++------ Tests/CMakeLib/testStringAlgorithms.cxx | 14 +++++++++++ 3 files changed, 63 insertions(+), 12 deletions(-) diff --git a/Source/cmStringAlgorithms.cxx b/Source/cmStringAlgorithms.cxx index f73c854..e559cfa 100644 --- a/Source/cmStringAlgorithms.cxx +++ b/Source/cmStringAlgorithms.cxx @@ -203,15 +203,23 @@ cmAlphaNum::cmAlphaNum(double val) MakeDigits(this->View_, this->Digits_, "%g", val); } -std::string cmCatViews(std::initializer_list views) +std::string cmCatViews(cm::optional&& first, + std::initializer_list views) { - std::size_t total_size = 0; + std::size_t totalSize = 0; for (cm::string_view const& view : views) { - total_size += view.size(); + totalSize += view.size(); } - std::string result(total_size, '\0'); - std::string::iterator sit = result.begin(); + std::string result; + std::string::size_type initialLen = 0; + if (first) { + totalSize += first->length(); + initialLen = first->length(); + result = std::move(*first); + } + result.resize(totalSize); + std::string::iterator sit = result.begin() + initialLen; for (cm::string_view const& view : views) { sit = std::copy_n(view.data(), view.size(), sit); } diff --git a/Source/cmStringAlgorithms.h b/Source/cmStringAlgorithms.h index 83938bc..bff2eda 100644 --- a/Source/cmStringAlgorithms.h +++ b/Source/cmStringAlgorithms.h @@ -12,6 +12,7 @@ #include #include +#include #include #include "cmRange.h" @@ -146,7 +147,8 @@ std::vector cmExpandedLists(InputIt first, InputIt last) } /** Concatenate string pieces into a single string. */ -std::string cmCatViews(std::initializer_list views); +std::string cmCatViews(cm::optional&& first, + std::initializer_list views); /** Utility class for cmStrCat. */ class cmAlphaNum @@ -189,13 +191,38 @@ private: char Digits_[32]; }; +template +class cmStrCatHelper +{ +public: + static std::string Compute(cmAlphaNum const& a, cmAlphaNum const& b, + AV const&... args) + { + return cmCatViews( + cm::nullopt, + { a.View(), b.View(), static_cast(args).View()... }); + } +}; + +template +class cmStrCatHelper +{ +public: + static std::string Compute(std::string&& a, cmAlphaNum const& b, + AV const&... args) + { + return cmCatViews( + std::move(a), + { b.View(), static_cast(args).View()... }); + } +}; + /** Concatenate string pieces and numbers into a single string. */ -template -inline std::string cmStrCat(cmAlphaNum const& a, cmAlphaNum const& b, - AV const&... args) +template +inline std::string cmStrCat(A&& a, B&& b, AV&&... args) { - return cmCatViews( - { a.View(), b.View(), static_cast(args).View()... }); + return cmStrCatHelper::Compute( + std::forward(a), std::forward(b), std::forward(args)...); } /** Joins wrapped elements of a range with separator into a single string. */ @@ -207,7 +234,9 @@ std::string cmWrap(cm::string_view prefix, Range const& rng, return std::string(); } return cmCatViews( - { prefix, cmJoin(rng, cmCatViews({ suffix, sep, prefix })), suffix }); + cm::nullopt, + { prefix, cmJoin(rng, cmCatViews(cm::nullopt, { suffix, sep, prefix })), + suffix }); } /** Joins wrapped elements of a range with separator into a single string. */ diff --git a/Tests/CMakeLib/testStringAlgorithms.cxx b/Tests/CMakeLib/testStringAlgorithms.cxx index 1e6b611..f73e62a 100644 --- a/Tests/CMakeLib/testStringAlgorithms.cxx +++ b/Tests/CMakeLib/testStringAlgorithms.cxx @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include #include @@ -144,6 +146,18 @@ int testStringAlgorithms(int /*unused*/, char* /*unused*/ []) d -= val; assert_ok((d < div) && (d > -div), "cmStrCat double"); } + { + std::string val; + std::string expect; + val.reserve(120 * cmStrLen("cmStrCat move")); + auto data = val.data(); + for (int i = 0; i < 100; i++) { + val = cmStrCat(std::move(val), "cmStrCat move"); + expect += "cmStrCat move"; + } + assert_ok((val.data() == data), "cmStrCat move"); + assert_string(val, expect, "cmStrCat move"); + } // ---------------------------------------------------------------------- // Test cmWrap -- cgit v0.12