lidavidm commented on a change in pull request #10544:
URL: https://github.com/apache/arrow/pull/10544#discussion_r653778253



##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -397,6 +397,136 @@ struct PowerChecked {
   }
 };
 
+// Stamp out a trig function for which +/-inf are domain errors
+#define TRIG_NO_INF(name, func)                                                
        \
+  struct Trig##name {                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) { \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };                                                                           
        \
+  struct Trig##name##Checked {                                                 
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val,    
        \
+                                                  Status* st) {                
        \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      if (ARROW_PREDICT_FALSE(std::isinf(val))) {                              
        \
+        *st = Status::Invalid("domain error");                                 
        \
+        return val;                                                            
        \
+      }                                                                        
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };
+
+TRIG_NO_INF(Sin, std::sin);
+TRIG_NO_INF(Cos, std::cos);
+TRIG_NO_INF(Tan, std::tan);
+
+struct TrigAsin {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::asin(val);
+  }
+};
+
+struct TrigAsinChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::asin(val);
+  }
+};
+
+struct TrigAcos {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::acos(val);
+  }
+};
+
+struct TrigAcosChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::acos(val);
+  }
+};
+
+struct TrigAtan {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::atan(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::atan(val);
+  }
+};
+
+struct TrigAtan2 {
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 x, 
Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 
x, Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+};

Review comment:
       What I see is that an implementation using IEEE floating point 
arithmetic specifically does not consider it a domain error: 
https://en.cppreference.com/w/cpp/numeric/math/atan2
   
   Though we could be more conservative.

##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -397,6 +397,136 @@ struct PowerChecked {
   }
 };
 
+// Stamp out a trig function for which +/-inf are domain errors
+#define TRIG_NO_INF(name, func)                                                
        \
+  struct Trig##name {                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) { \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };                                                                           
        \
+  struct Trig##name##Checked {                                                 
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val,    
        \
+                                                  Status* st) {                
        \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      if (ARROW_PREDICT_FALSE(std::isinf(val))) {                              
        \
+        *st = Status::Invalid("domain error");                                 
        \
+        return val;                                                            
        \
+      }                                                                        
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };
+
+TRIG_NO_INF(Sin, std::sin);
+TRIG_NO_INF(Cos, std::cos);
+TRIG_NO_INF(Tan, std::tan);
+
+struct TrigAsin {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::asin(val);
+  }
+};
+
+struct TrigAsinChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::asin(val);
+  }
+};
+
+struct TrigAcos {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::acos(val);
+  }
+};
+
+struct TrigAcosChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::acos(val);
+  }
+};
+
+struct TrigAtan {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::atan(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::atan(val);
+  }
+};
+
+struct TrigAtan2 {
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 x, 
Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 
x, Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+};

Review comment:
       Regarding underflow: I will adjust the kernels to check for this and see 
if I can find some values to test with.

##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -397,6 +397,136 @@ struct PowerChecked {
   }
 };
 
+// Stamp out a trig function for which +/-inf are domain errors
+#define TRIG_NO_INF(name, func)                                                
        \
+  struct Trig##name {                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) { \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };                                                                           
        \
+  struct Trig##name##Checked {                                                 
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val,    
        \
+                                                  Status* st) {                
        \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      if (ARROW_PREDICT_FALSE(std::isinf(val))) {                              
        \
+        *st = Status::Invalid("domain error");                                 
        \
+        return val;                                                            
        \
+      }                                                                        
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };
+
+TRIG_NO_INF(Sin, std::sin);
+TRIG_NO_INF(Cos, std::cos);
+TRIG_NO_INF(Tan, std::tan);
+
+struct TrigAsin {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::asin(val);
+  }
+};
+
+struct TrigAsinChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::asin(val);
+  }
+};
+
+struct TrigAcos {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::acos(val);
+  }
+};
+
+struct TrigAcosChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::acos(val);
+  }
+};
+
+struct TrigAtan {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::atan(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::atan(val);
+  }
+};
+
+struct TrigAtan2 {
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 x, 
Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 
x, Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+};

Review comment:
       Ah, ok. I do see that it'll set a floating-point exception that you can 
test for, but that may indeed be overkill.

##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -397,6 +397,136 @@ struct PowerChecked {
   }
 };
 
+// Stamp out a trig function for which +/-inf are domain errors
+#define TRIG_NO_INF(name, func)                                                
        \
+  struct Trig##name {                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) { \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };                                                                           
        \
+  struct Trig##name##Checked {                                                 
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val,    
        \
+                                                  Status* st) {                
        \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      if (ARROW_PREDICT_FALSE(std::isinf(val))) {                              
        \
+        *st = Status::Invalid("domain error");                                 
        \
+        return val;                                                            
        \
+      }                                                                        
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };
+
+TRIG_NO_INF(Sin, std::sin);
+TRIG_NO_INF(Cos, std::cos);
+TRIG_NO_INF(Tan, std::tan);
+
+struct TrigAsin {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::asin(val);
+  }
+};
+
+struct TrigAsinChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::asin(val);
+  }
+};
+
+struct TrigAcos {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::acos(val);
+  }
+};
+
+struct TrigAcosChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::acos(val);
+  }
+};
+
+struct TrigAtan {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::atan(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::atan(val);
+  }
+};
+
+struct TrigAtan2 {
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 x, 
Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 
x, Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+};

Review comment:
       I've fixed the docs and updated atan2 to explicitly check for the 0, 0 
case (including signed zeroes which are specified).

##########
File path: cpp/src/arrow/compute/kernels/scalar_arithmetic.cc
##########
@@ -397,6 +397,136 @@ struct PowerChecked {
   }
 };
 
+// Stamp out a trig function for which +/-inf are domain errors
+#define TRIG_NO_INF(name, func)                                                
        \
+  struct Trig##name {                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) { \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };                                                                           
        \
+  struct Trig##name##Checked {                                                 
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) 
{        \
+      static_assert(std::is_same<T, double>::value, "");                       
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+    template <typename T, typename Arg0>                                       
        \
+    static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val,    
        \
+                                                  Status* st) {                
        \
+      static_assert(std::is_same<T, Arg0>::value, "");                         
        \
+      if (ARROW_PREDICT_FALSE(std::isinf(val))) {                              
        \
+        *st = Status::Invalid("domain error");                                 
        \
+        return val;                                                            
        \
+      }                                                                        
        \
+      return func(val);                                                        
        \
+    }                                                                          
        \
+  };
+
+TRIG_NO_INF(Sin, std::sin);
+TRIG_NO_INF(Cos, std::cos);
+TRIG_NO_INF(Tan, std::tan);
+
+struct TrigAsin {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::asin(val);
+  }
+};
+
+struct TrigAsinChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::asin(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::asin(val);
+  }
+};
+
+struct TrigAcos {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::acos(val);
+  }
+};
+
+struct TrigAcosChecked {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::acos(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status* st) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    if (ARROW_PREDICT_FALSE((val < -1.0 || val > 1.0) && !std::isnan(val))) {
+      *st = Status::Invalid("domain error");
+      return val;
+    }
+    return std::acos(val);
+  }
+};
+
+struct TrigAtan {
+  template <typename T, typename Arg0>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 val, Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    return std::atan(val);
+  }
+
+  template <typename T, typename Arg0>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 val, 
Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    return std::atan(val);
+  }
+};
+
+struct TrigAtan2 {
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_integer<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 x, 
Status*) {
+    static_assert(std::is_same<T, double>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+
+  template <typename T, typename Arg0, typename Arg1>
+  static enable_if_floating_point<Arg0, T> Call(KernelContext*, Arg0 y, Arg1 
x, Status*) {
+    static_assert(std::is_same<T, Arg0>::value, "");
+    static_assert(std::is_same<Arg0, Arg1>::value, "");
+    return std::atan2(y, x);
+  }
+};

Review comment:
       Whoops, good catch - thanks!

##########
File path: cpp/src/arrow/compute/kernels/util_internal.h
##########
@@ -30,6 +30,24 @@ namespace arrow {
 namespace compute {
 namespace internal {
 
+// Used in some kernels and testing - not provided by default in MSVC
+// and _USE_MATH_DEFINES is not reliable with unity builds
+#ifndef M_E
+#define M_E 2.71828182845904523536
+#define M_LOG2E 1.44269504088896340736
+#define M_LOG10E 0.434294481903251827651
+#define M_LN2 0.693147180559945309417
+#define M_LN10 2.30258509299404568402
+#define M_PI 3.14159265358979323846
+#define M_PI_2 1.57079632679489661923
+#define M_PI_4 0.785398163397448309616
+#define M_1_PI 0.318309886183790671538
+#define M_2_PI 0.636619772367581343076
+#define M_2_SQRTPI 1.12837916709551257390
+#define M_SQRT2 1.41421356237309504880
+#define M_SQRT1_2 0.707106781186547524401
+#endif
+

Review comment:
       It's the opposite: if any translation unit forgets to add that define 
before including cmath, even if you don't forget, you'll get a build error, 
since cmath will already have been included.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to