Skip to content

Commit 28538e5

Browse files
romainfrancoisromainfrancois
authored
Fix missing value propagation in as_integers/doubles(), take 2 (#319)
* Revert "Fix missing value propagation in `as_integers/doubles()` (#265)" This reverts commit 3c87798. * is_convertible_without_loss_to_integer * is_na using sfinae to differentiate double and !double * install local cpp11 before cpp11test * make format * propage na * integers test too * new bullet * as_doubles(SEXP) * test for NA_REAL first
1 parent 3c87798 commit 28538e5

File tree

10 files changed

+197
-213
lines changed

10 files changed

+197
-213
lines changed

‎.github/workflows/R-CMD-check.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ jobs:
8585
run: |
8686
options(warn = 2)
8787
pak::local_install_dev_deps("cpp11test", dependencies = TRUE)
88+
install.packages(".", repos = NULL, type = "source")
8889
install.packages("cpp11test", repos = NULL, INSTALL_opts = "--install-tests", type = "source")
8990
shell: Rscript {0}
9091

‎Makefile

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,8 @@ test: all
1010

1111
clean:
1212
@Rscript -e 'devtools::clean_dll()'
13+
@Rscript -e 'devtools::clean_dll("cpp11test")'
1314

1415
clang_format=`which clang-format`
1516
format: $(shell find . -name '*.hpp') $(shell find . -name '*.cpp')
16-
ifeq ($(findstring version 10,$(shell ${clang_format} --version 2>/dev/null)),)
17-
@echo "clang-format 10 is required"
18-
else
1917
@${clang_format} -i $?
20-
endif

‎NEWS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# cpp11 (development version)
22

33
* `as_doubles()` and `as_integers()` now propagate missing values correctly
4-
(#265).
4+
(#265, #319).
55

66
* Fixed a performance issue related to nested `unwind_protect()` calls (#298).
77

‎cpp11test/R/cpp11.R

Lines changed: 52 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -8,30 +8,6 @@ data_frame_ <- function() {
88
.Call(`_cpp11test_data_frame_`)
99
}
1010

11-
my_stop <- function(mystring, myarg) {
12-
invisible(.Call(`_cpp11test_my_stop`, mystring, myarg))
13-
}
14-
15-
my_stop_n1 <- function(mystring) {
16-
invisible(.Call(`_cpp11test_my_stop_n1`, mystring))
17-
}
18-
19-
my_warning <- function(mystring, myarg) {
20-
invisible(.Call(`_cpp11test_my_warning`, mystring, myarg))
21-
}
22-
23-
my_warning_n1 <- function(mystring) {
24-
invisible(.Call(`_cpp11test_my_warning_n1`, mystring))
25-
}
26-
27-
my_message <- function(mystring, myarg) {
28-
invisible(.Call(`_cpp11test_my_message`, mystring, myarg))
29-
}
30-
31-
my_message_n1 <- function(mystring) {
32-
invisible(.Call(`_cpp11test_my_message_n1`, mystring))
33-
}
34-
3511
my_stop_n1fmt <- function(mystring) {
3612
invisible(.Call(`_cpp11test_my_stop_n1fmt`, mystring))
3713
}
@@ -56,6 +32,30 @@ my_message_n2fmt <- function(mystring, myarg) {
5632
invisible(.Call(`_cpp11test_my_message_n2fmt`, mystring, myarg))
5733
}
5834

35+
my_stop <- function(mystring, myarg) {
36+
invisible(.Call(`_cpp11test_my_stop`, mystring, myarg))
37+
}
38+
39+
my_stop_n1 <- function(mystring) {
40+
invisible(.Call(`_cpp11test_my_stop_n1`, mystring))
41+
}
42+
43+
my_warning <- function(mystring, myarg) {
44+
invisible(.Call(`_cpp11test_my_warning`, mystring, myarg))
45+
}
46+
47+
my_warning_n1 <- function(mystring) {
48+
invisible(.Call(`_cpp11test_my_warning_n1`, mystring))
49+
}
50+
51+
my_message <- function(mystring, myarg) {
52+
invisible(.Call(`_cpp11test_my_message`, mystring, myarg))
53+
}
54+
55+
my_message_n1 <- function(mystring) {
56+
invisible(.Call(`_cpp11test_my_message_n1`, mystring))
57+
}
58+
5959
remove_altrep <- function(x) {
6060
.Call(`_cpp11test_remove_altrep`, x)
6161
}
@@ -160,6 +160,34 @@ cpp11_safe_ <- function(x_sxp) {
160160
.Call(`_cpp11test_cpp11_safe_`, x_sxp)
161161
}
162162

163+
sum_dbl_for_ <- function(x) {
164+
.Call(`_cpp11test_sum_dbl_for_`, x)
165+
}
166+
167+
sum_dbl_for2_ <- function(x_sxp) {
168+
.Call(`_cpp11test_sum_dbl_for2_`, x_sxp)
169+
}
170+
171+
sum_dbl_for3_ <- function(x_sxp) {
172+
.Call(`_cpp11test_sum_dbl_for3_`, x_sxp)
173+
}
174+
175+
sum_dbl_foreach_ <- function(x) {
176+
.Call(`_cpp11test_sum_dbl_foreach_`, x)
177+
}
178+
179+
sum_dbl_foreach2_ <- function(x_sxp) {
180+
.Call(`_cpp11test_sum_dbl_foreach2_`, x_sxp)
181+
}
182+
183+
sum_dbl_accumulate_ <- function(x) {
184+
.Call(`_cpp11test_sum_dbl_accumulate_`, x)
185+
}
186+
187+
sum_dbl_accumulate2_ <- function(x_sxp) {
188+
.Call(`_cpp11test_sum_dbl_accumulate2_`, x_sxp)
189+
}
190+
163191
sum_int_for_ <- function(x) {
164192
.Call(`_cpp11test_sum_int_for_`, x)
165193
}
@@ -195,31 +223,3 @@ rcpp_sum_dbl_accumulate_ <- function(x_sxp) {
195223
rcpp_grow_ <- function(n_sxp) {
196224
.Call(`_cpp11test_rcpp_grow_`, n_sxp)
197225
}
198-
199-
sum_dbl_for_ <- function(x) {
200-
.Call(`_cpp11test_sum_dbl_for_`, x)
201-
}
202-
203-
sum_dbl_for2_ <- function(x_sxp) {
204-
.Call(`_cpp11test_sum_dbl_for2_`, x_sxp)
205-
}
206-
207-
sum_dbl_for3_ <- function(x_sxp) {
208-
.Call(`_cpp11test_sum_dbl_for3_`, x_sxp)
209-
}
210-
211-
sum_dbl_foreach_ <- function(x) {
212-
.Call(`_cpp11test_sum_dbl_foreach_`, x)
213-
}
214-
215-
sum_dbl_foreach2_ <- function(x_sxp) {
216-
.Call(`_cpp11test_sum_dbl_foreach2_`, x_sxp)
217-
}
218-
219-
sum_dbl_accumulate_ <- function(x) {
220-
.Call(`_cpp11test_sum_dbl_accumulate_`, x)
221-
}
222-
223-
sum_dbl_accumulate2_ <- function(x_sxp) {
224-
.Call(`_cpp11test_sum_dbl_accumulate2_`, x_sxp)
225-
}

‎cpp11test/src/cpp11.cpp

Lines changed: 97 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -21,54 +21,6 @@ extern "C" SEXP _cpp11test_data_frame_() {
2121
return cpp11::as_sexp(data_frame_());
2222
END_CPP11
2323
}
24-
// errors_fmt.cpp
25-
void my_stop(std::string mystring, int myarg);
26-
extern "C" SEXP _cpp11test_my_stop(SEXP mystring, SEXP myarg) {
27-
BEGIN_CPP11
28-
my_stop(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring), cpp11::as_cpp<cpp11::decay_t<int>>(myarg));
29-
return R_NilValue;
30-
END_CPP11
31-
}
32-
// errors_fmt.cpp
33-
void my_stop_n1(std::string mystring);
34-
extern "C" SEXP _cpp11test_my_stop_n1(SEXP mystring) {
35-
BEGIN_CPP11
36-
my_stop_n1(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring));
37-
return R_NilValue;
38-
END_CPP11
39-
}
40-
// errors_fmt.cpp
41-
void my_warning(std::string mystring, std::string myarg);
42-
extern "C" SEXP _cpp11test_my_warning(SEXP mystring, SEXP myarg) {
43-
BEGIN_CPP11
44-
my_warning(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring), cpp11::as_cpp<cpp11::decay_t<std::string>>(myarg));
45-
return R_NilValue;
46-
END_CPP11
47-
}
48-
// errors_fmt.cpp
49-
void my_warning_n1(std::string mystring);
50-
extern "C" SEXP _cpp11test_my_warning_n1(SEXP mystring) {
51-
BEGIN_CPP11
52-
my_warning_n1(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring));
53-
return R_NilValue;
54-
END_CPP11
55-
}
56-
// errors_fmt.cpp
57-
void my_message(std::string mystring, std::string myarg);
58-
extern "C" SEXP _cpp11test_my_message(SEXP mystring, SEXP myarg) {
59-
BEGIN_CPP11
60-
my_message(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring), cpp11::as_cpp<cpp11::decay_t<std::string>>(myarg));
61-
return R_NilValue;
62-
END_CPP11
63-
}
64-
// errors_fmt.cpp
65-
void my_message_n1(std::string mystring);
66-
extern "C" SEXP _cpp11test_my_message_n1(SEXP mystring) {
67-
BEGIN_CPP11
68-
my_message_n1(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring));
69-
return R_NilValue;
70-
END_CPP11
71-
}
7224
// errors.cpp
7325
void my_stop_n1fmt(std::string mystring);
7426
extern "C" SEXP _cpp11test_my_stop_n1fmt(SEXP mystring) {
@@ -117,6 +69,54 @@ extern "C" SEXP _cpp11test_my_message_n2fmt(SEXP mystring, SEXP myarg) {
11769
return R_NilValue;
11870
END_CPP11
11971
}
72+
// errors_fmt.cpp
73+
void my_stop(std::string mystring, int myarg);
74+
extern "C" SEXP _cpp11test_my_stop(SEXP mystring, SEXP myarg) {
75+
BEGIN_CPP11
76+
my_stop(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring), cpp11::as_cpp<cpp11::decay_t<int>>(myarg));
77+
return R_NilValue;
78+
END_CPP11
79+
}
80+
// errors_fmt.cpp
81+
void my_stop_n1(std::string mystring);
82+
extern "C" SEXP _cpp11test_my_stop_n1(SEXP mystring) {
83+
BEGIN_CPP11
84+
my_stop_n1(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring));
85+
return R_NilValue;
86+
END_CPP11
87+
}
88+
// errors_fmt.cpp
89+
void my_warning(std::string mystring, std::string myarg);
90+
extern "C" SEXP _cpp11test_my_warning(SEXP mystring, SEXP myarg) {
91+
BEGIN_CPP11
92+
my_warning(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring), cpp11::as_cpp<cpp11::decay_t<std::string>>(myarg));
93+
return R_NilValue;
94+
END_CPP11
95+
}
96+
// errors_fmt.cpp
97+
void my_warning_n1(std::string mystring);
98+
extern "C" SEXP _cpp11test_my_warning_n1(SEXP mystring) {
99+
BEGIN_CPP11
100+
my_warning_n1(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring));
101+
return R_NilValue;
102+
END_CPP11
103+
}
104+
// errors_fmt.cpp
105+
void my_message(std::string mystring, std::string myarg);
106+
extern "C" SEXP _cpp11test_my_message(SEXP mystring, SEXP myarg) {
107+
BEGIN_CPP11
108+
my_message(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring), cpp11::as_cpp<cpp11::decay_t<std::string>>(myarg));
109+
return R_NilValue;
110+
END_CPP11
111+
}
112+
// errors_fmt.cpp
113+
void my_message_n1(std::string mystring);
114+
extern "C" SEXP _cpp11test_my_message_n1(SEXP mystring) {
115+
BEGIN_CPP11
116+
my_message_n1(cpp11::as_cpp<cpp11::decay_t<std::string>>(mystring));
117+
return R_NilValue;
118+
END_CPP11
119+
}
120120
// find-intervals.cpp
121121
SEXP remove_altrep(SEXP x);
122122
extern "C" SEXP _cpp11test_remove_altrep(SEXP x) {
@@ -310,6 +310,55 @@ extern "C" SEXP _cpp11test_cpp11_safe_(SEXP x_sxp) {
310310
return cpp11::as_sexp(cpp11_safe_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(x_sxp)));
311311
END_CPP11
312312
}
313+
// sum.cpp
314+
double sum_dbl_for_(cpp11::doubles x);
315+
extern "C" SEXP _cpp11test_sum_dbl_for_(SEXP x) {
316+
BEGIN_CPP11
317+
return cpp11::as_sexp(sum_dbl_for_(cpp11::as_cpp<cpp11::decay_t<cpp11::doubles>>(x)));
318+
END_CPP11
319+
}
320+
// sum.cpp
321+
double sum_dbl_for2_(SEXP x_sxp);
322+
extern "C" SEXP _cpp11test_sum_dbl_for2_(SEXP x_sxp) {
323+
BEGIN_CPP11
324+
return cpp11::as_sexp(sum_dbl_for2_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(x_sxp)));
325+
END_CPP11
326+
}
327+
// sum.cpp
328+
double sum_dbl_for3_(SEXP x_sxp);
329+
extern "C" SEXP _cpp11test_sum_dbl_for3_(SEXP x_sxp) {
330+
BEGIN_CPP11
331+
return cpp11::as_sexp(sum_dbl_for3_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(x_sxp)));
332+
END_CPP11
333+
}
334+
// sum.cpp
335+
double sum_dbl_foreach_(cpp11::doubles x);
336+
extern "C" SEXP _cpp11test_sum_dbl_foreach_(SEXP x) {
337+
BEGIN_CPP11
338+
return cpp11::as_sexp(sum_dbl_foreach_(cpp11::as_cpp<cpp11::decay_t<cpp11::doubles>>(x)));
339+
END_CPP11
340+
}
341+
// sum.cpp
342+
double sum_dbl_foreach2_(SEXP x_sxp);
343+
extern "C" SEXP _cpp11test_sum_dbl_foreach2_(SEXP x_sxp) {
344+
BEGIN_CPP11
345+
return cpp11::as_sexp(sum_dbl_foreach2_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(x_sxp)));
346+
END_CPP11
347+
}
348+
// sum.cpp
349+
double sum_dbl_accumulate_(cpp11::doubles x);
350+
extern "C" SEXP _cpp11test_sum_dbl_accumulate_(SEXP x) {
351+
BEGIN_CPP11
352+
return cpp11::as_sexp(sum_dbl_accumulate_(cpp11::as_cpp<cpp11::decay_t<cpp11::doubles>>(x)));
353+
END_CPP11
354+
}
355+
// sum.cpp
356+
double sum_dbl_accumulate2_(SEXP x_sxp);
357+
extern "C" SEXP _cpp11test_sum_dbl_accumulate2_(SEXP x_sxp) {
358+
BEGIN_CPP11
359+
return cpp11::as_sexp(sum_dbl_accumulate2_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(x_sxp)));
360+
END_CPP11
361+
}
313362
// sum_int.cpp
314363
double sum_int_for_(cpp11::integers x);
315364
extern "C" SEXP _cpp11test_sum_int_for_(SEXP x) {
@@ -373,55 +422,6 @@ extern "C" SEXP _cpp11test_rcpp_grow_(SEXP n_sxp) {
373422
return cpp11::as_sexp(rcpp_grow_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(n_sxp)));
374423
END_CPP11
375424
}
376-
// sum.cpp
377-
double sum_dbl_for_(cpp11::doubles x);
378-
extern "C" SEXP _cpp11test_sum_dbl_for_(SEXP x) {
379-
BEGIN_CPP11
380-
return cpp11::as_sexp(sum_dbl_for_(cpp11::as_cpp<cpp11::decay_t<cpp11::doubles>>(x)));
381-
END_CPP11
382-
}
383-
// sum.cpp
384-
double sum_dbl_for2_(SEXP x_sxp);
385-
extern "C" SEXP _cpp11test_sum_dbl_for2_(SEXP x_sxp) {
386-
BEGIN_CPP11
387-
return cpp11::as_sexp(sum_dbl_for2_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(x_sxp)));
388-
END_CPP11
389-
}
390-
// sum.cpp
391-
double sum_dbl_for3_(SEXP x_sxp);
392-
extern "C" SEXP _cpp11test_sum_dbl_for3_(SEXP x_sxp) {
393-
BEGIN_CPP11
394-
return cpp11::as_sexp(sum_dbl_for3_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(x_sxp)));
395-
END_CPP11
396-
}
397-
// sum.cpp
398-
double sum_dbl_foreach_(cpp11::doubles x);
399-
extern "C" SEXP _cpp11test_sum_dbl_foreach_(SEXP x) {
400-
BEGIN_CPP11
401-
return cpp11::as_sexp(sum_dbl_foreach_(cpp11::as_cpp<cpp11::decay_t<cpp11::doubles>>(x)));
402-
END_CPP11
403-
}
404-
// sum.cpp
405-
double sum_dbl_foreach2_(SEXP x_sxp);
406-
extern "C" SEXP _cpp11test_sum_dbl_foreach2_(SEXP x_sxp) {
407-
BEGIN_CPP11
408-
return cpp11::as_sexp(sum_dbl_foreach2_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(x_sxp)));
409-
END_CPP11
410-
}
411-
// sum.cpp
412-
double sum_dbl_accumulate_(cpp11::doubles x);
413-
extern "C" SEXP _cpp11test_sum_dbl_accumulate_(SEXP x) {
414-
BEGIN_CPP11
415-
return cpp11::as_sexp(sum_dbl_accumulate_(cpp11::as_cpp<cpp11::decay_t<cpp11::doubles>>(x)));
416-
END_CPP11
417-
}
418-
// sum.cpp
419-
double sum_dbl_accumulate2_(SEXP x_sxp);
420-
extern "C" SEXP _cpp11test_sum_dbl_accumulate2_(SEXP x_sxp) {
421-
BEGIN_CPP11
422-
return cpp11::as_sexp(sum_dbl_accumulate2_(cpp11::as_cpp<cpp11::decay_t<SEXP>>(x_sxp)));
423-
END_CPP11
424-
}
425425

426426
extern "C" {
427427
/* .Call calls */

‎cpp11test/src/test-doubles.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -394,10 +394,10 @@ context("doubles-C++") {
394394
e.push_back("b");
395395
expect_error(cpp11::as_doubles(e));
396396

397-
cpp11::writable::integers na;
398-
na.push_back(cpp11::na<int>());
397+
cpp11::writable::integers na{NA_INTEGER, 42};
399398
cpp11::doubles na2(cpp11::as_doubles(na));
400399
expect_true(cpp11::is_na(na2[0]));
400+
expect_true(!cpp11::is_na(na2[1]));
401401
}
402402

403403
test_that("doubles operator[] and at") {

0 commit comments

Comments
 (0)