1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
|
Current TODO list:
o Finish improved vignette / R Journal writeup.
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).
o Add more packages that depend on or enhance RProtoBuf.
o Improve exception handling to prevent cases where an Rcpp exception
crashes the interactive R instance.
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.
o Add a FAQ and other documentation / examples.
o Add more unit tests.
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.
o What should we do when we unload the package? Any additional
resources to free that is not currently done?
o finalizers [murray: what is needed here?]
o Clean up formatting / whitespace of R code. The C++ code is now
kept consistent through clang-format and a new emacs directory level
config in pkg/src, and the style guide notes in the STYLE file on
R-Forge.
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:
o http-powered rpc implementation
o More as.Message methods
--- Detailed Notes ----
8. CLONE()
--[ examples from Murray illustrating the problem ]--
library(RProtoBuf)
InitGoogle()
if (!exists("protobuf_unittest.TestAllTypes", "RProtoBuf:DescriptorPool")) {
unittest.proto.file <- system.file("unitTests", "data", "unittest.proto",package="RProtoBuf")
readProtoFiles(file=unittest.proto.file)
}
test <- new(protobuf_unittest.TestAllTypes)
test$optional_foreign_message <- new(protobuf_unittest.ForeignMessage, c=3)
# Example 1:
test$optional_foreign_message$c
test$optional_foreign_message$clear()
test$optional_foreign_message$c
# didn't clear test$optional_foreign_message$c
# Example 2:
foo <- new(protobuf_unittest.ForeignMessage, c=3)
foo$c
foo$clear()
foo$c
# did clear foo$c
# Example 3:
baz <- test$optional_foreign_message
baz$c
baz$c <- 4
test$optional_foreign_message$c
# still 3, but would be 4 if we removed the CLONE().
Example 1 is currently I think very confusing semantically for users of RProtoBuf with nested messages and I would like to fix that case. Example 2 works correctly now and would not be affected by this change. Example 3 would change behavior and could cause problems for users. Would need to be clearly announced in the NEWS file and to our user list.
--[ Romain's thoughts ]--
`$<-` is the task of setMessageField:
https://github.com/RProtoBuf/RProtoBuf/blob/master/R/00classes.R#L197
https://github.com/RProtoBuf/RProtoBuf/blob/master/src/mutators.cpp#L377
For "$" we could perhaps move to some sort of copy on change semantics (similar to what R does), instead of what we use currently which is more like copy on access.
|
