vctrs icon indicating copy to clipboard operation
vctrs copied to clipboard

Infinite recursion with vec_c() and inherit_base_type = TRUE

Open krlmlr opened this issue 3 years ago • 6 comments

x <- vctrs::new_vctr(1.5:3.5, inherit_base_type = FALSE)
range(x)
#> <vctrs_vctr[2]>
#> [1] 1.5 3.5
x <- vctrs::new_vctr(1.5:3.5, inherit_base_type = TRUE)
range(x)
## Error: C stack usage  7973788 is too close to the limit

Created on 2021-03-24 by the reprex package (v1.0.0)

krlmlr avatar Mar 24 '21 02:03 krlmlr

Seems to be vec_c()

library(vctrs)

x <- new_vctr(1:3, inherit_base_type = TRUE)
vec_c(x, x)
# Error: C stack usage  7970416 is too close to the limit

y <- new_vctr(1:3, inherit_base_type = FALSE)
vec_c(y, y)
#> <vctrs_vctr[6]>
#> [1] 1 2 3 1 2 3

Created on 2021-03-24 by the reprex package (v1.0.0)

DavisVaughan avatar Mar 24 '21 12:03 DavisVaughan

This happens because needs_vec_c_fallback() here returns true, even though we tried to special case vctrs_vctr to return false.

https://github.com/r-lib/vctrs/blob/5eecfee07c6b231f29358b6883cee4c9e7f24576/src/c.c#L188-L204

The fallback ptype for this type is:

integer(0)
attr(,"class")
[1] "vctrs:::common_class_fallback"
attr(,"fallback_class")
[1] "vctrs_vctr" "integer"   

So the class used in needs_vec_c_fallback() is c("vctrs_vctr", "integer"). Because the last class isn't "vctrs_vctr", we don't detect that we should exit early

DavisVaughan avatar Mar 24 '21 12:03 DavisVaughan

The fallback paths are not fully fledged out unfortunately. The workaround is to subclass the vctrs_vctr.

Maybe this suggests we should implement explicit coercion methods for bare vctrs_vctr types. This is probably low priority. I'm not sure there should be an expectation that vctrs_vctr is a usable class in itself since dispatch will slow down many things and bare vectors are preferable to avoid this.

lionel- avatar Mar 25 '21 10:03 lionel-

Same with

x <- vctrs::new_vctr(1.5:3.5, class = "foo", inherit_base_type = TRUE)
range(x)

What do you mean by subclassing?

krlmlr avatar Mar 25 '21 10:03 krlmlr

Subclassing is what you did, creating a <foo/vctrs_vctr> class. Your class is incomplete though, the first thing to do is to create self-to-self coercion methods: https://vctrs.r-lib.org/reference/howto-faq-coercion.html#the-self-self-method

This is a bug, but since it occurs only in the very beginning of the creation of a class, it's not high priority (a fix won't make it in the next patch version).

lionel- avatar Mar 25 '21 10:03 lionel-

Thanks for the clarification. I'll add a self-self, no need to hurry.

krlmlr avatar Mar 25 '21 10:03 krlmlr