Missing push_back(SEXP) for some r_vector specializations
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 35/100
Research direction
Start by reading the r_vector push_back implementation in inst/include/cpp11/strings.hpp and the r_string constructors in inst/include/cpp11/r_string.hpp. Check the proposed SEXP handling across the r_vector specializations, then verify that named_arg insertion works where intended while accounting for the stated r_string and raws limitations.
Written by the indexing model from the issue text.
Description
Context
In #86, I tested push_back(named_arg) only for the list type. c31d05a942331803f1b3444be4d1a722df5d4fff fixed the empty names() issue, which makes the method now work properly with list.
Problem
Unfortunately list is a "special specialization" in the sense that it's typedefed as r_vector<SEXP>. The value type happens to match the type of named_arg.value(), which is not the case for other specializations of r_vector. Calling a.push_back(named_arg) for any type of a other than list will fail because of "no matching member function". This is because in https://github.com/r-lib/cpp11/blob/779669a4d0b07e9f9d9382114f44e4f6ff68eebb/inst/include/cpp11/strings.hpp#L173-L174
push_back(value.value()) only accept specialized type T, but named_arg.value() has fixed type SEXP.
Actually, at first I didn't expect other specializations to work with named_arg, but I found that all of them accept std::initializer_list<named_arg> at construction, so it seems logical to assume that they would continue to work with named_arg after construction.
Proposition
In 6d7fa26b923f040c2daeb5f1685d7753f0e05177 I tried to fix the above issue by providing a method push_back(SEXP) for all specializations (except for list which already has it). Currently it has the following limitations which are related to topics beyond the scope of this issue:
- For
r_stringI have to use the following cumbersome cast
push_back(r_string(as_cpp<std::string>(value)));
because during the construction of r_string, the ctor for SEXP is different from the ctors for std::string and char* though they all declare data_ as SEXP
https://github.com/r-lib/cpp11/blob/779669a4d0b07e9f9d9382114f44e4f6ff68eebb/inst/include/cpp11/r_string.hpp#L16-L18
This can be demonstrated by the following snippets:
# construct r_string from std::string, ok
cpp11::cpp_function('strings test() {
writable::strings named_list({r_string("FALSE")});
return named_list;
}')
print(test())
#> [1] "FALSE"
# construct r_string from SEXP, type error
cpp11::cpp_function('strings test() {
writable::strings named_list({r_string(as_sexp("FALSE"))});
return named_list;
}')
print(test())
#> Error: 'translateCharUTF8' must be called on a CHARSXP, but got 'character'
So maybe something could be done for the ctors of r_string, but it's not the concern here.
- I didn't add
push_back(SEXP)forrawstype because i'm unsure of the intended behavior.rawsistypedefed asr_vector<uint8_t>whereas currently there are noasfunctions for theuint8_t(or no converting functions betweencharanduint8_t). Once these are defined the method can be added forrawsas well.
Thank you for the reading, and if agreed I will open a PR draft from my fork.
- 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
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
AXERA-TECH/ax-llm#77 ·
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
games-on-whales/wolf#509 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
-
bug-unconfirmed
Difficulty 2/5 1-3 hours Newbie friendliness 76/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
NVIDIA/cuda-samples#453 ·