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

std::string_view support in Rcpp::wrap() #1356

Merged
merged 4 commits into from
Feb 4, 2025

Conversation

evalon32
Copy link
Contributor

@evalon32 evalon32 commented Jan 30, 2025

Currently, Rcpp::wrap() treats a std::string_view argument by default as a generic container and creates a character vector from single characters. This is different from how std::string is handled. This change aligns the two, so that wrap("foo"sv) is equivalent to wrap("foo"s). The added code is conditionally compiled for C++17.

Checklist

  • Code compiles correctly
  • R CMD check still passes all tests
  • Preferably, new tests were added which fail without the change
  • Document the changes by file in ChangeLog

@eddelbuettel
Copy link
Member

Would you mind filling in the template you carried over unchanged?

@evalon32
Copy link
Contributor Author

This is a draft I created solely to reference in an issue I'm about to open to start a discussion :)

@eddelbuettel
Copy link
Member

Looking forward to it -- ex ante it's a good topic and something that would be nice to fill in.

@evalon32 evalon32 marked this pull request as ready for review February 1, 2025 03:06
@eddelbuettel
Copy link
Member

(Updated your first box above: if you make it [x] without the space you had [ x] then the GH flavored markdown takes it as a completed tick box.)

This looks good. I will toss this at the reverse dependency checker which, being underpowered on an ancient old box, will take a while. We will know more in a few days.

@eddelbuettel
Copy link
Member

There were no regressions, or, in the parlance, "changes to worse" so no objections from the reverse-depends check.

@eddelbuettel eddelbuettel merged commit 6633770 into RcppCore:master Feb 4, 2025
9 checks passed
Copy link
Member

@eddelbuettel eddelbuettel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM so merged.

@evalon32 evalon32 deleted the wrap-string-view branch February 4, 2025 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants