as_sexp and as_cpp are not easily extensible
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 35/100
- Issue type
- Feature
- Clarity
- Mostly clear
- Activity status
- Stale
- Domain
- api, backend-api-design
Research direction
Start with the reproducer in cpp11test/src/test-as.cpp and inspect the conversion declarations included through cpp11.hpp. Compare the current as_cpp/as_sexp overload approach with the proposed custom_conversion trait. Done should mean custom types can provide conversions without pre-declaring overloads, while existing shared_ptr and vector conversions continue to work.
Written by the indexing model from the issue text.
Description
Arrow defines as_cpp and as_sexp for shared_ptr and vectors of shared_ptr using external_pointer, since both of those are used throughout the arrow API. This is currently accomplished by declaring the overloads of as_sexp before inclusion of the cpp11 headers:
namespace cpp11 {
template <typename T>
SEXP as_sexp(const std::shared_ptr<T>& ptr);
template <typename T>
SEXP as_sexp(const std::vector<std::shared_ptr<T>>& vec);
} // namespace cpp11
#include <cpp11.hpp>
https://github.com/apache/arrow/pull/7819/files#diff-db94c392857c3bad4f5f69e86cde917bR24-R32
Without this pre-declaration, ADL fails (for example) when named_arg::operator= attempts to instantiate as_sexp(const shared_ptr<T>&).
Reproducer:
diff --git a/cpp11test/src/test-as.cpp b/cpp11test/src/test-as.cpp
index 76b4ee3..c8770a8 100644
--- a/cpp11test/src/test-as.cpp
+++ b/cpp11test/src/test-as.cpp
@@ -8,7 +8,39 @@
#include "Rcpp.h"
+namespace test {
+
+struct triple {
+ std::string arch, vendor, os;
+};
+
+} // namespace test
+
+namespace cpp11 {
+
+template <typename T>
+cpp11::enable_if_t<std::is_same<T, test::triple>::value, test::triple> as_cpp(SEXP from) {
+ cpp11::strings r{from};
+
+ if (r.size() == 3) {
+ return test::triple{.arch = r[0], .vendor = r[1], .os = r[2]};
+ }
+
+ stop("Expected string vector of length 3");
+}
+
+SEXP as_sexp(const test::triple& from) {
+ return cpp11::writable::strings({from.arch, from.vendor, from.os});
+}
+
+} // namespace cpp11
+
context("as_cpp-C++") {
+ test_that("as_cpp<custom type>(SEXP)") {
+ cpp11::writable::list(
+ {"fs"_nm = test::triple{.arch = "seven", .vendor = "ono", .os = "sendai"}});
+ }
+
test_that("as_cpp<integer>(INTSEXP)") {
SEXP r = PROTECT(Rf_allocVector(INTSXP, 1));
INTEGER(r)[0] = 42;
This could be resolved by providing a user-specializable trait for conversion, for example:
template <typename T, typename Enable = void>
struct custom_conversion;
template <typename T>
auto as_cpp(SEXP from) -> decltype(custom_conversion<T>::as_cpp(from)) {
return custom_conversion<T>::as_cpp(from);
}
template <typename T>
auto as_sexp(const T& from) -> decltype(custom_conversion<T>::as_sexp(from)) {
return custom_conversion<T>::as_sexp(from);
}
Which in the case of the reproducer above would be used like so:
namespace cpp11 {
template <>
struct custom_conversion<test::triple> {
static test::triple as_cpp(SEXP from) {
cpp11::strings r{from};
if (r.size() == 3) {
return test::triple{.arch = r[0], .vendor = r[1], .os = r[2]};
}
stop("Expected string vector of length 3");
}
static SEXP as_sexp(const test::triple& from) {
return cpp11::writable::strings({from.arch, from.vendor, from.os});
}
};
}
- Dominant language
- C++
- Stars
- 224
- Forks
- 52
- PR merge metrics
- No merged PRs in 30d
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from r-lib/cpp11
-
Difficulty 3/5 1-2 days Newbie friendliness 35/100
-
Difficulty 3/5 1-2 days Newbie friendliness 48/100
-
Difficulty 5/5 Over a week Newbie friendliness 20/100
-
Difficulty 3/5 1-2 days Newbie friendliness 42/100
-
Difficulty 3/5 1-2 days Newbie friendliness 45/100
Similar issues
-
ai_reviewed
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
ydb-platform/ydb#53869 · 3 comments ·
-
bug cert blocker needs triage
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
project-chip/connectedhomeip#74373 ·
-
upstream update
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
conan-io/conan-center-index#31035 ·
-
Bug
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
documentation
Difficulty 1/5 Under an hour Newbie friendliness 85/100
vllm-project/vllm-ascend#17329 ·