r_string(char[N]) constructor

[romainfrancois]

This is an. experiment to see if this still gets this error on actions: https://github.com/r-lib/cpp11/runs/915320770#step:10:116

##[error]Error: package or namespace load failed for ‘cpp11test’ in dyn.load(file, DLLpath = DLLpath, ...):
117
ERROR: loading failed
118
 unable to load shared object '/Users/runner/work/_temp/Library/00LOCK-cpp11test/00new/cpp11test/libs/cpp11test.so':
119
  dlopen(/Users/runner/work/_temp/Library/00LOCK-cpp11test/00new/cpp11test/libs/cpp11test.so, 6): Symbol not found: __ZN5cpp118writable8r_vectorINS_8r_stringEE5proxyaSIA2_cEERS4_RKT_
120
  Referenced from: /Users/runner/work/_temp/Library/00LOCK-cpp11test/00new/cpp11test/libs/cpp11test.so
121
  Expected in: flat namespace
122
 in /Users/runner/work/_temp/Library/00LOCK-cpp11test/00new/cpp11test/libs/cpp11test.so

with:

$ c++filt __ZN5cpp118writable8r_vectorINS_8r_stringEE5proxyaSIA2_cEERS4_RKT_
cpp11::writable::r_vector<cpp11::r_string>::proxy& cpp11::writable::r_vector<cpp11::r_string>::proxy::operator=<char [2]>(char const (&) [2])

[romainfrancois]

🤷 nope, still getting this: https://github.com/r-lib/cpp11/pull/61/checks?check_run_id=915435071#step:10:95

##[error]Error: package or namespace load failed for ‘cpp11test’ in dyn.load(file, DLLpath = DLLpath, ...):
91
ERROR: loading failed
92
* removing ‘/Users/runner/work/_temp/Library/cpp11test’
93
 unable to load shared object '/Users/runner/work/_temp/Library/00LOCK-cpp11test/00new/cpp11test/libs/cpp11test.so':
94
* restoring previous ‘/Users/runner/work/_temp/Library/cpp11test’
95
  dlopen(/Users/runner/work/_temp/Library/00LOCK-cpp11test/00new/cpp11test/libs/cpp11test.so, 6): Symbol not found: __ZN5cpp118writable8r_vectorINS_8r_stringEE5proxyaSIA2_cEERS4_RKT_
96
  Referenced from: /Users/runner/work/_temp/Library/00LOCK-cpp11test/00new/cpp11test/libs/cpp11test.so
97
  Expected in: flat namespace
98
 in /Users/runner/work/_temp/Library/00LOCK-cpp11test/00new/cpp11test/libs/cpp11test.so

[bkietz]

Suggested change

	r_string(char data [N]) : data_(safe[Rf_mkCharLenCE](data, N, CE_UTF8)) {}
	r_string(char (&data)[N]) : data_(safe[Rf_mkCharLenCE](data, N, CE_UTF8)) {}

[bkietz]

More to the point, I don't see a definition for r_vector<T>::proxy::operator= inline or out of line with the other operator defs where I'd expect. Same for proxy<T>::operator T. I'd expect you to need something like

diff --git a/inst/include/cpp11/r_vector.hpp b/inst/include/cpp11/r_vector.hpp
index 1222af4..6e4a203 100644
--- a/inst/include/cpp11/r_vector.hpp
+++ b/inst/include/cpp11/r_vector.hpp
@@ -872,6 +872,18 @@ inline r_vector<T>::operator SEXP() const {
   return data_;
 }
 
+template <typename T>
+template <typename U>
+typename r_vector<T>::proxy& r_vector<T>::proxy::operator=(const U& rhs) {
+  set_value_at(data_, index_, is_altrep_, as_sexp(rhs));
+  return *this;
+}
+
+template <typename T>
+r_vector<T>::proxy::operator T() const {
+  return as_cpp<T>(get_value_at(data_, index_, is_altrep_));
+}
+
 template <typename T>
 inline typename r_vector<T>::proxy& r_vector<T>::proxy::operator+=(const T& rhs) {
   operator=(static_cast<T>(*this) + rhs);

[bkietz]

@jimhester I invented set_value_at and get_value_at, what should those be replaced with?

[jimhester]

They are defined in the headers the implement the specializations, e.g.

cpp11/inst/include/cpp11/doubles.hpp

Lines 51 to 72 in c17cd96

	template <>
	template <>
	inline typename r_vector<double>::proxy& r_vector<double>::proxy::operator=<double>(
	const double& rhs) {
	if (is_altrep_) {
	// NOPROTECT: likely too costly to unwind protect every set elt
	SET_REAL_ELT(data_, index_, rhs);
	} else {
	*p_ = rhs;
	}
	return *this;
	}
	template <>
	inline r_vector<double>::proxy::operator double() const {
	if (p_ == nullptr) {
	// NOPROTECT: likely too costly to unwind protect every elt
	return REAL_ELT(data_, index_);
	} else {
	return *p_;
	}
	}

[bkietz]

Hmmm, defining them this way opens us up to confusing errors. We won't get a compilation error for

writable::doubles d = //...
d[3] = "wtf";

instead we'll get a linker error complaining that r_vector<double>::proxy& r_vector<double>::proxy::operator=<char(&)[4]>(char (&)[4]) is not defined. What would you think of moving to a mixin based impl for vector proxies? Sketch:

master...bkietz:r_vector_proxy-mixins

[jimhester]

I think I would rather revert #57 make users use TRUE and FALSE as it was previously then complicate this code further.

[jimhester]

I have reverted #57 for now, maybe we can come up with a different solution to that issue that would avoid this issue.