From 991fa6682e819590c695f00c6b880548e55fa914 Mon Sep 17 00:00:00 2001
From: dmitrykobets-msft <89153909+dmitrykobets-msft@users.noreply.github.com>
Date: Tue, 11 Oct 2022 16:49:16 -0700
Subject: [PATCH] Prevent inefficient copying when using not_null::get (#1059)

Closes issue #550, which highlighted overhead in not_null::get for larger types such as shared_ptr. Every call to get would return a copy of the contained value.
This PR implements Herb's suggestion for changing the return type of not_null::get. The not_null's value will now only be copied if it is "trivially copyable"; otherwise, it will be returned by const reference.
Note: this change also forces the returned copy to be const.
---
 include/gsl/pointers    | 13 ++++++++++++-
 tests/notnull_tests.cpp |  2 +-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/gsl/pointers b/include/gsl/pointers
index a0a77ac..94a7192 100644
--- a/include/gsl/pointers
+++ b/include/gsl/pointers
@@ -46,6 +46,17 @@ namespace details
         : std::true_type
     {
     };
+
+    // Resolves to the more efficient of `const T` or `const T&`, in the context of returning a const-qualified value
+    // of type T.
+    //
+    // Copied from cppfront's implementation of the CppCoreGuidelines F.16 (https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-in)
+    template<typename T>
+    using value_or_reference_return_t = std::conditional_t<
+                                            sizeof(T) < 2*sizeof(void*) && std::is_trivially_copy_constructible<T>::value,
+                                            const T,
+                                            const T&>;
+
 } // namespace details
 
 //
@@ -104,7 +115,7 @@ public:
 
     not_null(const not_null& other) = default;
     not_null& operator=(const not_null& other) = default;
-    constexpr std::conditional_t<std::is_copy_constructible<T>::value, T, const T&> get() const
+    constexpr details::value_or_reference_return_t<T> get() const
     {
         Ensures(ptr_ != nullptr);
         return ptr_;
diff --git a/tests/notnull_tests.cpp b/tests/notnull_tests.cpp
index ccb652a..420c87b 100644
--- a/tests/notnull_tests.cpp
+++ b/tests/notnull_tests.cpp
@@ -52,7 +52,7 @@ template <typename T>
 struct CustomPtr
 {
     CustomPtr(T* p) : p_(p) {}
-    operator T*() { return p_; }
+    operator T*() const { return p_; }
     bool operator!=(std::nullptr_t) const { return p_ != nullptr; }
     T* p_ = nullptr;
 };