[Rprotobuf-commits] r670 - in pkg: . inst/unitTests src

noreply at r-forge.r-project.org noreply at r-forge.r-project.org
Tue Dec 31 13:07:37 CET 2013


Author: murray
Date: 2013-12-31 13:07:37 +0100 (Tue, 31 Dec 2013)
New Revision: 670

Modified:
   pkg/ChangeLog
   pkg/inst/unitTests/runit.extremevalues.R
   pkg/inst/unitTests/runit.int64.R
   pkg/src/wrapper_Message.cpp
Log:
Fix type coercion bug in add() method for uint32s and add a missing
break statement that erroneously raised an error when setting some
int64 fields.  Make more function arguments const and remove a
superfluous BEGIN_RCPP/END_RCPP.  Also add comment about long-vector
support.  All of these fixes were highlighted by Flexelint.  Add tests
verifying the bugfixes.



Modified: pkg/ChangeLog
===================================================================
--- pkg/ChangeLog	2013-12-31 11:00:05 UTC (rev 669)
+++ pkg/ChangeLog	2013-12-31 12:07:37 UTC (rev 670)
@@ -1,3 +1,14 @@
+2013-12-31  Murray Stokely  <mstokely at google.com>
+
+	* src/wrapper_Message.cpp: Fix type coercion bug in add() method
+	  for uint32s and add a missing break statement that erroneously
+	  raised an error when setting some int64 fields.  Make more
+	  function arguments const and remove a superfluous
+	  BEGIN_RCPP/END_RCPP.  Also add comment about long-vector
+	  support.  All of these fixes were highlighted by Flexelint.
+	* inst/unitTests/runit.int64.R (test.int64): Add tests for above.
+	* inst/unitTests/runit.extremevalues.R (test.uint32): Idem.
+
 2013-12-30  Murray Stokely  <mstokely at google.com>
 
 	* inst/unitTests/runit.extremevalues.R (test.uint32): Add test

Modified: pkg/inst/unitTests/runit.extremevalues.R
===================================================================
--- pkg/inst/unitTests/runit.extremevalues.R	2013-12-31 11:00:05 UTC (rev 669)
+++ pkg/inst/unitTests/runit.extremevalues.R	2013-12-31 12:07:37 UTC (rev 670)
@@ -30,7 +30,9 @@
                 "4294967295")
     checkEquals(foo$optional_uint32,
                 foo$repeated_uint32[[1]])
-
+    foo$add("repeated_uint32", c(2^32 - 1, 2^32 - 1))
+    checkEquals(length(unique(foo$repeated_uint32)), 1)
+    
     # fixed32 are a more efficient representation of uint32
     foo$optional_fixed32 <- 2^32 - 1
     foo$repeated_fixed32 <- c(foo$optional_fixed32, foo$optional_fixed32)

Modified: pkg/inst/unitTests/runit.int64.R
===================================================================
--- pkg/inst/unitTests/runit.int64.R	2013-12-31 11:00:05 UTC (rev 669)
+++ pkg/inst/unitTests/runit.int64.R	2013-12-31 12:07:37 UTC (rev 670)
@@ -25,6 +25,10 @@
 
     a <- new(protobuf_unittest.TestAllTypes)
     a$repeated_int64 <- 1
+    # Now just test that we can use add to set int64 fields.
+    a$add("repeated_int64", 2:10)
+    checkEquals(length(a$repeated_int64), 10)
+    
     # Verify we can set character strings of large 64-bit ints
     a$repeated_int64 <- c("9007199254740992", "9007199254740993")
     checkEquals(length(a$repeated_int64), 2)
@@ -49,4 +53,5 @@
     options("RProtoBuf.int64AsString" = TRUE)
     # But we can see they are different if we treat them as strings.
     checkEquals(length(unique(a$repeated_int64)), 2)
+
 }

Modified: pkg/src/wrapper_Message.cpp
===================================================================
--- pkg/src/wrapper_Message.cpp	2013-12-31 11:00:05 UTC (rev 669)
+++ pkg/src/wrapper_Message.cpp	2013-12-31 12:07:37 UTC (rev 670)
@@ -9,8 +9,8 @@
 /* helpers */
 
 /* this is only to be called for repeated fields */
-int MESSAGE_GET_REPEATED_INT(GPB::Message* message, GPB::FieldDescriptor* field_desc, int index) {
-    BEGIN_RCPP
+int MESSAGE_GET_REPEATED_INT(const GPB::Message* message, const GPB::FieldDescriptor* field_desc,
+			     int index) {
     const GPB::Reflection* ref = message->GetReflection();
 
     switch (field_desc->type()) {
@@ -33,14 +33,13 @@
         default:
             Rcpp_error("cannot cast to int");
     }
-    VOID_END_RCPP
-    return 0;  // -Wall
+    return 0;  // Unreachable for -Wall
 }
 
 /* this is only to be called for repeated fields */
-double MESSAGE_GET_REPEATED_DOUBLE(GPB::Message* message, GPB::FieldDescriptor* field_desc,
+double MESSAGE_GET_REPEATED_DOUBLE(const GPB::Message* message,
+				   const GPB::FieldDescriptor* field_desc,
                                    int index) {
-    BEGIN_RCPP
     const GPB::Reflection* ref = message->GetReflection();
 
     switch (field_desc->type()) {
@@ -51,7 +50,6 @@
         default:
             Rcpp_error("cannot cast to double");
     }
-    VOID_END_RCPP
     return 0;  // -Wall
 }
 
@@ -133,6 +131,7 @@
     int file = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666);
 
     /* using partial to allow partially filled messages */
+    // TODO(mstokely): Check return value and throw Rcpp::stop if 0?
     message->SerializePartialToFileDescriptor(file);
 
     close(file);
@@ -427,7 +426,7 @@
     return (res);
 }
 
-bool identical_messages_(GPB::Message* m1, GPB::Message* m2, double tol) {
+bool identical_messages_(const GPB::Message* m1, const GPB::Message* m2, double tol) {
     const GPB::Descriptor* d1 = m1->GetDescriptor();
     const GPB::Descriptor* d2 = m2->GetDescriptor();
 
@@ -535,7 +534,7 @@
                     for (int j = 0; j < fs; j++) {
                         const GPB::Message* mm1 = &ref->GetRepeatedMessage(*m1, field_desc, j);
                         const GPB::Message* mm2 = &ref->GetRepeatedMessage(*m2, field_desc, j);
-                        if (!identical_messages_((GPB::Message*)mm1, (GPB::Message*)mm2, tol)) {
+                        if (!identical_messages_(mm1, mm2, tol)) {
                             return false;
                         }
                     }
@@ -604,7 +603,7 @@
                 case TYPE_GROUP: {
                     const GPB::Message* mm1 = &ref->GetMessage(*m1, field_desc);
                     const GPB::Message* mm2 = &ref->GetMessage(*m2, field_desc);
-                    if (!identical_messages_((GPB::Message*)mm1, (GPB::Message*)mm2, tol)) {
+                    if (!identical_messages_(mm1, mm2, tol)) {
                         return false;
                     }
                     break;
@@ -618,12 +617,13 @@
     return true;
 }
 
-RPB_FUNCTION_2(bool, identical_messages, Rcpp::XPtr<GPB::Message> m1, Rcpp::XPtr<GPB::Message> m2) {
+RPB_FUNCTION_2(bool, identical_messages, Rcpp::XPtr<const GPB::Message> m1,
+	       Rcpp::XPtr<const GPB::Message> m2) {
     return identical_messages_(m1, m2, 0.0);
 }
 
-RPB_FUNCTION_3(bool, all_equal_messages, Rcpp::XPtr<GPB::Message> m1, Rcpp::XPtr<GPB::Message> m2,
-               double tol) {
+RPB_FUNCTION_3(bool, all_equal_messages, Rcpp::XPtr<const GPB::Message> m1,
+	       Rcpp::XPtr<const GPB::Message> m2, double tol) {
     return identical_messages_(m1, m2, tol);
 }
 
@@ -723,6 +723,7 @@
                         for (int i = 0; i < value_size; i++) {
                             ref->AddInt64(message, field_desc, GET_int64(values, i));
                         }
+			break;
                     default:
                         Rcpp::stop("Cannot convert to int64");
                 }
@@ -739,7 +740,7 @@
                     case LGLSXP:
                     case RAWSXP: {
                         for (int i = 0; i < value_size; i++) {
-                            ref->AddUInt32(message, field_desc, GET_int32(values, i));
+                            ref->AddUInt32(message, field_desc, GET_uint32(values, i));
                         }
                         break;
                     }
@@ -994,7 +995,7 @@
         default:
             throw std::range_error("unknown type");
     }
-    return R_NilValue;  // -Wall
+    return R_NilValue;  // Unreachable.  For -Wall
 }
 
 /**
@@ -1110,6 +1111,7 @@
                 default:
                     throw std::range_error("impossible to convert to a enum");
             }
+	    break;
         }
         case TYPE_MESSAGE:
         case TYPE_GROUP: {



More information about the Rprotobuf-commits mailing list