[CODE HEALTH] Fix clang-tidy warnings in nostd files (#3924)

Author: dbarker

api/include/opentelemetry/nostd/function_ref.h

@@ -37,7 +37,9 @@ class function_ref<R(Args...)>
   void BindTo(F &f) noexcept
   {
     callable_ = static_cast<void *>(std::addressof(f));
-    invoker_  = [](void *callable, Args... args) -> R {
+    // Args... matches the referenced signature and is forwarded as such.
+    // NOLINTNEXTLINE(performance-unnecessary-value-param)
+    invoker_ = [](void *callable, Args... args) -> R {
       return (*static_cast<FunctionPointer<F>>(callable))(std::forward<Args>(args)...);
     };
   }
@@ -78,14 +80,21 @@ class function_ref<R(Args...)>
           int>::type = 0>
   function_ref(F &&f)
   {
+    // Binding by named variable here intentionally keeps function_ref non-owning.
+    // NOLINTNEXTLINE(cppcoreguidelines-missing-std-forward)
     BindTo(f);  // not forward
   }
 
   function_ref(std::nullptr_t) {}
 
-  function_ref(const function_ref &) noexcept = default;
-  function_ref(function_ref &&) noexcept      = default;
+  ~function_ref()                                        = default;
+  function_ref(const function_ref &) noexcept            = default;
+  function_ref(function_ref &&) noexcept                 = default;
+  function_ref &operator=(const function_ref &) noexcept = default;
+  function_ref &operator=(function_ref &&) noexcept      = default;
 
+  // Args... is part of the function signature and forwarded to invoker_.
+  // NOLINTNEXTLINE(performance-unnecessary-value-param)
   R operator()(Args... args) const { return invoker_(callable_, std::forward<Args>(args)...); }
 
   explicit operator bool() const { return invoker_; }

api/include/opentelemetry/nostd/shared_ptr.h

@@ -47,6 +47,11 @@ class shared_ptr
 
     shared_ptr_wrapper(std::shared_ptr<T> &&ptr) noexcept : ptr_{std::move(ptr)} {}
 
+    shared_ptr_wrapper(const shared_ptr_wrapper &)            = default;
+    shared_ptr_wrapper &operator=(const shared_ptr_wrapper &) = default;
+    shared_ptr_wrapper(shared_ptr_wrapper &&)                 = default;
+    shared_ptr_wrapper &operator=(shared_ptr_wrapper &&)      = default;
+
     virtual ~shared_ptr_wrapper() {}
 
     virtual void CopyTo(PlacementBuffer &buffer) const noexcept
@@ -98,20 +103,20 @@ class shared_ptr
             typename std::enable_if<std::is_convertible<U *, pointer>::value>::type * = nullptr>
   shared_ptr(shared_ptr<U> &&other) noexcept
   {
-    other.wrapper().template MoveTo<T>(buffer_);
+    std::move(other).wrapper().template MoveTo<T>(buffer_);
   }
 
   shared_ptr(const shared_ptr &other) noexcept { other.wrapper().CopyTo(buffer_); }
 
   shared_ptr(unique_ptr<T> &&other) noexcept
   {
-    std::shared_ptr<T> ptr_(other.release());
+    std::shared_ptr<T> ptr_(std::move(other).release());
     new (buffer_.data) shared_ptr_wrapper{std::move(ptr_)};
   }
 
   shared_ptr(std::unique_ptr<T> &&other) noexcept
   {
-    std::shared_ptr<T> ptr_(other.release());
+    std::shared_ptr<T> ptr_(std::move(other).release());
     new (buffer_.data) shared_ptr_wrapper{std::move(ptr_)};
   }
 
@@ -155,10 +160,15 @@ class shared_ptr
 
   void swap(shared_ptr<T> &other) noexcept
   {
-    shared_ptr<T> tmp{std::move(other)};
+    if (this == &other)
+    {
+      return;
+    }
 
-    wrapper().MoveTo(other.buffer_);
-    tmp.wrapper().MoveTo(buffer_);
+    // Swap the live wrapper objects (object-level swap), not the raw
+    // PlacementBuffer bytes. This preserves object lifetime correctness and
+    // avoids moving `other` as an object.
+    std::swap(wrapper(), other.wrapper());
   }
 
   template <typename U>

api/include/opentelemetry/nostd/span.h

@@ -165,8 +165,14 @@ class span
 
   span(const span &) noexcept = default;
 
+  ~span() noexcept = default;
+
+  span(span &&) noexcept = default;
+
   span &operator=(const span &) noexcept = default;
 
+  span &operator=(span &&) noexcept = default;
+
   bool empty() const noexcept { return Extent == 0; }
 
   T *data() const noexcept { return data_; }
@@ -247,8 +253,14 @@ class span<T, dynamic_extent>
 
   span(const span &) noexcept = default;
 
+  ~span() noexcept = default;
+
+  span(span &&) noexcept = default;
+
   span &operator=(const span &) noexcept = default;
 
+  span &operator=(span &&) noexcept = default;
+
   bool empty() const noexcept { return extent_ == 0; }
 
   T *data() const noexcept { return data_; }

api/include/opentelemetry/nostd/unique_ptr.h

@@ -56,14 +56,17 @@ class unique_ptr
 
   unique_ptr(unique_ptr &&other) noexcept : ptr_{other.release()} {}
 
+  unique_ptr(const unique_ptr &)            = delete;
+  unique_ptr &operator=(const unique_ptr &) = delete;
+
   template <class U,
             typename std::enable_if<std::is_convertible<U *, pointer>::value>::type * = nullptr>
-  unique_ptr(unique_ptr<U> &&other) noexcept : ptr_{other.release()}
+  unique_ptr(unique_ptr<U> &&other) noexcept : ptr_{std::move(other).release()}
   {}
 
   template <class U,
             typename std::enable_if<std::is_convertible<U *, pointer>::value>::type * = nullptr>
-  unique_ptr(std::unique_ptr<U> &&other) noexcept : ptr_{other.release()}
+  unique_ptr(std::unique_ptr<U> &&other) noexcept : ptr_{std::move(other).release()}
   {}
 
   ~unique_ptr() { reset(); }
@@ -84,15 +87,15 @@ class unique_ptr
             typename std::enable_if<std::is_convertible<U *, pointer>::value>::type * = nullptr>
   unique_ptr &operator=(unique_ptr<U> &&other) noexcept
   {
-    reset(other.release());
+    reset(std::move(other).release());
     return *this;
   }
 
   template <class U,
             typename std::enable_if<std::is_convertible<U *, pointer>::value>::type * = nullptr>
   unique_ptr &operator=(std::unique_ptr<U> &&other) noexcept
   {
-    reset(other.release());
+    reset(std::move(other).release());
     return *this;
   }
 

api/include/opentelemetry/nostd/variant.h

@@ -51,6 +51,8 @@ OPENTELEMETRY_END_NAMESPACE
 
 #  include "opentelemetry/nostd/internal/absl/base/options.h"
 
+// Forward declarations needed by the local Abseil snapshot bridge.
+// NOLINTBEGIN(abseil-no-namespace)
 namespace absl
 {
 namespace OTABSL_OPTION_NAMESPACE_NAME
@@ -61,6 +63,7 @@ template <typename... Ts>
 class variant;
 }  // namespace OTABSL_OPTION_NAMESPACE_NAME
 }  // namespace absl
+// NOLINTEND(abseil-no-namespace)
 
 #  include "opentelemetry/nostd/internal/absl/types/variant.h"
 

api/test/nostd/shared_ptr_test.cc

@@ -166,6 +166,31 @@ TEST(SharedPtrTest, Swap)
   EXPECT_EQ(ptr2.get(), value1);
 }
 
+TEST(SharedPtrTest, SwapSelfNoOp)
+{
+  struct TestStruct
+  {
+    explicit TestStruct(int &destruct_count) noexcept : destruct_count_{&destruct_count} {}
+
+    ~TestStruct() { ++(*destruct_count_); }
+
+    int *destruct_count_;
+  };
+
+  int destruct_count{0};
+
+  {
+    shared_ptr<TestStruct> ptr{std::make_shared<TestStruct>(destruct_count)};
+    auto *ptr_before = ptr.get();
+
+    ptr.swap(ptr);
+
+    EXPECT_EQ(ptr.get(), ptr_before);
+  }
+
+  EXPECT_EQ(destruct_count, 1);
+}
+
 TEST(SharedPtrTest, Comparison)
 {
   shared_ptr<int> ptr1{new int{123}};