From 60adc098da2d3026280bd0caa10915d6c8e6e14f Mon Sep 17 00:00:00 2001 From: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> Date: Fri, 18 Feb 2022 17:16:30 -0600 Subject: [PATCH] Fixed implicit conversion to bool in AZStd::to_string. (#7755) This specifically implies to the value returning to_string overloads Added a range based StringFunc::Join overload. Reduced the number of AZStd::to_string calls in the TranslationKey operator<< function. Signed-off-by: lumberyard-employee-dm <56135373+lumberyard-employee-dm@users.noreply.github.com> --- .../AzCore/AzCore/StringFunc/StringFunc.h | 29 +++++++--- .../AzCore/AzCore/std/string/conversions.h | 8 ++- Code/Framework/AzCore/Tests/AZStd/String.cpp | 53 ++++++++++++++----- Code/Framework/AzCore/Tests/StringFunc.cpp | 13 +++++ .../Code/Source/Translation/TranslationBus.h | 14 ++--- 5 files changed, 90 insertions(+), 27 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/StringFunc/StringFunc.h b/Code/Framework/AzCore/AzCore/StringFunc/StringFunc.h index 55236a0fff..d7f0a86853 100644 --- a/Code/Framework/AzCore/AzCore/StringFunc/StringFunc.h +++ b/Code/Framework/AzCore/AzCore/StringFunc/StringFunc.h @@ -416,27 +416,27 @@ namespace AZ Join(fixedOutput, example.begin(), example.end(), ","); // fixedOutput == "test,string,joining" */ - template + template inline void Join( - TStringType& joinTarget, - const TConvertableToStringViewIterator& iteratorBegin, - const TConvertableToStringViewIterator& iteratorEnd, - const TSeparatorString& separator) + StringType& joinTarget, + const ConvertableToStringViewIterator& iteratorBegin, + const ConvertableToStringViewIterator& iteratorEnd, + const SeparatorString& separator) { if (iteratorBegin == iteratorEnd) { return; } - using CharType = typename TStringType::value_type; - using CharTraitsType = typename TStringType::traits_type; + using CharType = typename StringType::value_type; + using CharTraitsType = typename StringType::traits_type; size_t size = joinTarget.size() + AZStd::basic_string_view(*iteratorBegin).size(); for (auto currentIterator = AZStd::next(iteratorBegin); currentIterator != iteratorEnd; ++currentIterator) { size += AZStd::basic_string_view(*currentIterator).size(); // Special case for when the separator is just the character type - if constexpr (AZStd::is_same_v, CharType>) + if constexpr (AZStd::is_same_v, CharType>) { size += 1; } @@ -455,6 +455,19 @@ namespace AZ } } + template && + AZStd::convertible_to, + AZStd::basic_string_view> + >> + void Join(StringType& joinTarget, Range&& stringViewConvertibleRange, const SeparatorString& separator) + { + Join(joinTarget, + AZStd::ranges::begin(stringViewConvertibleRange), + AZStd::ranges::end(stringViewConvertibleRange), + separator); + } + ////////////////////////////////////////////////////////////////////////// //! StringFunc::NumberFormatting Namespace /*! For string functions supporting string representations of numbers diff --git a/Code/Framework/AzCore/AzCore/std/string/conversions.h b/Code/Framework/AzCore/AzCore/std/string/conversions.h index 9427eaccf6..d99d05075e 100644 --- a/Code/Framework/AzCore/AzCore/std/string/conversions.h +++ b/Code/Framework/AzCore/AzCore/std/string/conversions.h @@ -302,7 +302,13 @@ namespace AZStd inline AZStd::string to_string(long long val) { AZStd::string str; to_string(str, val); return str; } inline AZStd::string to_string(unsigned long long val) { AZStd::string str; to_string(str, val); return str; } inline AZStd::string to_string(long double val) { AZStd::string str; to_string(str, val); return str; } - inline AZStd::string to_string(bool val) { AZStd::string str; to_string(str, val); return str; } + template + auto to_string(BoolType value) -> enable_if_t, bool>, AZStd::string> + { + AZStd::string str; + to_string(str, value); + return str; + } // In our engine we assume AZStd::string is Utf8 encoded! template diff --git a/Code/Framework/AzCore/Tests/AZStd/String.cpp b/Code/Framework/AzCore/Tests/AZStd/String.cpp index 4ecfdcab22..5725cf80bc 100644 --- a/Code/Framework/AzCore/Tests/AZStd/String.cpp +++ b/Code/Framework/AzCore/Tests/AZStd/String.cpp @@ -656,18 +656,6 @@ namespace UnitTest fval = AZStd::stof(wfloatStr); AZ_TEST_ASSERT_FLOAT_CLOSE(fval, 2.32f); - AZStd::to_string(intStr, 20); - AZ_TEST_ASSERT(intStr == "20"); - - EXPECT_EQ("20", AZStd::to_string(static_cast(20))); - EXPECT_EQ("20", AZStd::to_string(static_cast(20))); - EXPECT_EQ("20", AZStd::to_string(static_cast(20))); - EXPECT_EQ("20", AZStd::to_string(static_cast(20))); - EXPECT_EQ("20", AZStd::to_string(static_cast(20))); - EXPECT_EQ("20", AZStd::to_string(static_cast(20))); - EXPECT_EQ("false", AZStd::to_string(false)); - EXPECT_EQ("true", AZStd::to_string(true)); - // wstring to string AZStd::string str1; AZStd::to_string(str1, wstr); @@ -978,6 +966,47 @@ namespace UnitTest AZ_TEST_ASSERT(*vecIt++ == "Xiph Xlater 10000"); } + // Concept to model if AZStd::to_string() is a valid expression + template + constexpr bool IsToStringInvocable = false; + + template + constexpr bool IsToStringInvocable()))>> = true; + + TEST_F(String, String_to_stringOverload_DoesNotImplicitlyConvertToBool) + { + AZStd::string intStr; + AZStd::to_string(intStr, 20); + EXPECT_EQ("20", intStr); + + EXPECT_EQ("20", AZStd::to_string(static_cast(20))); + EXPECT_EQ("20", AZStd::to_string(static_cast(20))); + EXPECT_EQ("20", AZStd::to_string(static_cast(20))); + EXPECT_EQ("20", AZStd::to_string(static_cast(20))); + EXPECT_EQ("20", AZStd::to_string(static_cast(20))); + EXPECT_EQ("20", AZStd::to_string(static_cast(20))); + EXPECT_EQ("false", AZStd::to_string(false)); + EXPECT_EQ("true", AZStd::to_string(true)); + + // AZStd::to_string should not be invocable with a char or wchar_t literal + static_assert(!IsToStringInvocable); + static_assert(!IsToStringInvocable); + + // AZStd::to_string should + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + static_assert(IsToStringInvocable); + } + class Regex : public AllocatorsFixture { diff --git a/Code/Framework/AzCore/Tests/StringFunc.cpp b/Code/Framework/AzCore/Tests/StringFunc.cpp index dc4bc40e5b..7e1efd02b9 100644 --- a/Code/Framework/AzCore/Tests/StringFunc.cpp +++ b/Code/Framework/AzCore/Tests/StringFunc.cpp @@ -196,6 +196,19 @@ namespace AZ ASSERT_EQ(joinResult, expectedResult); } + TEST_F(StringFuncTest, Join_NonPathJoin_CanJoinRange) + { + AZStd::string result; + AZ::StringFunc::Join(result, AZStd::initializer_list{ "1", "2", "3", "4", "3" }, '/'); + EXPECT_EQ("1/2/3/4/3", result); + + result.clear(); + + // Try joining with a string literal instead of a char literal + AZ::StringFunc::Join(result, AZStd::initializer_list{ "1", "2", "3", "4", "3" }, "/"); + EXPECT_EQ("1/2/3/4/3", result); + } + TEST_F(StringFuncTest, Tokenize_SingleDelimeter_Empty) { AZStd::string input = ""; diff --git a/Gems/GraphCanvas/Code/Source/Translation/TranslationBus.h b/Gems/GraphCanvas/Code/Source/Translation/TranslationBus.h index 95c37b4daa..42eef90ba8 100644 --- a/Gems/GraphCanvas/Code/Source/Translation/TranslationBus.h +++ b/Gems/GraphCanvas/Code/Source/Translation/TranslationBus.h @@ -47,22 +47,24 @@ namespace GraphCanvas } template - auto operator << (T value) -> AZStd::enable_if_t, void>, TranslationKey&> + auto operator<< (T&& value) -> + AZStd::enable_if_t>, TranslationKey&> { - if (!m_key.empty() && !AZStd::to_string(value).empty()) + AZStd::string valueString = AZStd::to_string(AZStd::forward(value)); + if (!m_key.empty() && !valueString.empty()) { m_key.append("."); } - if (!AZStd::to_string(value).empty()) + if (!valueString.empty()) { - m_key.append(AZStd::to_string(value)); + m_key.append(valueString); } return *this; } - TranslationKey& operator << (const AZStd::string& value) + TranslationKey& operator<< (const AZStd::string& value) { if (!m_key.empty() && !value.empty()) { @@ -70,7 +72,7 @@ namespace GraphCanvas } if (!value.empty()) { - m_key.append(value.c_str()); + m_key.append(value); } return *this;