Unconditionally clone in df_proxy()
#1838
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1837
I think it was a mistake to not clone in
df_proxy()
if the original input was not referenced. There are cases where we:x = vec_cast(x, ptype)
to cast to a common type)x_proxy = vec_proxy_equal(x)
and perform some manipulation on the proxyx
using information gained from the proxyThis is what is done in
vec_set_*()
.When a data frame is supplied as
x
, the current implementation ofdf_proxy()
means thevec_proxy_equal()
step from above modifiesx
directly (because it wasn't referenced), so the last step of slicingx
ends up slicing and returning a proxy rather than the original object.It doesn't look like the more sensitive functions are affected by this.
I checked
vec_rbind()
in particular, and it isn't affected because we doxs <- vec_cast_common(xs)
, which does create freshx
elements, but it immediately stores them in a list soMAYBE_REFERENCED()
was already returningtrue
for them.