From 8744cf95ef999a814c879eab835ce8e915b81cb9 Mon Sep 17 00:00:00 2001
From: Martchus <martchus@gmx.net>
Date: Tue, 18 Feb 2020 19:29:23 +0100
Subject: [PATCH] Ensure no copy is made when using argsToString()

It seems that std::make_tuple() is using __decay_and_strip so
the arguments get copied. Using the std::tuple c'tor directly
instead.

When using the %-operator it is already taken care that strings
are stored as pointers and not by value.
---
 conversion/stringbuilder.h |  7 ++++++-
 tests/conversiontests.cpp  | 26 +++++++++++++++++++++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/conversion/stringbuilder.h b/conversion/stringbuilder.h
index 0237275..ff4c8c2 100644
--- a/conversion/stringbuilder.h
+++ b/conversion/stringbuilder.h
@@ -241,6 +241,11 @@ constexpr void append(StringType &target, TupleType &&tuple, typename StringType
     return TupleToString<StringType, TupleType, std::tuple_size_v<std::decay_t<TupleType>>>::append(std::forward<TupleType>(tuple), target);
 }
 
+template <typename... Elements> constexpr std::tuple<Elements &&...> makeTupleHoldingRefsToTemporaryObjects(Elements &&... args) noexcept
+{
+    return std::tuple<Elements &&...>(std::forward<Elements>(args)...);
+}
+
 } // namespace Helper
 /// \endcond
 
@@ -257,7 +262,7 @@ template <class StringType = std::string, class... Args> inline StringType tuple
 
 template <class StringType = std::string, class... Args> inline StringType argsToString(Args &&... args)
 {
-    return tupleToString(std::make_tuple(std::forward<Args>(args)...));
+    return tupleToString(Helper::makeTupleHoldingRefsToTemporaryObjects(std::forward<Args>(args)...));
 }
 
 /*!
diff --git a/tests/conversiontests.cpp b/tests/conversiontests.cpp
index a7d5a1c..3ec60c6 100644
--- a/tests/conversiontests.cpp
+++ b/tests/conversiontests.cpp
@@ -387,6 +387,23 @@ struct ConvertibleToString {
     operator std::string() const;
 };
 
+struct StringThatDoesNotLikeToBeCopiedOrMoved : public std::string {
+    explicit StringThatDoesNotLikeToBeCopiedOrMoved(const char *value)
+        : std::string(value)
+    {
+    }
+    [[noreturn]] StringThatDoesNotLikeToBeCopiedOrMoved(const StringThatDoesNotLikeToBeCopiedOrMoved &other)
+        : std::string(other)
+    {
+        CPPUNIT_FAIL("attempt to copy string: " + other);
+    }
+    [[noreturn]] StringThatDoesNotLikeToBeCopiedOrMoved(StringThatDoesNotLikeToBeCopiedOrMoved &&other)
+        : std::string(std::move(other))
+    {
+        CPPUNIT_FAIL("attempt to move string: " + other);
+    }
+};
+
 void ConversionTests::testStringBuilder()
 {
     // check whether type traits work as expected
@@ -398,7 +415,8 @@ void ConversionTests::testStringBuilder()
     static_assert(Helper::IsStringViewType<std::wstring, std::wstring_view>::value);
     static_assert(Helper::IsConvertibleToConstStringRef<std::string, ConvertibleToString>::value);
 #ifdef CPP_UTILITIES_USE_STANDARD_FILESYSTEM
-    static_assert(!Helper::IsConvertibleToConstStringRef<std::filesystem::path::string_type, std::filesystem::path>::value, "conversion via native() preferred");
+    static_assert(!Helper::IsConvertibleToConstStringRef<std::filesystem::path::string_type, std::filesystem::path>::value,
+        "conversion via native() preferred");
 #endif
     static_assert(
         !Helper::IsConvertibleToConstStringRef<std::string, std::string>::value, "yes, in this context this should not be considered convertible");
@@ -428,4 +446,10 @@ void ConversionTests::testStringBuilder()
     CPPUNIT_ASSERT_EQUAL_MESSAGE(
         "regular + operator still works (no problems with ambiguity)"s, "regular + still works"s, "regular"s + " + still works");
     CPPUNIT_ASSERT_EQUAL_MESSAGE("using string_view", "foobar123"s, "foo"sv % "bar"sv + 123);
+
+    // check that for the internal tuple construction no copies are made
+    StringThatDoesNotLikeToBeCopiedOrMoved str(" happen ");
+    const StringThatDoesNotLikeToBeCopiedOrMoved str2("for this");
+    CPPUNIT_ASSERT_EQUAL("no copy/move should happen for this!"s,
+        argsToString(StringThatDoesNotLikeToBeCopiedOrMoved("no copy/move should"), str, str2, StringThatDoesNotLikeToBeCopiedOrMoved("!")));
 }