[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