Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

should cpp11 support string_view? #37

Closed
CreRecombinase opened this issue Jul 1, 2020 · 4 comments
Closed

should cpp11 support string_view? #37

CreRecombinase opened this issue Jul 1, 2020 · 4 comments
Labels
feature a feature request or enhancement

Comments

@CreRecombinase
Copy link

R strings seem like a pretty perfect use case for std::string_view. For compilers that don't support c++17, there are backports available. Any interest in a pull request?

@jimhester
Copy link
Member

I don't think it is entirely trivial to do this. First as you mentioned we would need to use compatibility library so we could use it with C++11.

Second and perhaps more importantly cpp11 uses translateCharUT8() to access character data from R vectors. This translates the characters to UTF-8 and (if needed) allocates memory that reclaimed at the end of the enclosing .Call(). If we use string_view() on this data and the string_view() outlasts the .Call() boundary the data will no longer be valid.

It might be that this particular case is rare enough and the benefits of string_view are substantial enough that it makes sense, I am not sure.

@jimhester
Copy link
Member

Another pretty serious issue with using string_view in cpp11 is that nearly all R APIs that have const char* expect null terminated strings, which may not be the case with a string_view.

@CreRecombinase
Copy link
Author

Yeah the point about possible allocation by translateCharUTF8 totally makes sense. I guess I was thinking of string_view as modeling the contents of a CHARSXP "as it is". Operationally, I think of a string_view as

  1. non-owning
  2. cheap to copy
  3. immutable (unlike other "data" SEXP, you can't muck easily around with the contents of a CHARSXP in-place because of the CHARSXP cache. string_view, unlike its cousin span, will only give you a const pointer to its contents)
  4. "knowing" its own length

@CreRecombinase CreRecombinase changed the title should string be string_view? should cpp11 support string_view? Jul 2, 2020
@jimhester jimhester added the feature a feature request or enhancement label Jul 6, 2020
@jimhester
Copy link
Member

Closing this issue for now, while I do think using string_view is a nice idea in theory, I don't think it really will work in practice for cpp11 due to the concerns expressed above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants