commit 36a34693232408f47742c41dde17928ca6412228
author Patrick Williams <patrick@stwcx.xyz> Sat Aug 28 16:46:00 2021 -0500
committer Patrick Williams <patrick@stwcx.xyz> Mon Aug 30 21:54:49 2021 +0000
tree 06dddb5d8a5d9d039c652be60ee9257a1a9a5204
parent 511a4752d23e00e6ad9f6b8d3f4be3964eeec220

lg2: conversion: reduce std::forward clutter

It was observed in another repository that if the same std::string
was passed twice into lg2 function that clang-tidy would complain
about a potential use-after-realloc due to the 'string.data()' call
for the first instance followed by a 'std::forward' on the second
instance.  I don't think there was actually any issue with that, but
we don't really leverage r-value references in this code because there
is never an opportunity to 'std::move' any contents.  Remove the
clutter of 'std::forward' everywhere and simply rely on l-value
references, which is all the code needs anyhow.  This also placated
clang-tidy's memory use checker.

Signed-off-by: Patrick Williams <patrick@stwcx.xyz>
Change-Id: I89582e2f868732e9a7a08a7a24f858b9130d56bc

lib/include/phosphor-logging/lg2/conversion.hpp

diff --git a/lib/include/phosphor-logging/lg2/conversion.hpp b/lib/include/phosphor-logging/lg2/conversion.hpp
index 1ab7f09..4517a11 100644
--- a/lib/include/phosphor-logging/lg2/conversion.hpp
+++ b/lib/include/phosphor-logging/lg2/conversion.hpp
@@ -168,7 +168,7 @@
 
 /** Logging conversion for string-likes. */
 template <log_flags... Fs, string_like_type V>
-static auto log_convert(const char* h, log_flag<Fs...> f, V&& v)
+static auto log_convert(const char* h, log_flag<Fs...> f, const V& v)
 {
     // Compile-time checks for valid formatting flags.
     prohibit(f, bin);
@@ -186,7 +186,7 @@
     //  - 'const char*' and similar use static cast.
     //  - 'std::filesystem::path' use c_str() function.
     //  - 'std::string' and 'std::string_view' use data() function.
-    auto str_data = [](V&& v) {
+    auto str_data = [](const V& v) {
         if constexpr (std::is_same_v<const char*, std::decay_t<V>> ||
                       std::is_same_v<char*, std::decay_t<V>>)
         {
@@ -204,7 +204,7 @@
     };
 
     // Add 'str' flag, force to 'const char*' for variadic passing.
-    return std::make_tuple(h, (f | str).value, str_data(std::forward<V>(v)));
+    return std::make_tuple(h, (f | str).value, str_data(v));
 }
 
 /** Logging conversion for pointer-types. */
@@ -263,7 +263,7 @@
     static void done(level l, const lg2::source_location& s, const char* m,
                      Ts&&... ts)
     {
-        do_log(l, s, m, std::forward<Ts>(ts)..., nullptr);
+        do_log(l, s, m, ts..., nullptr);
     }
 
     /** Apply the tuple from the end of 'step' into done.
@@ -317,7 +317,7 @@
         else
         {
             step(std::tuple_cat(std::move(ts), log_convert(h.data(), f, v)),
-                 std::forward<Ss>(ss)...);
+                 ss...);
         }
     }
 
@@ -346,7 +346,7 @@
         {
             step(std::tuple_cat(std::move(ts),
                                 log_convert(h.data(), log_flag<>{}, v)),
-                 std::forward<Ss>(ss)...);
+                 ss...);
         }
     }
 
@@ -384,7 +384,7 @@
         // Handle all the Ts by recursively calling 'step'.
         else
         {
-            step(std::forward_as_tuple(l, s, msg), std::forward<Ts>(ts)...);
+            step(std::forward_as_tuple(l, s, msg), ts...);
         }
     }
 };