Missing push_back(SEXP) for some r_vector specializations

Đang mở
#89 2 bình luận 0 reaction 0 người được giao Xem trên GitHub

Chưa có ai nhận issue này.

Đánh giá

Độ khó
4/5
Thời gian dự kiến
3-5 ngày
Mức phù hợp với người mới
35/100
Loại issue
Lỗi
Độ rõ ràng
Khá rõ ràng
Mức độ hoạt động
Đình trệ
Công nghệ
cpp, r
Lĩnh vực
api

Hướng nghiên cứu

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.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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:

  1. For r_string I 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.

  1. I didn't add push_back(SEXP) for raws type because i'm unsure of the intended behavior. raws is typedefed as r_vector<uint8_t> whereas currently there are no as functions for the uint8_t (or no converting functions between char and uint8_t). Once these are defined the method can be added for raws as well.

Thank you for the reading, and if agreed I will open a PR draft from my fork.

Ngôn ngữ chính
C++
Star
224
Fork
52
Chỉ số merge pull request
Không có pull request nào được merge trong 30 ngày

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của r-lib/cpp11

Tất cả issue của r-lib/cpp11

Issue tương tự

Thêm issue về C++

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.