ARROW-15415: [C++] Fixes for MSVC + vcpkg Debug build

cpp/cmake_modules/FindThrift.cmake

@@ -47,9 +47,17 @@ endfunction(EXTRACT_THRIFT_VERSION)
 if(MSVC_TOOLCHAIN AND NOT DEFINED THRIFT_MSVC_LIB_SUFFIX)
   if(NOT ARROW_THRIFT_USE_SHARED)
     if(ARROW_USE_STATIC_CRT)
-      set(THRIFT_MSVC_LIB_SUFFIX "mt")
+      if("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
+        set(THRIFT_MSVC_LIB_SUFFIX "mtd")
+      else()
+        set(THRIFT_MSVC_LIB_SUFFIX "mt")
+      endif()
     else()
-      set(THRIFT_MSVC_LIB_SUFFIX "md")
+      if("${CMAKE_BUILD_TYPE}" STREQUAL "DEBUG")
+        set(THRIFT_MSVC_LIB_SUFFIX "mdd")
+      else()
+        set(THRIFT_MSVC_LIB_SUFFIX "md")
+      endif()
     endif()
   endif()
 endif()

cpp/src/arrow/compute/exec/exec_plan.cc

@@ -85,9 +85,11 @@ struct ExecPlanImpl : public ExecPlan {
 #ifdef ARROW_WITH_OPENTELEMETRY
     if (HasMetadata()) {
       auto pairs = metadata().get()->sorted_pairs();
+      opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> span =
+          ::arrow::internal::tracing::UnwrapSpan(span_.details.get());
       std::for_each(std::begin(pairs), std::end(pairs),
-                    [this](std::pair<std::string, std::string> const& pair) {
-                      span_.Get().span->SetAttribute(pair.first, pair.second);
+                    [span](std::pair<std::string, std::string> const& pair) {
+                      span->SetAttribute(pair.first, pair.second);
                     });
     }
 #endif

cpp/src/arrow/compute/exec/hash_join.cc

@@ -29,6 +29,7 @@
 #include "arrow/compute/exec/key_hash.h"
 #include "arrow/compute/exec/task_util.h"
 #include "arrow/compute/kernels/row_encoder.h"
+#include "arrow/util/tracing_internal.h"
 
 namespace arrow {
 namespace compute {

cpp/src/arrow/compute/exec/hash_join.h

@@ -28,7 +28,7 @@
 #include "arrow/result.h"
 #include "arrow/status.h"
 #include "arrow/type.h"
-#include "arrow/util/tracing_internal.h"
+#include "arrow/util/tracing.h"
 
 namespace arrow {
 namespace compute {

cpp/src/arrow/compute/exec/hash_join_node.cc

@@ -28,6 +28,7 @@
 #include "arrow/util/future.h"
 #include "arrow/util/make_unique.h"
 #include "arrow/util/thread_pool.h"
+#include "arrow/util/tracing_internal.h"
 
 namespace arrow {
 

cpp/src/arrow/compute/function.cc

@@ -215,6 +215,7 @@ Result<Datum> Function::Execute(const std::vector<Datum>& args,
   }
 
   util::tracing::Span span;
+
   START_COMPUTE_SPAN(span, name(),
                      {{"function.name", name()},
                       {"function.options", options ? options->ToString() : "<NULLPTR>"},

cpp/src/arrow/compute/light_array_test.cc

@@ -18,6 +18,7 @@
 #include "arrow/compute/light_array.h"
 
 #include <gtest/gtest.h>
+#include <numeric>
 
 #include "arrow/compute/exec/test_util.h"
 #include "arrow/testing/generator.h"

cpp/src/arrow/util/tracing.cc

@@ -16,30 +16,28 @@
 // under the License.
 
 #include "arrow/util/tracing.h"
+
+#include "arrow/util/config.h"
 #include "arrow/util/make_unique.h"
 #include "arrow/util/tracing_internal.h"
 
 namespace arrow {
+
+using internal::make_unique;
 namespace util {
 namespace tracing {
 
 #ifdef ARROW_WITH_OPENTELEMETRY
 
-Span::Impl& Span::Set(const Impl& impl) {
-  inner_impl.reset(new Impl(impl));
-  return *inner_impl;
-}
+Span::Span() noexcept { details = make_unique<::arrow::internal::tracing::SpanImpl>(); }
 
-Span::Impl& Span::Set(Impl&& impl) {
-  inner_impl.reset(new Impl(std::move(impl)));
-  return *inner_impl;
+#else
+
+Span::Span() noexcept { /* details is left a nullptr */
 }
 
 #endif
 
-// Default destructor when impl type is complete.
-Span::~Span() = default;
-
 }  // namespace tracing
 }  // namespace util
 }  // namespace arrow

cpp/src/arrow/util/tracing.h

@@ -19,49 +19,21 @@
 
 #include <memory>
 
-#include "arrow/util/logging.h"
+#include "arrow/util/visibility.h"
 
 namespace arrow {
-
-namespace internal {
-namespace tracing {
-
-// Forward declaration SpanImpl.
-class SpanImpl;
-
-}  // namespace tracing
-}  // namespace internal
-
 namespace util {
 namespace tracing {
 
-class ARROW_EXPORT Span {
+class ARROW_EXPORT SpanDetails {
  public:
-  using Impl = arrow::internal::tracing::SpanImpl;
-
-  Span() = default;  // Default constructor. The inner_impl is a nullptr.
-  ~Span();  // Destructor. Default destructor defined in tracing.cc where impl is a
-            // complete type.
-
-  Impl& Set(const Impl&);
-  Impl& Set(Impl&&);
-
-  const Impl& Get() const {
-    ARROW_CHECK(inner_impl)
-        << "Attempted to dereference a null pointer. Use Span::Set before "
-           "dereferencing.";
-    return *inner_impl;
-  }
-
-  Impl& Get() {
-    ARROW_CHECK(inner_impl)
-        << "Attempted to dereference a null pointer. Use Span::Set before "
-           "dereferencing.";
-    return *inner_impl;
-  }
+  virtual ~SpanDetails() {}
+};
 
- private:
-  std::unique_ptr<Impl> inner_impl;
+class ARROW_EXPORT Span {
+ public:
+  Span() noexcept;
+  std::unique_ptr<SpanDetails> details;
 };
 
 }  // namespace tracing

cpp/src/arrow/util/tracing_internal.cc

@@ -202,14 +202,38 @@ opentelemetry::trace::Tracer* GetTracer() {
   return tracer.get();
 }
 
-#ifdef ARROW_WITH_OPENTELEMETRY
+opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+    ::arrow::util::tracing::SpanDetails* span) {
+  SpanImpl* span_impl = checked_cast<SpanImpl*>(span);
+  ARROW_CHECK(span_impl->ot_span)
+      << "Attempted to dereference a null pointer. Use Span::Set before "
+         "dereferencing.";
+  return span_impl->ot_span;
+}
+
+const opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+    const ::arrow::util::tracing::SpanDetails* span) {
+  const SpanImpl* span_impl = checked_cast<const SpanImpl*>(span);
+  ARROW_CHECK(span_impl->ot_span)
+      << "Attempted to dereference a null pointer. Use Span::Set before "
+         "dereferencing.";
+  return span_impl->ot_span;
+}
+
+opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& RewrapSpan(
+    ::arrow::util::tracing::SpanDetails* span,
+    opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> ot_span) {
+  SpanImpl* span_impl = checked_cast<SpanImpl*>(span);
+  span_impl->ot_span = std::move(ot_span);
+  return span_impl->ot_span;
+}
+
 opentelemetry::trace::StartSpanOptions SpanOptionsWithParent(
     const util::tracing::Span& parent_span) {
   opentelemetry::trace::StartSpanOptions options;
-  options.parent = parent_span.Get().span->GetContext();
+  options.parent = UnwrapSpan(parent_span.details.get())->GetContext();
   return options;
 }
-#endif
 
 }  // namespace tracing
 }  // namespace internal

cpp/src/arrow/util/tracing_internal.h

@@ -106,48 +106,63 @@ AsyncGenerator<T> PropagateSpanThroughAsyncGenerator(AsyncGenerator<T> wrapped)
   return PropagateSpanThroughAsyncGenerator(std::move(wrapped), std::move(span));
 }
 
-class SpanImpl {
+class SpanImpl : public ::arrow::util::tracing::SpanDetails {
  public:
-  opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> span;
+  ~SpanImpl() override = default;
+  opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> ot_span;
 };
 
+opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+    ::arrow::util::tracing::SpanDetails* span);
+
+const opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& UnwrapSpan(
+    const ::arrow::util::tracing::SpanDetails* span);
+
+opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span>& RewrapSpan(
+    ::arrow::util::tracing::SpanDetails* span,
+    opentelemetry::nostd::shared_ptr<opentelemetry::trace::Span> ot_span);
+
 opentelemetry::trace::StartSpanOptions SpanOptionsWithParent(
     const util::tracing::Span& parent_span);
 
-#define START_SPAN(target_span, ...)                                                \
-  auto opentelemetry_scope##__LINE__ =                                              \
-      ::arrow::internal::tracing::GetTracer()->WithActiveSpan(                      \
-          target_span                                                               \
-              .Set(::arrow::util::tracing::Span::Impl{                              \
-                  ::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__)}) \
-              .span)
-
-#define START_SPAN_WITH_PARENT(target_span, parent_span, ...)                           \
-  auto opentelemetry_scope##__LINE__ =                                                  \
-      ::arrow::internal::tracing::GetTracer()->WithActiveSpan(                          \
-          target_span                                                                   \
-              .Set(::arrow::util::tracing::Span::Impl{                                  \
-                  ::arrow::internal::tracing::GetTracer()->StartSpan(                   \
-                      __VA_ARGS__,                                                      \
-                      ::arrow::internal::tracing::SpanOptionsWithParent(parent_span))}) \
-              .span)
-
-#define START_COMPUTE_SPAN(target_span, ...) \
-  START_SPAN(target_span, __VA_ARGS__);      \
-  target_span.Get().span->SetAttribute(      \
-      "arrow.memory_pool_bytes", ::arrow::default_memory_pool()->bytes_allocated())
+#define START_SPAN(target_span, ...)                           \
+  auto opentelemetry_scope##__LINE__ =                         \
+      ::arrow::internal::tracing::GetTracer()->WithActiveSpan( \
+          ::arrow::internal::tracing::RewrapSpan(              \
+              target_span.details.get(),                       \
+              ::arrow::internal::tracing::GetTracer()->StartSpan(__VA_ARGS__)))
+
+#define START_SPAN_WITH_PARENT(target_span, parent_span, ...)     \
+  auto opentelemetry_scope##__LINE__ =                            \
+      ::arrow::internal::tracing::GetTracer()->WithActiveSpan(    \
+          ::arrow::internal::tracing::RewrapSpan(                 \
+              target_span.details.get(),                          \
+                                                                  \
+              ::arrow::internal::tracing::GetTracer()->StartSpan( \
+                  __VA_ARGS__,                                    \
+                  ::arrow::internal::tracing::SpanOptionsWithParent(parent_span))))
+
+#define START_COMPUTE_SPAN(target_span, ...)                        \
+  START_SPAN(target_span, __VA_ARGS__);                             \
+  ::arrow::internal::tracing::UnwrapSpan(target_span.details.get()) \
+      ->SetAttribute("arrow.memory_pool_bytes",                     \
+                     ::arrow::default_memory_pool()->bytes_allocated())
 
 #define START_COMPUTE_SPAN_WITH_PARENT(target_span, parent_span, ...) \
   START_SPAN_WITH_PARENT(target_span, parent_span, __VA_ARGS__);      \
-  target_span.Get().span->SetAttribute(                               \
-      "arrow.memory_pool_bytes", ::arrow::default_memory_pool()->bytes_allocated())
+  ::arrow::internal::tracing::UnwrapSpan(target_span.details.get())   \
+      ->SetAttribute("arrow.memory_pool_bytes",                       \
+                     ::arrow::default_memory_pool()->bytes_allocated())
 
-#define EVENT(target_span, ...) target_span.Get().span->AddEvent(__VA_ARGS__)
+#define EVENT(target_span, ...) \
+  ::arrow::internal::tracing::UnwrapSpan(target_span.details.get())->AddEvent(__VA_ARGS__)
 
-#define MARK_SPAN(target_span, status) \
-  ::arrow::internal::tracing::MarkSpan(status, target_span.Get().span.get())
+#define MARK_SPAN(target_span, status)  \
+  ::arrow::internal::tracing::MarkSpan( \
+      status, ::arrow::internal::tracing::UnwrapSpan(target_span.details.get()).get())
 
-#define END_SPAN(target_span) target_span.Get().span->End()
+#define END_SPAN(target_span) \
+  ::arrow::internal::tracing::UnwrapSpan(target_span.details.get())->End()
 
 #define END_SPAN_ON_FUTURE_COMPLETION(target_span, target_future, target_capture) \
   target_future = target_future.Then(                                             \