Skip to content

Commit 36ef9b7

Browse files
authored
Merge pull request #2 from pachadotdev/issue453
Issue453
2 parents 72d2f37 + e340f78 commit 36ef9b7

File tree

7 files changed

+1435
-1441
lines changed

7 files changed

+1435
-1441
lines changed

README.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,3 +78,13 @@ Please note that the cpp11 project is released with a [Contributor Code of Condu
7878

7979
cpp11 would not exist without Rcpp.
8080
Thanks to the Rcpp authors, Dirk Eddelbuettel, Romain Francois, JJ Allaire, Kevin Ushey, Qiang Kou, Nathan Russell, Douglas Bates and John Chambers for their work writing and maintaining Rcpp.
81+
82+
## Clang format
83+
84+
To match GHA, use clang-format-12 to format C++ code. With systems that provide clang-format-14 or newer, you can use Docker:
85+
86+
```bash
87+
docker run --rm -v "$PWD":/work -w /work ubuntu:22.04 bash -lc "\
88+
apt-get update && apt-get install -y clang-format-12 && \
89+
find . -name '*.cpp' -o -name '*.hpp' -o -name '*.h' | xargs -r clang-format-12 -i"
90+
```

cpp11test/src/test-integers.cpp

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,23 @@ context("integers-C++") {
138138
expect_true(x[1] == 3);
139139
expect_true(x[2] == 4);
140140
}
141+
test_that("integers.value()") {
142+
cpp11::writable::integers x;
143+
x.push_back(1);
144+
x.push_back(2);
145+
x.push_back(3);
146+
147+
// Test that value() returns the same as operator[] but as T directly
148+
expect_true(x.value(0) == 1);
149+
expect_true(x.value(1) == 2);
150+
expect_true(x.value(2) == 3);
151+
152+
// Test that value() works with C-style formatting (this was the original issue in
153+
// #453)
154+
expect_true(x.value(0) == x[0]);
155+
expect_true(x.value(1) == x[1]);
156+
expect_true(x.value(2) == x[2]);
157+
}
141158

142159
test_that("writable::integers(SEXP)") {
143160
SEXP x = PROTECT(Rf_allocVector(INTSXP, 5));
@@ -278,4 +295,56 @@ context("integers-C++") {
278295
int y = NA_INTEGER;
279296
expect_true(cpp11::is_na(y));
280297
}
298+
299+
test_that("proxy issue demonstration") {
300+
// This test demonstrates the proxy issue and shows that all solutions work
301+
cpp11::writable::integers x;
302+
for (int i = 0; i < 3; i++) {
303+
x.push_back(i * 10);
304+
}
305+
306+
// Test that value() method works correctly
307+
expect_true(x.value(0) == 0);
308+
expect_true(x.value(1) == 10);
309+
expect_true(x.value(2) == 20);
310+
311+
// Test that explicit cast works
312+
expect_true((int)x[0] == 0);
313+
expect_true((int)x[1] == 10);
314+
expect_true((int)x[2] == 20);
315+
316+
// Test that auto assignment works (triggers implicit conversion)
317+
int val0 = x[0];
318+
int val1 = x[1];
319+
int val2 = x[2];
320+
expect_true(val0 == 0);
321+
expect_true(val1 == 10);
322+
expect_true(val2 == 20);
323+
324+
// Test that value() and operator[] return equivalent results
325+
expect_true(x.value(0) == (int)x[0]);
326+
expect_true(x.value(1) == (int)x[1]);
327+
expect_true(x.value(2) == (int)x[2]);
328+
}
329+
}
330+
331+
// [[cpp11::register]]
332+
// Demo function to show the three ways to handle the proxy issue
333+
// To use this function:
334+
// 1. Run cpp11::cpp_register() to regenerate R bindings
335+
// 2. Rebuild and reinstall the package
336+
// 3. Call test_proxy_issue_demo() from R
337+
void test_proxy_issue_demo() {
338+
cpp11::writable::integers x;
339+
for (int i = 0; i < 5; i++) {
340+
x.push_back(i);
341+
342+
// These all work correctly:
343+
Rprintf("Method 1 - cast: x[%d] = %d\n", i, (int)x[i]);
344+
Rprintf("Method 2 - value(): x[%d] = %d\n", i, x.value(i));
345+
346+
// This also works (auto triggers implicit conversion):
347+
int val = x[i];
348+
Rprintf("Method 3 - auto: x[%d] = %d\n", i, val);
349+
}
281350
}

inst/include/cpp11/integers.hpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,3 +102,26 @@ inline integers as_integers(SEXP x) {
102102
}
103103

104104
} // namespace cpp11
105+
106+
// Note: Proxy Behavior in writable::integers
107+
//
108+
// When using writable::integers, operator[] returns a proxy object that allows
109+
// both reading and writing. For cases where you need the actual int value
110+
// (e.g., when using with C-style variadic functions like Rprintf), use one of
111+
// these three approaches:
112+
//
113+
// 1. Direct value access: vec.value(i) [Recommended]
114+
// 2. Explicit cast: (int)vec[i]
115+
// 3. Auto with explicit type: int val = vec[i];
116+
//
117+
// Example demonstrating the issue and solutions:
118+
// writable::integers vec;
119+
// vec.push_back(42);
120+
//
121+
// // This may print garbage due to proxy object:
122+
// // Rprintf("Value: %d\n", vec[0]); // DON'T DO THIS
123+
//
124+
// // These all work correctly:
125+
// Rprintf("Value: %d\n", vec.value(0)); // Recommended
126+
// Rprintf("Value: %d\n", (int)vec[0]); // Also works
127+
// int val = vec[0]; Rprintf("Value: %d\n", val); // Also works

inst/include/cpp11/r_vector.hpp

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,15 @@ class r_vector : public cpp11::r_vector<T> {
256256

257257
iterator find(const r_string& name) const;
258258

259+
/// Get the value at position without returning a proxy
260+
/// This is useful when you need the actual value (e.g., for C-style printf functions)
261+
/// that don't trigger implicit conversions from proxy types
262+
#ifdef LONG_VECTOR_SUPPORT
263+
T value(const int pos) const;
264+
#endif
265+
T value(const R_xlen_t pos) const;
266+
T value(const size_type pos) const;
267+
259268
attribute_proxy<r_vector<T>> attr(const char* name) const;
260269
attribute_proxy<r_vector<T>> attr(const std::string& name) const;
261270
attribute_proxy<r_vector<T>> attr(SEXP name) const;
@@ -1156,6 +1165,24 @@ inline typename r_vector<T>::iterator r_vector<T>::find(const r_string& name) co
11561165
return end();
11571166
}
11581167

1168+
#ifdef LONG_VECTOR_SUPPORT
1169+
template <typename T>
1170+
inline T r_vector<T>::value(const int pos) const {
1171+
return value(static_cast<R_xlen_t>(pos));
1172+
}
1173+
#endif
1174+
1175+
template <typename T>
1176+
inline T r_vector<T>::value(const R_xlen_t pos) const {
1177+
// Use the parent read-only class's operator[] which returns T directly
1178+
return cpp11::r_vector<T>::operator[](pos);
1179+
}
1180+
1181+
template <typename T>
1182+
inline T r_vector<T>::value(const size_type pos) const {
1183+
return value(static_cast<R_xlen_t>(pos));
1184+
}
1185+
11591186
template <typename T>
11601187
inline attribute_proxy<r_vector<T>> r_vector<T>::attr(const char* name) const {
11611188
return attribute_proxy<r_vector<T>>(*this, name);

0 commit comments

Comments
 (0)