cpp11 icon indicating copy to clipboard operation
cpp11 copied to clipboard

Support nullable `external_ptr<>`

Open krlmlr opened this issue 2 years ago • 1 comments
trafficstars

Currently, a nullptr but not an R_NilValue can be assigned to an external_ptr<> object. This is inconsistent and surprising, in particular because the roundtrip works correctly the other way. Should we support this?

CC @hannes @TMonster.

library(cpp11)

# Works
cpp_source(
  code = '
  #include "cpp11/external_pointer.hpp"
  #include "cpp11/strings.hpp"

  [[cpp11::register]]
  cpp11::external_pointer<int> test() {
    return cpp11::external_pointer<int>(nullptr);
  }
  '
)

test()
#> NULL

# Fails, but perhaps shouldn't
cpp_source(
  code = '
  #include "cpp11/external_pointer.hpp"
  #include "cpp11/strings.hpp"

  [[cpp11::register]]
  cpp11::external_pointer<int> test() {
    return cpp11::external_pointer<int>(R_NilValue);
  }
  '
)

test()
#> Error: Invalid input type, expected 'externalptr' actual 'NULL'

Created on 2023-04-02 with reprex v2.0.2

krlmlr avatar Apr 02 '23 14:04 krlmlr

I can't see a good reason why it shouldn't accept R_NilValue (given that that is what the default constructor assigns). A simple change to external_pointer::valid_type() should fix this, i.e.

  static SEXP valid_type(SEXP data) {
    if (data == nullptr) {
      throw type_error(EXTPTRSXP, NILSXP);
    }
    if (TYPEOF(data) != EXTPTRSXP && data != R_NilValue) {
      throw type_error(EXTPTRSXP, TYPEOF(data));
    }

    return data;
  }

stephematician avatar Apr 16 '23 04:04 stephematician