Revisit special casing of `vec_ptype2.logical.list()`

[DavisVaughan]

See #812 (comment)

vec_ptype2.logical.list() was special cased to not allow vec_assign(list(1), 1, NA), but vec_assign(list(1), 1, unspecified(1)) happened to slip through because it isn't caught by that method. This special case complicates the vec_type2() C code and is why I need vec_ptype2_dispatch_unspecified_list().

I personally think that vec_assign(list(1), 1, NA) should be allowed, and is well defined. NA is cast to list(NULL), and then assigned.

So my suggestion would be to:

• Remove vec_ptype2.logical.list() entirely, and let vec_ptype2.logical.default() be called

• Remove the C level vec_ptype2_dispatch_unspecified_list() special case (remove C level stop_incompatible_type() if we do this)

• Update the test in test-slice-assign.R to allow NA to be assigned into lists (and continue monitoring the existing test that unspecified() can also be assigned into lists)

[DavisVaughan]

Easier explanation:

library(vctrs)

# why is this not allowed?
vec_assign(list(1), 1, NA)
#> Error: No common type for `value` <logical> and `x` <list>.

# it should be equivalent to this
vec_assign(list(1), 1, list(NULL))
#> [[1]]
#> NULL

# we can still do this too
vec_assign(list(1), 1, list(NA))
#> [[1]]
#> [1] NA

# we can currently already do this, which i think is just an oversight
vec_assign(list(1), 1, unspecified(1))
#> [[1]]
#> NULL

^{Created on 2020-02-14 by the reprex package (v0.3.0)}

[hadley]

Yeah, agreed

[lionel-]

vec_assign(list(1), 1, NA) should be allowed, and is well defined. NA is cast to list(NULL), and then assigned.

This is now clear to me as well. It wasn't as clear in older versions of vctrs when we were less consistent regarding the coercibility of unspecified to list. It was based on coercibility of atomics and lists:

# vctrs 0.1.0
vctrs::vec_cast(NA, list())
#> [[1]]
#> [1] NA

Since we were using vec_cast() for chopping atomic vectors into lists, we couldn't use the normal unspecified rules because there was a fundamental ambiguity between list(NA) and list(NULL).

The landscape is different now that (1) we're going towards a stricter vec_cast() where only unspecified (and not logical) can be cast to list; (2) we have vec_chop() to convert any vector to a list. In that context basing the coercibility of NA + list() on unspecified is well defined and clearly the right approach.