Another rchk issue to look into

[DavisVaughan]

trafficstars

Function cpp11::writable::r_vector<SEXPREC*>::r_vector(std::initializer_list<cpp11::named_arg>)::{lambda()#1}::operator()() const
  [UP] unprotected variable names while calling allocating function Rf_mkCharCE cpp11/include/cpp11/list.hpp:91

Function cpp11::writable::r_vector<cpp11::r_bool>::r_vector(std::initializer_list<cpp11::named_arg>)::{lambda()#1}::operator()() const
  [UP] unprotected variable names while calling allocating function Rf_mkCharCE cpp11/include/cpp11/r_vector.hpp:890

https://raw.githubusercontent.com/kalibera/cran-checks/master/rchk/results/adfExplorer.out https://raw.githubusercontent.com/kalibera/cran-checks/master/rchk/results/cpp11bigwig.out

I believe it is saying we have an unprotected SEXP called names at the point that we call Rf_mkCharCE(). This is surely a false alarm, because Rf_setAttrib() protects names by installed it as an attribute on data_, but I guess we can try to work around it.

template <>
inline r_vector<SEXP>::r_vector(std::initializer_list<named_arg> il)
    : cpp11::r_vector<SEXP>(safe[Rf_allocVector](VECSXP, il.size())),
      capacity_(il.size()) {
  unwind_protect([&] {
    SEXP names = Rf_allocVector(STRSXP, capacity_);
    Rf_setAttrib(data_, R_NamesSymbol, names);

    auto it = il.begin();

    for (R_xlen_t i = 0; i < capacity_; ++i, ++it) {
      SEXP elt = it->value();
      set_elt(data_, i, elt);

      SEXP name = Rf_mkCharCE(it->name(), CE_UTF8);
      SET_STRING_ELT(names, i, name);
    }
  });
}

[pachadotdev]

@DavisVaughan how about https://github.com/r-lib/cpp11/pull/457 ?

[krlmlr]

@kalibera: I'm seeing this with RSQLite and other packages. Do you agree that the early Rf_setAttrib() calls in https://github.com/r-lib/cpp11/blob/main/inst/include/cpp11/list.hpp#L83 protects the names variable? The set_elt() call is a simple SET_VECTOR_ELT(), and nothing else affects data_ (the owner of the protection)? Can you change rchk to stop flagging this as an error, or is it easier to work around (the fix by Mauricio is easy enough)? What would it take for a fix to appear on CRAN? Thanks!

[kalibera]

I'd consider changing the code. It is not guaranteed that if you set names attribute to a given value that the value will be used by reference. So, to fill in names , I would do something like

names = PROTECT(allocVector(...)) loop to fill in element of names Rf_setAttrib(x, R_NamesSymbol, names); UNPROTECT(1); /* names */

I think it will make the code more correct/defensive. It could also get rid of the rchk warning.

In the current implementation of R, if x (data_) is a string vector and the value of names is also a string vector, it will be used by reference, but very likely rchk is not able to see this from this code. Under some other conditions, setting names attribute of a value will use a new object (the old names argument to setAttrib() will not only not be protected, but wouldn't reflect the attributes of the object).

[DavisVaughan]

Under some other conditions, setting names attribute of a value will use a new object (the old names argument to setAttrib() will not only not be protected, but wouldn't reflect the attributes of the object).

I don't think the names argument would ever be unprotected when using Rf_setAttrib() and R_NamesSymbol:

• setAttrib() starts here
• In the normal path it drops through to namesgets(), which immediately protects val itself
  • https://github.com/wch/r-source/blob/6f3a17be9d4d10205ac4c8a1957235f9a2b45373/src/main/attrib.c#L258
  • https://github.com/wch/r-source/blob/6f3a17be9d4d10205ac4c8a1957235f9a2b45373/src/main/attrib.c#L969
• In the string name edge case it protects val before doing any funny business with name
• In the NULL val case, protection is not needed

---

Regardless, in this particular case it's not worth fighting rchk, so we can make the change when I work on cpp11 next

[kalibera]

That setAttrib() protects names during its own processing is another thing. What I meant is that after setAttrib() finishes, the argument "names" that has been passed to it may not be protected any longer as it would not be (by reference) used by the object. When names is a string vector and x is a vector, this wouldn't happen - it the names object will be re-used by reference. But it would happen e.g. for pairlists, see namesgets().