From e730f91d8cb6af19a172d6a36b4279181a02a9ff Mon Sep 17 00:00:00 2001
From: Enrico Seiler <eseiler@users.noreply.github.com>
Date: Wed, 5 Jul 2023 19:05:08 +0200
Subject: [PATCH] Fix passing non-const lvalue refs to DoNotOptimize (#1622)

---
 cmake/GoogleTest.cmake        | 20 +++++++++-----
 include/benchmark/benchmark.h | 50 +++++++++++++++++++++++++----------
 test/CMakeLists.txt           |  5 ++++
 3 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/cmake/GoogleTest.cmake b/cmake/GoogleTest.cmake
index 44adbfbe..e66e9d1a 100644
--- a/cmake/GoogleTest.cmake
+++ b/cmake/GoogleTest.cmake
@@ -29,19 +29,25 @@ set(gtest_force_shared_crt ON CACHE BOOL "" FORCE)
 
 include(${GOOGLETEST_PREFIX}/googletest-paths.cmake)
 
-# googletest doesn't seem to want to stay build warning clean so let's not hurt ourselves.
-if (MSVC)
-  add_compile_options(/wd4244 /wd4722)
-else()
-  add_compile_options(-w)
-endif()
-
 # Add googletest directly to our build. This defines
 # the gtest and gtest_main targets.
 add_subdirectory(${GOOGLETEST_SOURCE_DIR}
                  ${GOOGLETEST_BINARY_DIR}
                  EXCLUDE_FROM_ALL)
 
+# googletest doesn't seem to want to stay build warning clean so let's not hurt ourselves.
+if (MSVC)
+  target_compile_options(gtest PRIVATE "/wd4244" "/wd4722")
+  target_compile_options(gtest_main PRIVATE "/wd4244" "/wd4722")
+  target_compile_options(gmock PRIVATE "/wd4244" "/wd4722")
+  target_compile_options(gmock_main PRIVATE "/wd4244" "/wd4722")
+else()
+  target_compile_options(gtest PRIVATE "-w")
+  target_compile_options(gtest_main PRIVATE "-w")
+  target_compile_options(gmock PRIVATE "-w")
+  target_compile_options(gmock_main PRIVATE "-w")
+endif()
+
 if(NOT DEFINED GTEST_COMPILE_COMMANDS)
     set(GTEST_COMPILE_COMMANDS ON)
 endif()
diff --git a/include/benchmark/benchmark.h b/include/benchmark/benchmark.h
index 1444ec61..e3857e71 100644
--- a/include/benchmark/benchmark.h
+++ b/include/benchmark/benchmark.h
@@ -465,19 +465,24 @@ inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp const& value) {
 }
 
 template <class Tp>
-inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(
-#ifdef BENCHMARK_HAS_CXX11
-    Tp&& value
-#else
-    Tp& value
-#endif
-) {
+inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp& value) {
 #if defined(__clang__)
   asm volatile("" : "+r,m"(value) : : "memory");
 #else
   asm volatile("" : "+m,r"(value) : : "memory");
 #endif
 }
+
+#ifdef BENCHMARK_HAS_CXX11
+template <class Tp>
+inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp&& value) {
+#if defined(__clang__)
+  asm volatile("" : "+r,m"(value) : : "memory");
+#else
+  asm volatile("" : "+m,r"(value) : : "memory");
+#endif
+}
+#endif
 #elif defined(BENCHMARK_HAS_CXX11) && (__GNUC__ >= 5)
 // Workaround for a bug with full argument copy overhead with GCC.
 // See: #1340 and https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105519
@@ -503,6 +508,22 @@ inline BENCHMARK_ALWAYS_INLINE
   asm volatile("" : : "m"(value) : "memory");
 }
 
+template <class Tp>
+inline BENCHMARK_ALWAYS_INLINE
+    typename std::enable_if<std::is_trivially_copyable<Tp>::value &&
+                            (sizeof(Tp) <= sizeof(Tp*))>::type
+    DoNotOptimize(Tp& value) {
+  asm volatile("" : "+m,r"(value) : : "memory");
+}
+
+template <class Tp>
+inline BENCHMARK_ALWAYS_INLINE
+    typename std::enable_if<!std::is_trivially_copyable<Tp>::value ||
+                            (sizeof(Tp) > sizeof(Tp*))>::type
+    DoNotOptimize(Tp& value) {
+  asm volatile("" : "+m"(value) : : "memory");
+}
+
 template <class Tp>
 inline BENCHMARK_ALWAYS_INLINE
     typename std::enable_if<std::is_trivially_copyable<Tp>::value &&
@@ -532,15 +553,16 @@ inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp const& value) {
 }
 
 template <class Tp>
-inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(
-#ifdef BENCHMARK_HAS_CXX11
-    Tp&& value
-#else
-    Tp& value
-#endif
-) {
+inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp& value) {
   asm volatile("" : "+m"(value) : : "memory");
 }
+
+#ifdef BENCHMARK_HAS_CXX11
+template <class Tp>
+inline BENCHMARK_ALWAYS_INLINE void DoNotOptimize(Tp&& value) {
+  asm volatile("" : "+m"(value) : : "memory");
+}
+#endif
 #endif
 
 #ifndef BENCHMARK_HAS_CXX11
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 78d6d517..fd881319 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -122,6 +122,11 @@ compile_benchmark_test(skip_with_error_test)
 add_test(NAME skip_with_error_test COMMAND skip_with_error_test --benchmark_min_time=0.01s)
 
 compile_benchmark_test(donotoptimize_test)
+# Enable errors for deprecated deprecations (DoNotOptimize(Tp const& value)).
+check_cxx_compiler_flag(-Werror=deprecated-declarations BENCHMARK_HAS_DEPRECATED_DECLARATIONS_FLAG)
+if (BENCHMARK_HAS_DEPRECATED_DECLARATIONS_FLAG)
+  target_compile_options (donotoptimize_test PRIVATE "-Werror=deprecated-declarations")
+endif()
 # Some of the issues with DoNotOptimize only occur when optimization is enabled
 check_cxx_compiler_flag(-O3 BENCHMARK_HAS_O3_FLAG)
 if (BENCHMARK_HAS_O3_FLAG)