cpp11 icon indicating copy to clipboard operation
cpp11 copied to clipboard

Unclear error when `r_string` is used as an argument

Open stephematician opened this issue 2 years ago • 3 comments
trafficstars

I know why this doesn't work; however it took a while to track down (and I didn't find help elsewhere) - so at least if I put this here it might help others.

Basic idea is what if you want to pass a 'scalar' character vector (i.e. invoke a function in R like foo("test")), it looks like the pattern I have to use is:

cpp11::cpp_source(code='
#include <string>
#include "cpp11.hpp"
[[cpp11::register]]
void foo(cpp11::strings bar) {
    if (bar.size() != 1) cpp11::stop("Expected one string");
    std::string str = bar[0];
}')
foo("hello")

No problems.

On the other hand, trying this (and hoping that cpp11 does the hard work) fails:

cpp11::cpp_source(code='
#include <string>
#include "cpp11.hpp"
[[cpp11::register]]
void foo(cpp11::r_string bar) { std::string str = bar;}')
foo("hello")
Error: 'translateCharUTF8' must be called on a CHARSXP, but got 'character'

As a suggestion, perhaps have a valid_type check on the data similar to r_vector; e.g.

// update the assignment in the constructor to check type
  r_string(SEXP data) : data_(valid_type(data)) {}
// definition of valid_type somewhere
inline SEXP r_string::valid_type(SEXP data) {
  if (data == nullptr) {
    throw std::exception( /* construct error msg */);
  }
  if (TYPEOF(data) != CHARSXP) {
    throw std::exception(/* construct error msg */);
  }
  return data;
}

stephematician avatar Mar 20 '23 10:03 stephematician

hi,I met a problem in laerning R ,can you give me some help? My question link is : How to pass parameters in .R file to cpp11 function in .cpp file

thank you very much if you can lend me some time!

Eunsolfs avatar Apr 13 '23 02:04 Eunsolfs

hi,I met a problem in laerning R ,can you give me some help? My question link is : How to pass parameters in .R file to cpp11 function in .cpp file

thank you very much if you can lend me some time!

I don't know if you did, but I would try https://www.stackoverflow.com to post problems like this.

stephematician avatar Apr 14 '23 23:04 stephematician

thank you for your help. Finally i got the solution –—— n <- sum(c(1.1,2.2,3.5)) use c(...) to pass vector of numbers

---- Replied Message ---- | From | @.> | | Date | 04/15/2023 07:27 | | To | @.> | | Cc | @.>@.> | | Subject | Re: [r-lib/cpp11] Unclear error when r_string is used as an argument (Issue #311) |

hi,I met a problem in laerning R ,can you give me some help? My question link is : How to pass parameters in .R file to cpp11 function in .cpp file

thank you very much if you can lend me some time!

I don't know if you did, but I would try https://www.stackoverflow.com to post problems like this.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

Eunsolfs avatar Apr 14 '23 23:04 Eunsolfs

I think we want the r_string(SEXP data) constructor to be very very fast, as it is used on each element of a character vector in some cases, so I don't think we should add that type check.

FWIW we do allow conversion directly into a std::string, which is what you wanted I think

cpp11::cpp_source(code='
#include <string>
#include "cpp11.hpp"

[[cpp11::register]]
cpp11::writable::strings foo(std::string bar) {
  cpp11::writable::strings out = { bar }; 
  return out;
}'
)

foo("hello")
#> [1] "hello"

Note to self that this occurs because we have enable_if_constructible_from_sexp here: https://github.com/r-lib/cpp11/blob/b0576f2a0094cdf7af3663df9f8b049930e6b894/inst/include/cpp11/as.hpp#L33-L37

Used in https://github.com/r-lib/cpp11/blob/b0576f2a0094cdf7af3663df9f8b049930e6b894/inst/include/cpp11/as.hpp#L81-L84

And r_string is of course constructible from a SEXP, just a CHARSXP, not a STRSXP https://github.com/r-lib/cpp11/blob/b0576f2a0094cdf7af3663df9f8b049930e6b894/inst/include/cpp11/r_string.hpp#L17

It's possible we could add a more specific as_cpp() condition for something like enable_is_r_string, but I'm not sure that would conflict with enable_if_constructible_from_sexp since both would technically be valid.

DavisVaughan avatar Aug 26 '24 19:08 DavisVaughan

Thanks. I think I had an unusual use-case; most of the time casting to std::string works a-ok, but it does involve a copy.

FWIW - I'd be surprised if there was a substantial performance hit; between inlining, branch prediction, and the simple pointer access via TYPEOF, most CPUs will cruise through it ... but I am willing to be corrected if anyone did some profiling.

Thanks again for looking into this.

stephematician avatar Aug 27 '24 02:08 stephematician