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

noreply at r-forge.r-project.org noreply at r-forge.r-project.org
Fri Dec 27 09:27:34 CET 2013


Author: murray
Date: 2013-12-27 09:27:34 +0100 (Fri, 27 Dec 2013)
New Revision: 604

Modified:
   pkg/.Rbuildignore
   pkg/ChangeLog
   pkg/TODO
   pkg/inst/unitTests/runit.bool.R
   pkg/src/mutators.cpp
Log:
Refactor setMessageField into four separate functions, correct a few
minor typos, and wrap everything in a try/catch block so we catch any
exceptions generated by Rcpp::as or other functions and forward it
along to an R-language stop() error instead of terminating our R
instance.



Modified: pkg/.Rbuildignore
===================================================================
--- pkg/.Rbuildignore	2013-12-27 03:08:36 UTC (rev 603)
+++ pkg/.Rbuildignore	2013-12-27 08:27:34 UTC (rev 604)
@@ -1,2 +1,3 @@
 m4
 configure.in
+vignettes/Sweave.sty

Modified: pkg/ChangeLog
===================================================================
--- pkg/ChangeLog	2013-12-27 03:08:36 UTC (rev 603)
+++ pkg/ChangeLog	2013-12-27 08:27:34 UTC (rev 604)
@@ -1,3 +1,11 @@
+2013-12-27  Murray Stokely  <murray at hilbert.local>
+
+	* src/mutators.cpp: Refactor setMessageField into four separate
+	  functions, correct a few minor typos, and wrap everything in a
+	  try/catch block so we catch any exceptions generated by Rcpp::as
+	  or other functions and forward it along to an R-language stop()
+	  error instead of terminating our R instance.
+
 2013-12-26  Murray Stokely  <mstokely at google.com>
 
 	* src/mutators.cpp: Support setting int32 values with character

Modified: pkg/TODO
===================================================================
--- pkg/TODO	2013-12-27 03:08:36 UTC (rev 603)
+++ pkg/TODO	2013-12-27 08:27:34 UTC (rev 604)
@@ -1,47 +1,43 @@
 Current TODO list:
 
-1. Finish improved vignette / R Journal writeup.
+o Finish improved vignette / R Journal writeup.
 
-2. Refactor massive 800+ line setMessageField function in
-   src/mutators.cpp into more modular, testable, easier to read
-   subroutines.
+o Push some type coercion hacks done in RProtoBuf upstream to Rcpp
+  (Rcpp:as<int> types should work on character strings representing
+  numbers, especially for int64s since we don't otherwise have a way
+  to represent 64-bit integers in base R).
 
-3. Push some type coercion hacks done in RProtoBuf upstream to Rcpp
-   (Rcpp:as<int> types should work on character strings representing
-   numbers, especially for int64s since we don't otherwise have a way
-   to represent 64-bit integers in base R).
+o Add more packages that depend on or enhance RProtoBuf.
 
-4. Add more packages that depend on or enhance RProtoBuf.
-
-5. Improve exception handling to prevent cases where an Rcpp exception
+o Improve exception handling to prevent cases where an Rcpp exception
    crashes the interactive R instance.
 
-6. Investigate Rcpp Modules support for some classes.  Murray is not
-   personally super enthusiastic about this one, as I think it may
-   require non trivial changes to Rcpp and/or result in losing some of
-   the user-friendliness we've crafted in the explicit RcppExported
-   function entry points.  Still, it could be explored and may result
-   in significantly fewer lines of code if successful.
+o Investigate Rcpp Modules support for some classes.  Murray is not
+  personally super enthusiastic about this one, as I think it may
+  require non trivial changes to Rcpp and/or result in losing some of
+  the user-friendliness we've crafted in the explicit RcppExported
+  function entry points.  Still, it could be explored and may result
+  in significantly fewer lines of code if successful.
 
-7. Add a FAQ and other documentation / examples.
+o Add a FAQ and other documentation / examples.
 
-8. Add more unit tests.
+o Add more unit tests.
 
-9. Explore removing the CLONE() in extractors.cpp.  This makes for
-   unusual semantics for any mutable methods of sub-messages.  For
-   example, clear(), setExtension(), and probably also update() ( but
-   "$<-" on sub-messages is not a problem, it seems).  More details below.
+o Explore removing the CLONE() in extractors.cpp.  This makes for
+  unusual semantics for any mutable methods of sub-messages.  For
+  example, clear(), setExtension(), and probably also update() ( but
+  "$<-" on sub-messages is not a problem, it seems).  More details below.
 
-10. What should we do when we unload the package?  Any additional
-   resources to free that is not currently done?
+o What should we do when we unload the package?  Any additional
+  resources to free that is not currently done?
 
-11. finalizers [murray: what is needed here?]
+o finalizers [murray: what is needed here?]
 
-12. Clean up formatting / whitespace (its awful, run it all through
-    clang-format?)
+o Clean up formatting / whitespace (its awful, run it all through
+  clang-format?)
 
-13. Thoroughly audit extensions support and other deeply nested types
-   support to ensure everything is implemented as expected.
+o Thoroughly audit extensions support and other deeply nested types
+  support to ensure everything is implemented as expected.
 
 Stuff I think belongs in additional packages that depend on RProtoBuf,
 rather than inside RProtoBuf directly:

Modified: pkg/inst/unitTests/runit.bool.R
===================================================================
--- pkg/inst/unitTests/runit.bool.R	2013-12-27 03:08:36 UTC (rev 603)
+++ pkg/inst/unitTests/runit.bool.R	2013-12-27 08:27:34 UTC (rev 604)
@@ -34,13 +34,12 @@
     checkEquals(length(unique(a$repeated_bool)), 2)
 
     # Verify we can't set any garbage string to a bool.
-    # TODO(mstokely): Fix and uncomment these.
-#    checkException(a$optional_bool <- "100")
-#    checkException(a$optional_bool <- "invalid")
+    checkException(a$optional_bool <- "100")
+    checkException(a$optional_bool <- "invalid")
 
     # Verify we can't set any garbage string to a repeated bool.
-#    checkException(a$repeated_bool <-c("invalid", "invalid"))
-#    checkException(a$repeated_bool <-c("33-"))
+    checkException(a$repeated_bool <-c("invalid", "invalid"))
+    checkException(a$repeated_bool <-c("33-"))
 
     # Verify we can set NA
     checkException(a$repeated_bool <- c(TRUE, FALSE, TRUE, NA))

Modified: pkg/src/mutators.cpp
===================================================================
--- pkg/src/mutators.cpp	2013-12-27 03:08:36 UTC (rev 603)
+++ pkg/src/mutators.cpp	2013-12-27 08:27:34 UTC (rev 604)
@@ -382,8 +382,856 @@
 	
 }
 
+/**
+ * check that all repeated values are ok before doing anything,
+ * otherwise this could lead to modifying a few values then failing in
+ * an inconsistent state.
+ */
+void CHECK_repeated_vals(GPB::FieldDescriptor* field_desc,
+						 SEXP value, int value_size) {
+	switch( field_desc->type() ){
+	case TYPE_MESSAGE:
+	case TYPE_GROUP:
+		{
+			switch( TYPEOF( value ) ){
+			case VECSXP :
+				{
+					/* check that it is a list of Messages of the appropriate type */
+					for( int i=0; i<value_size; i++){
+						if( !isMessage( VECTOR_ELT(value, i), field_desc->message_type()->full_name().c_str() ) ){
+							/* TODO: include i, target type and actual type in the message */
+							throwException( "incorrect type", "IncorrectMessageTypeException" ) ;
+						}
+					}
+					break ;
+				}
+			case S4SXP: 
+				{
+					/* check that this is a message of the appropriate type */
+					if( !isMessage( value, field_desc->message_type()->full_name().c_str() ) ){
+						throwException( "incorrect type", "IncorrectMessageTypeException" ) ;
+					}
+					break ;
+				}
+			default:
+				{
+					throwException( "impossible to convert to a message" , "ConversionException" ) ; 
+				}
+			}
+			break ;
+		}
+	case TYPE_ENUM : 
+		{
+			const GPB::EnumDescriptor* enum_desc = field_desc->enum_type() ;
 
+			/* check first, it means we have to loop twice, but
+			   otherwise this could have some side effects before the
+			   exception is thrown */
+    				   
+			/* FIXME: the checking should go before the resizing */   
+    				
+			switch( TYPEOF( value ) ){
+				// {{{ numbers 
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:
+				{
+					int nenums = enum_desc->value_count() ;
+					std::vector<int> possibles(nenums) ;
+					for( int i=0; i< nenums; i++){
+						possibles[i] = enum_desc->value(i)->number(); 
+					}
+    							
+					/* loop around the numbers to see if they are in the possibles */
+					for( int i=0; i<value_size; i++){
+						int val = GET_int(value, i ); 
+						int ok = 0; 
+						for( int j=0; j<nenums; j++){
+							if( val == possibles[j] ){
+								ok = 1; 
+								break ; 
+							}
+						}
+						if( !ok ){
+							throwException( "wrong value for enum", "WrongEnumValueException" ) ; 
+						}
+					}
+					break ;
+				}
+				// }}}
+    					
+				// {{{ STRSXP
+			case STRSXP:
+				{
+					int nenums = enum_desc->value_count() ;
+					std::vector<std::string> possibles(nenums) ;
+					for( int i=0; i< nenums; i++){
+						possibles[i] = enum_desc->value(i)->name() ; 
+					}
+    							
+					/* loop around the numbers to see if they are in the possibles */
+					for( int i=0; i<value_size; i++){
+						const char* val = CHAR( STRING_ELT(value, i )) ; 
+						int ok = 0; 
+						/* FIXME: there is probably something more efficient */
+						for( int j=0; j<nenums; j++){
+							if( !possibles[j].compare(val) ){
+								ok = 1; 
+								break ; 
+							}
+						}
+						if( !ok ){
+							throwException( "wrong value for enum", "WrongEnumValueException" ) ; 
+						}
+					}
+     							
+					break ;
+				}
+				// }}}
+    					
+			default:
+				throwException( "impossible to convert to a enum" , "ConversionException" ) ; 
+			}
+			break ;
+		}
+	case TYPE_DOUBLE : 
+	case TYPE_FLOAT : 
+	case TYPE_INT64 : 
+	case TYPE_UINT64 : 
+	case TYPE_INT32 : 
+	case TYPE_FIXED64 : 
+	case TYPE_FIXED32 : 
+	case TYPE_BOOL : 
+	case TYPE_STRING : 
+	case TYPE_BYTES : 
+	case TYPE_UINT32 : 
+	case TYPE_SFIXED32 : 
+	case TYPE_SFIXED64 : 
+	case TYPE_SINT32 : 
+	case TYPE_SINT64 : 
+		{
+			; // nothing, just to satisfy -Wall 
+		}
+	}
+	// }}}
+}
+
 /**
+ * set a non-repeated message field to a new value
+ *
+ * @param message pointer to a message
+ * @param ref pointer to reflection object for message
+ * @param field_desc pointer to field descriptor to update
+ * @param value new value for the field
+ * @param value_size size of the new value for the field
+ *
+ * @return void, the message is modified by reference
+ */
+void setNonRepeatedMessageField(GPB::Message* message,
+								const Reflection* ref,
+								GPB::FieldDescriptor* field_desc,
+								SEXP value, int value_size) {
+	if (value_size > 1) {
+		throwException( "cannot set non-repeated field to vector of length > 1",
+						"CastException" ) ;
+	}
+	switch( GPB::FieldDescriptor::TypeToCppType( field_desc->type() ) ){
+		// {{{ simple cases using macro expansion
+#undef HANDLE_SINGLE_FIELD
+#define HANDLE_SINGLE_FIELD( CPPTYPE, CAMEL, TYPE )						\
+		case CPPTYPE :													\
+			{															\
+				ref->Set##CAMEL( message, field_desc, Rcpp::as<TYPE>(value) ) ;	\
+				break ;													\
+			}
+
+		HANDLE_SINGLE_FIELD( CPPTYPE_DOUBLE, Double, double) ;
+		HANDLE_SINGLE_FIELD( CPPTYPE_FLOAT, Float, float) ;
+	case CPPTYPE_BOOL :
+		{
+			// TODO(mstokely): Rcpp should handle this!
+			if ((TYPEOF(value) == LGLSXP) &&
+				(LOGICAL(value)[0] == NA_LOGICAL)) {
+				throwException("NA boolean values can not be stored in "
+							   "bool protocol buffer fields",
+							   "CastException");
+			} else if ((TYPEOF(value) == INTSXP) &&
+					   (INTEGER(value)[0] == R_NaInt)) {
+				throwException( "NA boolean values can not be stored in "
+								"bool protocol buffer fields",
+								"CastException");
+			} else if ((TYPEOF(value) == REALSXP) &&
+					   (REAL(value)[0] == R_NaReal)) {
+				throwException( "NA boolean values can not be stored in "
+								"bool protocol buffer fields",
+								"CastException");
+			}
+			ref->SetBool(message, field_desc, Rcpp::as<bool>(value));
+			break ;
+		}
+	case CPPTYPE_INT32 :
+		{
+			if (TYPEOF(value) == STRSXP) {
+				const string int32str = COPYSTRING(CHAR(
+														STRING_ELT(value, 0)));
+				ref->SetInt32(message, field_desc,
+							  Int32FromString<GPB::int32>(int32str));
+				break ;
+			} else {
+				ref->SetInt32( message, field_desc, Rcpp::as<GPB::int32>(value));
+				break;
+			}
+		}
+	case CPPTYPE_UINT32 :
+		{
+			if (TYPEOF(value) == STRSXP) {
+				const string uint32str = COPYSTRING(CHAR(
+														 STRING_ELT(value, 0)));
+				ref->SetUInt32(message, field_desc,
+							   Int32FromString<GPB::uint32>(uint32str));
+				break ;
+			} else {
+				ref->SetUInt32( message, field_desc, Rcpp::as<GPB::uint32>(value));
+				break;
+			}
+		}
+
+#ifdef RCPP_HAS_LONG_LONG_TYPES
+	case CPPTYPE_INT64 :
+		{
+			// TODO(mstokely) Rcpp::as<int64> of a STRSEXP should just
+			// work for strings representing int64s.
+			if (TYPEOF(value) == STRSXP) {
+				const string int64str = COPYSTRING(CHAR(
+														STRING_ELT(value, 0)));
+				ref->SetInt64(message, field_desc,
+							  Int64FromString<GPB::int64>(int64str));
+				break ;
+			} else {
+				ref->SetInt64( message, field_desc, Rcpp::as<GPB::int64>(value));
+				break;
+			}
+		}
+	case CPPTYPE_UINT64 :
+		{
+			// TODO(mstokely) Rcpp::as<int64> of a STRSEXP should just
+			// work for strings representing int64s.
+			if (TYPEOF(value) == STRSXP) {
+				const string int64str = COPYSTRING(CHAR(
+														STRING_ELT(value, 0)));
+				ref->SetUInt64(message, field_desc,
+							   Int64FromString<GPB::uint64>(int64str));
+				break ;
+			} else {
+				ref->SetUInt64(message, field_desc,
+							   Rcpp::as<GPB::uint64>(value));
+				break;
+			}
+		}
+#endif
+#undef HANDLE_SINGLE_FIELD
+	default:
+		throwException("Unsupported type", "ConversionException");
+// }}}  
+     		
+		// {{{ string
+	case CPPTYPE_STRING:
+		{
+			switch( TYPEOF( value ) ){
+			case STRSXP:
+				{
+					ref->SetString(message, field_desc,
+								   COPYSTRING( CHAR(STRING_ELT(value,0 )) ) ) ;
+					break ;
+				}
+			case RAWSXP:
+				{
+					ref->SetString(message, field_desc,  GET_bytes(value, 0)) ;
+					break ;
+				}
+			case S4SXP:
+				{
+					/* check if value is a message */
+					if( !Rf_inherits( value, "Message" ) ){
+						throwException( "Can only convert S4 objects of class 'Message'",
+										"ConversionException" ) ;
+					}
+					GPB::Message* __mess = GET_MESSAGE_POINTER_FROM_S4(value);
+					ref->SetString(message, field_desc,
+								   __mess->SerializeAsString());
+					break ;
+				}
+			default: 
+				{
+					throwException( "Cannot convert to string",
+									"ConversionException" ) ;
+				}
+			}
+			break ; 
+		}
+		// }}}
+    		
+		// {{{ message
+	case CPPTYPE_MESSAGE:
+		{
+			if( TYPEOF( value ) == S4SXP && Rf_inherits( value, "Message" ) ){
+				GPB::Message* mess = (GPB::Message*) EXTPTR_PTR( GET_SLOT( value, Rf_install("pointer") ) ) ;
+				const char* type = mess->GetDescriptor()->full_name().c_str() ;
+				const char* target = field_desc->message_type()->full_name().c_str() ; 
+				if( strcmp( type, target ) ){
+					throwException( "wrong message type", "WrongMessageException" ) ;
+				}
+				GPB::Message* m = ref->MutableMessage( message, field_desc ) ; 
+				m->CopyFrom( *mess ) ;
+			} else {
+				throwException( "type mismatch, expecting a 'Message' object",
+								"TypeMismatchException" ) ;
+			}
+			break ;
+		}
+		// }}}
+    		
+		// {{{ enum
+	case CPPTYPE_ENUM : 
+		{
+			const GPB::EnumDescriptor* enum_desc = field_desc->enum_type() ;
+			switch( TYPEOF( value ) ){
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:
+				{
+					int val = Rcpp::as<int>(value) ;
+					const GPB::EnumValueDescriptor* evd = enum_desc->FindValueByNumber(val) ;
+					if( !evd ){
+						throwException( "wrong value for enum",
+										"WrongEnumValueException" ) ; 
+					} else {
+						ref->SetEnum( message, field_desc, evd ); 
+					}
+					break ;
+				}
+			case STRSXP:
+				{
+					std::string val = Rcpp::as<std::string>( value ) ;
+					const GPB::EnumValueDescriptor* evd = enum_desc->FindValueByName(val) ;
+					if( !evd ){
+						throwException( "wrong value for enum", "WrongEnumValueException" ) ; 
+					} else {
+						ref->SetEnum( message, field_desc, evd ); 
+					}
+					break ;
+				}
+			default: 
+				{
+					throwException( "cannot set enum value", "WrongTypeEnumValueException" ) ; 
+				}
+			}
+		}
+		// }}}
+	}
+	// }}}
+}
+
+/**
+ * set a repeated message field to a new value
+ *
+ * @param message pointer to a message
+ * @param ref pointer to reflection object for message
+ * @param field_desc pointer to field descriptor to update
+ * @param value new value for the field
+ * @param value_size size of the new value for the field
+ *
+ * @return void, the message is modified by reference
+ */
+
+void setRepeatedMessageField(GPB::Message* message,
+							 const Reflection* ref,
+							 GPB::FieldDescriptor* field_desc,
+							 SEXP value, int value_size) {
+	// The number of elements already in the repeated field.
+	int field_size = ref->FieldSize( *message, field_desc ) ;
+		
+	/* {{{ in case of messages or enum, we have to check that all
+	   values are ok before doing anything, othewise this could leed
+	   to modify a few values and then fail which is not good */
+	CHECK_repeated_vals(field_desc, value, value_size);
+		  
+	/* Remove some items if there are too many. */
+	if( field_size > value_size ) {
+		/* we need to remove some */
+		while( field_size > value_size ){
+			ref->RemoveLast( message, field_desc ) ;
+			field_size-- ;
+		}
+	}
+		
+	switch( field_desc->type() ){
+		// {{{ int32
+	case TYPE_INT32:
+	case TYPE_SINT32:
+	case TYPE_SFIXED32:
+		{
+			switch( TYPEOF( value ) ){
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:	
+			case STRSXP: // For int32, we support chars.
+				{
+					int i = 0;
+					/* in any case, fill the values up to field_size */
+					for( ; i<field_size; i++){
+						ref->SetRepeatedInt32( message, field_desc, i,
+											   GET_int32(value,i) ) ;
+					}
+   								
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							ref->AddInt32( message, field_desc,
+										   GET_int32(value,i) ) ;
+						}
+					}
+    							
+					break ;
+				}
+
+			default: 
+				{
+					throwException( "Cannot convert to int32",
+									"ConversionException" ) ; 
+				}
+			}
+			break ;   
+		}
+		// }}}
+    		
+		// {{{ int64
+	case TYPE_INT64:
+	case TYPE_SINT64:
+	case TYPE_SFIXED64:
+		{
+			switch( TYPEOF( value ) ){
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:
+			case STRSXP: // For int64, we support chars.
+				{
+					int i = 0;
+
+					/* in any case, fill the values up to field_size */
+					for( ; i<field_size; i++){
+						ref->SetRepeatedInt64( message, field_desc, i,
+											   GET_int64(value,i) ) ;
+					}
+	    						
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							ref->AddInt64( message, field_desc,
+										   GET_int64(value,i) ) ;
+						}
+					}
+					break ;
+				}
+
+			default: 
+				throwException( "Cannot convert to int64",
+								"ConversionException" ) ; 
+			}
+			break ;
+		}
+		// }}}	
+    			
+		// {{{ uint32
+	case TYPE_UINT32:
+	case TYPE_FIXED32:
+		{
+			switch( TYPEOF( value ) ){
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:	
+			case STRSXP: // For int32, we support chars.
+				{
+					int i = 0;
+					/* in any case, fill the values up to field_size */
+					for( ; i<field_size; i++){
+						ref->SetRepeatedUInt32( message, field_desc, i,
+												GET_int32(value,i) ) ;
+					}
+	    						
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							ref->AddUInt32( message, field_desc,
+											GET_int32(value,i) ) ;
+						}
+					}
+					break ;
+				}
+			default: 
+				throwException( "Cannot convert to uint32",
+								"ConversionException" ) ; 
+			}
+			break ;   
+		}
+		// }}}
+     		
+		// {{{ uint64
+	case TYPE_UINT64:
+	case TYPE_FIXED64:
+		{
+			switch( TYPEOF( value ) ){
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:
+			case STRSXP: // For int64, we support chars.
+				{
+					int i = 0;
+					/* in any case, fill the values up to field_size */
+					for( ; i<field_size; i++){
+						ref->SetRepeatedUInt64( message, field_desc, i,
+												GET_uint64(value,i) ) ;
+					}
+	    						
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							ref->AddUInt64( message, field_desc,
+											GET_uint64(value,i) ) ;
+						}
+					}
+					break ;
+				}
+			default: 
+				throwException( "Cannot convert to int64",
+								"ConversionException" ) ; 
+			}
+			break ;   
+		}
+		// }}}
+  
+		// {{{ double
+	case TYPE_DOUBLE:
+		{
+			switch( TYPEOF( value ) ){
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:	
+				{
+					int i = 0;
+					/* in any case, fill the values up to field_size */
+					for( ; i<field_size; i++){
+						ref->SetRepeatedDouble( message, field_desc, i,
+												GET_double(value,i) ) ;
+					}
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							ref->AddDouble( message, field_desc,
+											GET_double(value,i) ) ;
+						}
+					}
+					break ;
+				}
+			default: 
+				throwException( "Cannot convert to double",
+								"ConversionException" ) ; 
+			}
+			break ;   
+		}
+		// }}}	
+    			
+		// {{{ float
+	case TYPE_FLOAT:
+		{
+			switch( TYPEOF( value ) ){
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:	
+				{
+					int i = 0;
+					/* in any case, fill the values up to field_size */
+					for( ; i<field_size; i++){
+						ref->SetRepeatedFloat( message, field_desc, i,
+											   GET_float(value,i) ) ;
+					}
+	    						
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							ref->AddFloat( message, field_desc,
+										   GET_float(value,i) ) ;
+						}
+					}
+					break ;
+				}
+			default: 
+				throwException( "Cannot convert to float",
+								"ConversionException" ) ; 
+			}
+			break ;
+		}
+		// }}}
+    		
+		// {{{ bool
+	case TYPE_BOOL:
+		{
+			switch( TYPEOF( value ) ){
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:	
+				{
+					int i = 0;
+					/* in any case, fill the values up to field_size */
+					for( ; i<field_size; i++){
+						ref->SetRepeatedBool( message, field_desc, i,
+											  GET_bool(value,i) ) ;
+					}
+	    						
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							ref->AddBool( message, field_desc,
+										  GET_bool(value,i) ) ;
+						}
+					}
+					break ;
+				}
+			default: 
+				throwException( "Cannot convert to bool",
+								"ConversionException" ) ; 
+			}
+			break ;   
+		}
+		// }}}
+			
+		// {{{ string
+	case TYPE_STRING:
+	case TYPE_BYTES:
+		{
+			switch( TYPEOF(value) ){
+			case STRSXP :
+				{ 
+					/* in any case, fill the values up to field_size */
+					int i = 0;
+					for( ; i<field_size; i++){
+						ref->SetRepeatedString( message, field_desc, i,
+												COPYSTRING( CHAR(STRING_ELT(value,i )) ) ) ;
+					}
+			    				
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							ref->AddString( message, field_desc,
+											COPYSTRING( CHAR(STRING_ELT(value,i )) ) ) ;
+						}
+					}
+					break ;
+				} 
+			case RAWSXP:
+				{
+					/* in any case, fill the values up to field_size */
+					int i = 0;
+					for ( ; i<field_size; i++) {
+						ref->SetRepeatedString( message, field_desc, i,
+												GET_bytes(value, 0)) ;
+					}
+								
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++) {
+							ref->AddString( message, field_desc,
+											GET_bytes(value, 0)) ;
+						}
+					}
+					break;
+				}
+			case S4SXP:
+				{
+					/* check if value is a message */
+					if( !Rf_inherits( value, "Message" ) ){
+						throwException( "Can only convert S4 objects of class 'Message' ",
+										"ConversionException" ) ;
+					}
+					GPB::Message* __mess = GET_MESSAGE_POINTER_FROM_S4( value ) ;
+					ref->SetRepeatedString(message, field_desc, 0,
+										   __mess->SerializeAsString() ) ;
+					break ;
+				}
+			case VECSXP:
+				{
+					// we know it is a list of messages or raws because it 
+					// has been tested above
+					if (LENGTH(value) > 0 && TYPEOF(VECTOR_ELT(value, 0)) == RAWSXP ) {
+						/* in any case, fill the values up to field_size */
+						int i = 0;
+						for( ; i<field_size; i++) {
+							ref->SetRepeatedString( message, field_desc, i,
+													GET_bytes(value,i )) ;
+						}
+									
+						/* then add some if needed */
+						if( value_size > field_size ) {
+							for( ; i<value_size; i++){
+								ref->AddString( message, field_desc,
+												GET_bytes(value,i )) ;
+							}
+						}
+					} else {
+						// FIXME: we should probably use SerializeToString 
+						//        as indicated in the protobuf api documentation
+						//        http://code.google.com/apis/protocolbuffers/docs/reference/cpp/google.protobuf.message_lite.html#MessageLite.SerializeAsString.details
+						GPB::Message* __mess ;
+									
+						/* in any case, fill the values up to field_size */
+						int i = 0;
+						for( ; i<field_size; i++){
+							__mess = GET_MESSAGE_POINTER_FROM_S4( VECTOR_ELT( value, i ) ); 
+							ref->SetRepeatedString( message, field_desc, i,
+													__mess->SerializeAsString() ) ;
+						}
+						/* then add some if needed */
+						if( value_size > field_size ){
+							for( ; i<value_size; i++){
+								__mess = GET_MESSAGE_POINTER_FROM_S4( VECTOR_ELT( value, i ) ); 
+								ref->AddString( message, field_desc,
+												__mess->SerializeAsString() ) ;
+							}
+						}
+					}
+					break ;
+				}
+			default: 
+				throwException( "Cannot convert to string",
+								"ConversionException" ) ;
+			}
+			break ; 
+		}
+		// }}}
+    		
+		// {{{ message
+	case TYPE_MESSAGE:
+	case TYPE_GROUP: 
+		{    
+			if( TYPEOF( value ) == S4SXP ) { 
+				/* we know it is a message of the correct type (tested above) */
+				GPB::Message* mess = GET_MESSAGE_POINTER_FROM_S4( value ) ; 
+    					
+				if( field_size == 1 ) {
+					/* FIXME: we should not __copy__ the message */ 
+					ref->MutableRepeatedMessage(message, field_desc, 0 )->CopyFrom( *mess ) ;
+				} else {
+					/* FIXME */
+					ref->ClearField( message, field_desc ); 
+    						
+					/* FIXME: we should not __copy__ the message */
+					ref->AddMessage( message, field_desc )->CopyFrom( *mess ) ; 
+				}
+			} else if( TYPEOF(value) == VECSXP )  {
+				/* in any case, fill the values up to field_size */
+				int i = 0;
+				for( ; i<field_size; i++){
+					GPB::Message* mess = GET_MESSAGE_POINTER_FROM_S4( VECTOR_ELT( value, i) ) ; 
+					/* we already know it is of the correct type because of the 
+					   premptive chjeck above */
+					ref->MutableRepeatedMessage(message, field_desc, i )->CopyFrom( *mess ) ;
+				}
+			    		
+				/* then add some if needed */
+				if( value_size > field_size ){
+					for( ; i<value_size; i++){
+						GPB::Message* mess = GET_MESSAGE_POINTER_FROM_S4( VECTOR_ELT( value, i) ) ; 
+						/* we already know it is of the correct type
+						   because of the premptive chjeck above */
+			    				
+						ref->AddMessage(message, field_desc)->CopyFrom( *mess ) ; 
+					}
+				}
+			} else{
+				throwException( "type mismatch, expecting a 'Message' object or a list of them", "TypeMismatchException" ) ;
+			}
+			break ;
+		}
+		// }}}
+    		
+		// {{{ enum
+	case TYPE_ENUM : 
+		{
+			const GPB::EnumDescriptor* enum_desc = field_desc->enum_type() ;
+
+			switch( TYPEOF( value ) ){
+				// {{{ numbers 
+			case INTSXP:
+			case REALSXP:
+			case LGLSXP:
+			case RAWSXP:
+				{
+					int i = 0;
+					/* in any case, fill the values up to field_size */
+					for( ; i<field_size; i++){
+						int val = GET_int(value, i ); 
+						ref->SetRepeatedEnum( message, field_desc, i,
+											  enum_desc->FindValueByNumber(val) ) ;
+					}
+	    						
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							int val = GET_int(value, i ); 
+							ref->AddEnum( message, field_desc,
+										  enum_desc->FindValueByNumber(val) ) ;
+						}
+					}
+					break ;
+				}
+				// }}}
+
+				// {{{ STRSXP
+			case STRSXP:
+				{
+					/* in any case, fill the values up to field_size */
+					int i = 0;
+					for( ; i<field_size; i++){
+						std::string val = CHAR( STRING_ELT( value, i) ) ;
+						const GPB::EnumValueDescriptor* evd = enum_desc->FindValueByName(val) ;
+						ref->SetRepeatedEnum( message, field_desc, i, evd ) ; 
+					}
+	    						
+					/* then add some if needed */
+					if( value_size > field_size ){
+						for( ; i<value_size; i++){
+							std::string val = CHAR( STRING_ELT( value, i) ) ;
+							const GPB::EnumValueDescriptor* evd = enum_desc->FindValueByName(val) ;
+							ref->AddEnum( message, field_desc, evd ) ; 
+						}
+					}
+					break ;
+				}
+				// }}}
+    						
+				// {{{ default
+			default: 
+				{
+					throwException( "cannot set enum value", "WrongTypeEnumValueException" ) ; 
+				}
+				// }}}
+			}
+		}
+		// }}}
+	}
+	// }}}
+}
+
+/**
  * set a message field to a new value
  *
  * @param pointer external pointer to a message
@@ -402,12 +1250,10 @@
 PRINT_DEBUG_INFO( "value", value ) ;
 #endif
 
+ try {
 	/* grab the Message pointer */
 	GPB::Message* message = GET_MESSAGE_POINTER_FROM_XP(pointer) ;
 
-	/* the message descriptor */
-	// const GPB::Descriptor* desc = message->GetDescriptor() ;
-	
 	/* check that we can get a file descriptor from name */
 	GPB::FieldDescriptor* field_desc = getFieldDescriptor( message, name ); 
 	
@@ -443,802 +1289,17 @@
 	// }}}
 
 	if( field_desc->is_repeated() ){
-		// {{{ repeated fields
-		
-		// The number of elements already in the repeated field.
-		int field_size = ref->FieldSize( *message, field_desc ) ;
-		
-		/* {{{ in case of messages or enum, we have to check that all values
-		  are ok before doing anything, othewise this could leed to modify a few values 
-		  and then fail which is not good */
-		
-		switch( field_desc->type() ){
-    		case TYPE_MESSAGE:
-    		case TYPE_GROUP:
-    			{
-    				switch( TYPEOF( value ) ){
-    					case VECSXP :
-    						{
-    							/* check that it is a list of Messages of the appropriate type */
-    							for( int i=0; i<value_size; i++){
-	    							if( !isMessage( VECTOR_ELT(value, i), field_desc->message_type()->full_name().c_str() ) ){
-	    								/* TODO: include i, target type and actual type in the message */
-	    								throwException( "incorrect type", "IncorrectMessageTypeException" ) ;
-	    							}
-	    						}
-	    						break ;
-	    					}
-	    				case S4SXP: 
-	    					{
-	    						/* check that this is a message of the appropriate type */
-	    						if( !isMessage( value, field_desc->message_type()->full_name().c_str() ) ){
-    								throwException( "incorrect type", "IncorrectMessageTypeException" ) ;
-    							}
-    							break ;
-    						}
-    					default:
-    						{
-    							throwException( "impossible to convert to a message" , "ConversionException" ) ; 
-    						}
-    				}
-    				break ;
-    			}
-    		case TYPE_ENUM : 
-    			{
-    				const GPB::EnumDescriptor* enum_desc = field_desc->enum_type() ;
-
-    				/* check first, it means we have to loop twice, but 
-    				   otherwise this could have some side effects before 
-    				   the exception is thrown */
-    				   
-    				/* FIXME: the checking should go before the resizing */   
-    				
-    				switch( TYPEOF( value ) ){
-    					// {{{ numbers 
-    					case INTSXP:
-    					case REALSXP:
-    					case LGLSXP:
-    					case RAWSXP:
-    						{
-    							int nenums = enum_desc->value_count() ;
-    							std::vector<int> possibles(nenums) ;
-    							for( int i=0; i< nenums; i++){
-    								possibles[i] = enum_desc->value(i)->number(); 
-    							}
-    							
-    							/* loop around the numbers to see if they are in the possibles */
-    							for( int i=0; i<value_size; i++){
-    								int val = GET_int(value, i ); 
-    								int ok = 0; 
-    								for( int j=0; j<nenums; j++){
-    									if( val == possibles[j] ){
-    										ok = 1; 
-    										break ; 
-    									}
-    								}
-    								if( !ok ){
-    									throwException( "wrong value for enum", "WrongEnumValueException" ) ; 
-    								}
-    							}
-    							
-    							break ;
-    						}
-    					// }}}
-    					
-    					// {{{ STRSXP
-    					case STRSXP:
-    						{
-    							int nenums = enum_desc->value_count() ;
-    							std::vector<std::string> possibles(nenums) ;
-    							for( int i=0; i< nenums; i++){
-    								possibles[i] = enum_desc->value(i)->name() ; 
-    							}
-    							
-    							/* loop around the numbers to see if they are in the possibles */
-    							for( int i=0; i<value_size; i++){
-    								const char* val = CHAR( STRING_ELT(value, i )) ; 
-    								int ok = 0; 
-    								/* FIXME: there is probably something more efficient */
-    								for( int j=0; j<nenums; j++){
-    									if( !possibles[j].compare(val) ){
-    										ok = 1; 
-    										break ; 
-    									}
-    								}
-    								if( !ok ){
-    									throwException( "wrong value for enum", "WrongEnumValueException" ) ; 
-    								}
-    							}
-     							
-    							break ;
-    						}
-    					// }}}
-    					
-    					default:
-    						throwException( "impossible to convert to a enum" , "ConversionException" ) ; 
-    				}
-    				break ;
-    			}
-    		case TYPE_DOUBLE : 
-    		case TYPE_FLOAT : 
-    		case TYPE_INT64 : 
-    		case TYPE_UINT64 : 
-    		case TYPE_INT32 : 
[TRUNCATED]

To get the complete diff run:
    svnlook diff /svnroot/rprotobuf -r 604


More information about the Rprotobuf-commits mailing list