Add try_*_get() for each vector type, and fail more elegantly in Vector element extraction

[DavisVaughan]

Closes #499 (alternative approach)
Addresses posit-dev/positron#4537

If we try and extract an element with get_unchecked() that causes an error in an ALTREP Elt method (including but not limited to an OOM error), we now log an error and return error_elt() for that type, which is typically the NA value for that type, but for raw vectors it is 0 as an Rbyte.

I considered propagating the error up through the return value of get_unchecked() but that was painful. We use it in so many places that don't expect it to fail that propagating an error through would be a huge ordeal that doesn't seem worth it for this somewhat rare case. I think logging an error + showing NA or something like it is possibly the best we can do.

I confirmed that

x <- 1:1e10
names(x) <- x

doesn't crash immediately anymore, but expanding x in the Variables pane hangs and causes a 5 second timeout. Something is definitely not right here because even x <- 1:1e5 hangs for a long time on main when you try and expand x in the Variables pane.

[DavisVaughan]

In general things like this should no longer immediately crash the R session. Note that printing z in the console will immediately trigger the R error.

Also note that using STRING_ELT() on z results in the allocation error, which is why you see NA values in the Variables pane. Using STRING_ELT() is the best we can do in the general case for ALTREP objects (because it gives them a chance to do low memory operations if they can) but it just happens to trigger allocations for the deferred string ALTREP class in use here. I think trying to optimize any more for the deferred string case would be overoptimizing vs having a decent solution that works in the general case for ALTREP objects.

x <- 1:1e10
names(x) <- x
z <- names(x)

[dfalbel]

LGTM! Is it worth copying the tests from #499 ?

[lionel-]

LGTM!

I'm slightly worried about the performance overhead. This could be improved by:

• Detecting whether a vector is altrep or not on construction.
• If not altrep, get full vector slice and use that to index an element
• If altrep, use protected get, and this could be improved later on to retrieve regions so the protection would only happen once per region.

[lionel-]

Nice!

[lionel-]

Suggested change

	assert!(success.is_match(display.as_str()) || failure.is_match(display.as_str()));
	assert!(success.is_match(display.as_str()));

[lionel-]

and this could be improved later on to retrieve regions so the protection would only happen once per region.

I'm not familiar with altrep but would this behave better than the _ELT() methods? I.e. would this avoid a full materialisation? It seems like that's the purpose of retrieving a region slice?

[lionel-]

Do we need to be worried about the variable and data viewer panes being confusing with altrep objects? Would it be possible for the formatting paths to use the Result getter and display, say, <error> instead of NA, so that the output is at least googlable?

[DavisVaughan]

Note to self to wait for #508 and rework this on top of that after it is merged

[DavisVaughan]

would this avoid a full materialisation

It is completely up to the ALTREP class. The ALTREP author could decide that INTEGER_GET_REGION() can be implemented without materializing (by providing an INTEGER_OR_NULL() method), but INTEGER_ELT() can't. The important thing for us is that we don't have any control over that, and just have to try and use "the friendliest thing we can".

---

Using INTEGER_GET_REGION() combined with having a small buffer vector of the underlying C type is not unreasonable. cpp11 does exactly this actually. It may avoid repeated INTEGER_ELT() calls if the ALTREP type can provide a const DATAPTR without materializing:
https://github.com/wch/r-source/blob/64873e17ff8be3c305b0a7487e01b266bc4660c9/src/main/altrep.c#L404-L417

Otherwise it falls back to this:
https://github.com/wch/r-source/blob/64873e17ff8be3c305b0a7487e01b266bc4660c9/src/main/altrep.c#L755-L763

And FWIW, deferred strings can't provide a dataptr before it has fully materialized, so it would always fall back to the ELT loop fallback approach
https://github.com/wch/r-source/blob/64873e17ff8be3c305b0a7487e01b266bc4660c9/src/main/altclasses.c#L764-L768