Update cpp/src/arrow/type.h

Co-authored-by: Antoine Pitrou <pitrou@free.fr>

Author: harshkumar-2005

cpp/src/arrow/scalar_test.cc

@@ -180,8 +180,7 @@ TEST(TestScalar, IdentityCast) {
     test_identity_cast_for_type(type);
   }
   for (auto& type : {
-           arrow::fixed_size_list(arrow::int32(), 20),
-           arrow::list(arrow::int32()),
+           arrow::fixed_size_list(arrow::int32(), 20), arrow::list(arrow::int32()),
            arrow::large_list(arrow::int32()),
            // TODO(GH-45430): CastTo for ListView is not implemented yet.
            // arrow::list_view(arrow::int32()), arrow::large_list_view(arrow::int32())

cpp/src/arrow/type.cc

@@ -3554,22 +3554,15 @@ const std::vector<Type::type>& DecimalTypeIds() {
 }
 
 Result<std::shared_ptr<DataType>> type_singleton(Type::type id) {
-  struct Visitor {
-    Result<std::shared_ptr<DataType>> result;
-
-    template <typename T>
-    Status Visit(const T* type) {
-      if constexpr (TypeTraits<T>::is_parameter_free) {
-        result = TypeTraits<T>::type_singleton();
-        return Status::OK();
-      }
-      return Status::TypeError("Type ", ToString(T::type_id), " is not a parameter-free type");
+  auto visit = [](auto type) -> Result<std::shared_ptr<DataType>> {
+    using T = std::decay_t<decltype(*type)>;
+    if constexpr (TypeTraits<T>::is_parameter_free) {
+      return TypeTraits<T>::type_singleton();
     }
+    return Status::TypeError("Type ", internal::ToString(T::type_id),
+                             " is not a parameter-free type");
   };
-
-  Visitor visitor;
-  RETURN_NOT_OK(VisitTypeIdInline(id, &visitor));
-  return visitor.result;
+  return VisitTypeId(id, visit);
 }
 
 }  // namespace arrow

cpp/src/arrow/type.h

@@ -2653,8 +2653,8 @@ const std::vector<Type::type>& DecimalTypeIds();
 /// this function will return an error.
 ///
 /// \param[in] id The type ID to create a type instance for
-/// \return Result with a shared pointer to the created DataType instance,
-///         or an error if the type requires parameters
+/// \return The type instance for the given type ID,
+///         or a TypeError if the type requires parameters
 ARROW_EXPORT
 Result<std::shared_ptr<DataType>> type_singleton(Type::type id);
 

cpp/src/arrow/type_test.cc

@@ -40,7 +40,6 @@
 #include "arrow/type_traits.h"
 #include "arrow/util/checked_cast.h"
 #include "arrow/util/key_value_metadata.h"
-#include "arrow/util/logging.h"
 
 namespace arrow {
 
@@ -55,20 +54,15 @@ TEST(TestTypeId, AllTypeIds) {
 TEST(TestTypeSingleton, ParameterFreeTypes) {
   // Test successful cases - parameter-free types (sample a few)
   std::vector<std::pair<Type::type, std::shared_ptr<DataType>>> cases = {
-      {Type::NA, null()},
-      {Type::BOOL, boolean()},
-      {Type::INT32, int32()},
-      {Type::STRING, utf8()},
-      {Type::DATE32, date32()},
+      {Type::NA, null()},     {Type::BOOL, boolean()},  {Type::INT32, int32()},
+      {Type::STRING, utf8()}, {Type::DATE32, date32()},
   };
 
   for (const auto& test_case : cases) {
-    SCOPED_TRACE("Testing type: " + std::to_string(static_cast<int>(test_case.first)));
+    ARROW_SCOPED_TRACE("Testing type: ", internal::ToString(test_case.first));
     auto result = type_singleton(test_case.first);
     ASSERT_OK_AND_ASSIGN(auto type, result);
-    ASSERT_TRUE(type->Equals(*test_case.second))
-        << "Failed on type " << test_case.first << ". Expected "
-        << test_case.second->ToString() << " but got " << type->ToString();
+    AssertTypeEqual(*type, *test_case.second);
   }
 }
 
@@ -77,14 +71,7 @@ TEST(TestTypeSingleton, ParameterizedTypes) {
   auto result = type_singleton(Type::TIMESTAMP);
   ASSERT_RAISES(TypeError, result);
   EXPECT_THAT(result.status().message(),
-              testing::HasSubstr("is not a parameter-free singleton type"));
-}
-
-TEST(TestTypeSingleton, InvalidType) {
-  // Test with an invalid type ID
-  auto result = type_singleton(static_cast<Type::type>(9999));
-  ASSERT_FALSE(result.ok());
-  EXPECT_TRUE(result.status().IsTypeError());
+              testing::HasSubstr("is not a parameter-free type"));
 }
 
 template <typename ReprFunc>

cpp/src/arrow/type_traits.cc

@@ -16,9 +16,11 @@
 // under the License.
 
 #include "arrow/type_traits.h"
-#include "arrow/type.h"
+
 #include "arrow/util/logging_internal.h"
 
+namespace arrow {
+
 int RequiredValueAlignmentForBuffer(Type::type type_id, int buffer_index) {
   if (buffer_index == 2 && type_id == Type::DENSE_UNION) {
     // A dense union array is the only array (so far) that requires alignment

cpp/src/arrow/type_traits.h

@@ -22,10 +22,8 @@
 #include <type_traits>
 #include <vector>
 
-#include "arrow/result.h"
 #include "arrow/type.h"
 #include "arrow/util/bit_util.h"
-#include "arrow/util/macros.h"
 
 namespace arrow {
 

cpp/src/arrow/visit_type_inline.h

@@ -71,10 +71,8 @@ inline Status VisitTypeInline(const DataType& type, VISITOR* visitor, ARGS&&...
 /// \tparam ARGS Additional arguments, if any, will be passed to the Visit function after
 /// the `type` argument
 ///
-/// Unlike VisitTypeInline which calls `visitor.Visit`, here `visitor`
+/// Unlike VisitTypeInline which calls `visitor->Visit`, here `visitor`
 /// itself is called.
-/// `visitor` must support a `const DataType&` argument as a fallback,
-/// in addition to concrete type classes.
 ///
 /// The intent is for this to be called on a generic lambda
 /// that may internally use `if constexpr` or similar constructs.
@@ -114,4 +112,32 @@ inline Status VisitTypeIdInline(Type::type id, VISITOR* visitor, ARGS&&... args)
 
 #undef TYPE_ID_VISIT_INLINE
 
+#define TYPE_ID_VISIT_INLINE(TYPE_CLASS)                                              \
+  case TYPE_CLASS##Type::type_id: {                                                   \
+    const TYPE_CLASS##Type* concrete_ptr = NULLPTR;                                   \
+    return std::forward<VISITOR>(visitor)(concrete_ptr, std::forward<ARGS>(args)...); \
+  }
+
+/// \brief Calls `visitor` with a nullptr of the corresponding concrete type class
+/// \tparam ARGS Additional arguments, if any, will be passed to the Visit function after
+/// the `type` argument
+///
+/// Unlike VisitTypeIdInline which calls `visitor->Visit`, here `visitor`
+/// itself is called.
+///
+/// The intent is for this to be called on a generic lambda
+/// that may internally use `if constexpr` or similar constructs.
+template <typename VISITOR, typename... ARGS>
+inline auto VisitTypeId(Type::type id, VISITOR&& visitor, ARGS&&... args)
+    -> decltype(std::forward<VISITOR>(visitor)(std::declval<DataType*>(), args...)) {
+  switch (id) {
+    ARROW_GENERATE_FOR_ALL_TYPES(TYPE_ID_VISIT_INLINE);
+    default:
+      break;
+  }
+  return Status::NotImplemented("Type not implemented");
+}
+
+#undef TYPE_ID_VISIT_INLINE
+
 }  // namespace arrow