Some reformatting, + a NEWS entry

Author: kevinushey

inst/NEWS.Rd

@@ -11,6 +11,10 @@
       objects
       \item Add missing default constructor to Reference class that was
       omitted in the header-only rewrite
+      \item Fixes re: the NA, NaN handling for the IndexHash class, as well
+      as the vector .sort() method. These fixes ensure that sugar functions
+      depending on IndexHash (unique, sort_unique, match) will now properly
+      handle NA and NaN values for numeric vectors.
     }
   }
 }

inst/include/Rcpp/hash/IndexHash.h

@@ -5,6 +5,7 @@
 //
 // Copyright (C) 2010, 2011  Simon Urbanek
 // Copyright (C) 2012  Dirk Eddelbuettel and Romain Francois
+// Copyright (C) 2014  Dirk Eddelbuettel, Romain Francois and Kevin Ushey
 //
 // This file is part of Rcpp.
 //

inst/include/Rcpp/internal/NAComparator.h

@@ -28,54 +28,55 @@ namespace Rcpp{
 namespace internal {
 
 inline int StrCmp(SEXP x, SEXP y) {
-  if (x == NA_STRING) return (y == NA_STRING ? 0 : 1);
-  if (y == NA_STRING) return -1;
-  if (x == y) return 0;  // same string in cache
-  return strcmp(char_nocheck(x), char_nocheck(y));
+    if (x == NA_STRING) return (y == NA_STRING ? 0 : 1);
+    if (y == NA_STRING) return -1;
+    if (x == y) return 0;  // same string in cache
+    return strcmp(char_nocheck(x), char_nocheck(y));
 }
 
 template <typename T>
 struct NAComparator {
-  inline bool operator()(T left, T right) const {
-    return left < right;
-  }
+    inline bool operator()(T left, T right) const {
+        return left < right;
+    }
 };
 
 template <>
 struct NAComparator<int> {
-  inline bool operator()(int left, int right) const {
-    if (left == NA_INTEGER) return false;
-    if (right == NA_INTEGER) return true;
-    return left < right;
-  }
+    inline bool operator()(int left, int right) const {
+        if (left == NA_INTEGER) return false;
+        if (right == NA_INTEGER) return true;
+        return left < right;
+    }
 };
 
 template <>
 struct NAComparator<double> {  
-  inline bool operator()(double left, double right) const {
+    inline bool operator()(double left, double right) const {
 
-    bool leftNaN = (left != left);
-    bool rightNaN = (right != right);
-    
-    // this branch inspired by data.table: see
-    // https://github.com/arunsrinivasan/datatable/commit/1a3e476d3f746e18261662f484d2afa84ac7a146#commitcomment-4885242
-    if (Rcpp_IsNaN(right) and Rcpp_IsNA(left)) return true;
+        bool leftNaN = (left != left);
+        bool rightNaN = (right != right);
+        
+        // this branch inspired by data.table: see
+        // https://github.com/arunsrinivasan/datatable/commit/1a3e476d3f746e18261662f484d2afa84ac7a146#commitcomment-4885242
+        if (Rcpp_IsNaN(right) and Rcpp_IsNA(left)) 
+            return true;
+        
+        if (leftNaN != rightNaN) {
+            return leftNaN < rightNaN;
+        } else {
+            return left < right;
+        }
 
-    if (leftNaN != rightNaN) {
-      return leftNaN < rightNaN;
-    } else {
-      return left < right;
-    }
-
-  }
+      }
 
 };
 
 template <>
 struct NAComparator<SEXP> {
-  inline bool operator()(SEXP left, SEXP right) const {
-    return StrCmp(left, right) < 0;
-  }
+    inline bool operator()(SEXP left, SEXP right) const {
+        return StrCmp(left, right) < 0;
+    }
 };
 
 }

inst/include/Rcpp/internal/NAEquals.h

@@ -26,17 +26,17 @@ namespace internal {
 
 template <typename T>
 struct NAEquals {
-  inline bool operator()(T left, T right) const {
-    return left == right;
-  }
+    inline bool operator()(T left, T right) const {
+        return left == right;
+    }
 };
 
 // TODO: check different kinds of NA, NaNs
 template <>
 struct NAEquals<double> {
-  inline bool operator()(double left, double right) const {
-    return memcmp(&left, &right, sizeof(double)) == 0;
-  }
+    inline bool operator()(double left, double right) const {
+        return memcmp(&left, &right, sizeof(double)) == 0;
+    }
 };
 
 }

inst/include/Rcpp/internal/na.h

@@ -21,77 +21,80 @@
 // along with Rcpp.  If not, see <http://www.gnu.org/licenses/>.
 
 namespace Rcpp {
-  namespace internal {
+namespace internal {
 
-    // On 64bit processors, NAs can change
-    // we can still get a performance benefit by checking for specific
-    // bit patterns, though
-    
-    // we rely on the presence of unsigned long long types (could do it with
-    // a union, but that's messier; this is cleaner)
-    #ifdef RCPP_HAS_LONG_LONG_TYPES
-    
-    #ifdef HAS_STATIC_ASSERT
-    static_assert( 
-      sizeof(rcpp_ulong_long_type) == sizeof(double),
-      "unsigned long long and double have same size"
-    );
-    #endif
-    
-    static const rcpp_ulong_long_type SmallNA = 0x7FF00000000007A2;
-    static const rcpp_ulong_long_type LargeNA = 0x7FF80000000007A2;
-    
-    struct NACanChange {
-      enum { value = sizeof(void*) == 8 };
-    };
-    
-    template <bool NACanChange>
-    bool Rcpp_IsNA__impl(double);
-    
-    template <>
-    inline bool Rcpp_IsNA__impl<true>(double x) {
-      return memcmp(
+// On 64bit processors, NAs can change
+// we can still get a performance benefit by checking for specific
+// bit patterns, though
+
+// we rely on the presence of unsigned long long types (could do it with
+// a union, but that's messier; this is cleaner)
+#ifdef RCPP_HAS_LONG_LONG_TYPES
+
+#ifdef HAS_STATIC_ASSERT
+static_assert( 
+    sizeof(rcpp_ulong_long_type) == sizeof(double),
+    "unsigned long long and double have same size"
+);
+#endif
+
+// motivation: on 32bit architectures, we only see 'LargeNA'
+// as defined ashead; on 64bit architectures, R defaults to
+// 'SmallNA' for R_NaReal, but this can get promoted to 'LargeNA'
+// if a certain operation can create a 'signalling' NA.
+static const rcpp_ulong_long_type SmallNA = 0x7FF00000000007A2;
+static const rcpp_ulong_long_type LargeNA = 0x7FF80000000007A2;
+
+struct NACanChange {
+    enum { value = sizeof(void*) == 8 };
+};
+
+template <bool NACanChange>
+bool Rcpp_IsNA__impl(double);
+
+template <>
+inline bool Rcpp_IsNA__impl<true>(double x) {
+    return memcmp(
         (void*) &x,
         (void*) &SmallNA,
         sizeof(double)
-      ) == 0 or memcmp(
+    ) == 0 or memcmp(
         (void*) &x,
         (void*) &LargeNA,
         sizeof(double)
-      ) == 0;
-    }
-    
-    template <>
-    inline bool Rcpp_IsNA__impl<false>(double x) {
-      printf("false\n");
-      return memcmp(
+    ) == 0;
+}
+
+template <>
+inline bool Rcpp_IsNA__impl<false>(double x) {
+    return memcmp(
         (void*) &x,
         (void*) &LargeNA,
         sizeof(double)
-      ) == 0;
-    }
-    
-    inline bool Rcpp_IsNA(double x) {
-      return Rcpp_IsNA__impl< NACanChange::value >(x);
-    }
-    
-    inline bool Rcpp_IsNaN(double x) {
-      return R_IsNaN(x);
-    }
-    
-    #else
-    
-    // fallback when we don't have unsigned long long
-    
-    inline bool Rcpp_IsNA(double x) {
-      return R_IsNA(x);
-    }
-    
-    inline bool Rcpp_IsNaN(double x) {
-      return R_IsNaN(x);
-    }
-    
-    #endif
+    ) == 0;
+}
+
+inline bool Rcpp_IsNA(double x) {
+    return Rcpp_IsNA__impl< NACanChange::value >(x);
+}
+
+inline bool Rcpp_IsNaN(double x) {
+    return R_IsNaN(x);
+}
+
+#else
+
+// fallback when we don't have unsigned long long
+
+inline bool Rcpp_IsNA(double x) {
+    return R_IsNA(x);
+}
+
+inline bool Rcpp_IsNaN(double x) {
+    return R_IsNaN(x);
+}
+
+#endif
 
-  }
+}
 }

inst/unitTests/cpp/na.cpp

@@ -24,21 +24,21 @@ using namespace Rcpp ;
 
 // [[Rcpp::export]]
 bool Rcpp_IsNA(double x) {
-  return internal::Rcpp_IsNA(x);
+    return internal::Rcpp_IsNA(x);
 }
 
 // [[Rcpp::export]]
 bool Rcpp_IsNaN(double x) {
-  return internal::Rcpp_IsNaN(x);
+    return internal::Rcpp_IsNaN(x);
 }
 
 // [[Rcpp::export]]
 bool R_IsNA_(double x) {
-  return ::R_IsNA(x);
+    return ::R_IsNA(x);
 }
 
 // [[Rcpp::export]]
 bool R_IsNaN_(double x) {
-  return ::R_IsNaN(x);
+    return ::R_IsNaN(x);
 }
 

inst/unitTests/runit.na.R

@@ -21,17 +21,17 @@
 .runThisTest <- Sys.getenv("RunAllRcppTests") == "yes"
 
 if (.runThisTest) {
-  
-  .setUp <- Rcpp:::unitTestSetup("na.cpp")
-  
-  test.na <- function() {
+    
+.setUp <- Rcpp:::unitTestSetup("na.cpp")
+
+test.na <- function() {
     checkIdentical( R_IsNA_(NA_real_), Rcpp_IsNA(NA_real_) )
     checkIdentical( R_IsNA_(NA_real_+1), Rcpp_IsNA(NA_real_+1) )
     checkIdentical( R_IsNA_(NA_real_+NaN), Rcpp_IsNA(NA_real_+NaN) )
     checkIdentical( R_IsNA_(NaN+NA_real_), Rcpp_IsNA(NaN+NA_real_) )
     checkIdentical( R_IsNaN_(NA_real_), Rcpp_IsNaN(NA_real_) )
     checkIdentical( R_IsNaN_(NaN), Rcpp_IsNaN(NaN) )
     checkIdentical( R_IsNaN_(NaN+1), Rcpp_IsNaN(NaN+1) )
-  }
-  
+}
+    
 }