Skip to content

Conversation

@DavisVaughan
Copy link
Member

@DavisVaughan DavisVaughan commented Nov 11, 2025

Closes #2094

Basic idea is that when we see this

list_combine(
  x = list(NA),
  indices = list(1),
  size = 1,
  default = "a"
)

We need the common type across !!!x and default.

We "optimize" the ptype computation in list_combine() as

vec_ptype2(
  vec_ptype_common(x, .arg = x_arg),
  default,
  y_arg = default_arg
)

rather than as

vec_ptype_common(!!!x, default_arg := default)

Because this avoids us having to reallocate a possibly very large list just to slip default in there. It also avoids forcing default_arg. I'm also not entirely sure how we'd use x_arg if we did that.

But in this particular list_combine() example, we get

vec_ptype2(
  vec_ptype_common(x), # this returns a finalized `logical()`
  default, # this is `character()`
  y_arg = default_arg
)
# Error cant combine <logical> and <character>

So we actually need a way to tell vec_ptype_common() that we'd like to opt out of ptype finalization and promise to do it "manually" later on. Now that we've ensured that:

  • vec_ptype() and vec_ptype2() never finalize
  • vec_ptype_common() always finalizes

I think it makes some sense that we should have an internal way to make vec_ptype_common() act exactly like repeated vec_ptype2() for cases like this one.

…tion

Then use this in `list_combine()` to delay finalization until `default` has been considered.
ptype = KEEP(vec_ptype_common(
xs,
ptype,
PTYPE_FINALISE_false,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this is so we can set this to false

If you look at how ptype_common_with_default() works, it computes the common type of the list of xs using vec_ptype_common(), then needs to incorporate default's type via vec_ptype2() as well.

So we need the result of vec_ptype_common() to not be finalized one the way out, we will do it manually after calling vec_ptype2().

An example of this came up in revdeps with case_when() #2094

I consider this an internal "performance optimization". An alternative way to do this is to combine the list of xs with default and default_arg as something like list2(!!!xs, default_arg := default) but this is way too expensive for something like list_combine(). The point of this helper is to avoid requiring that.

@DavisVaughan DavisVaughan changed the title Always finalize vec_ptype_common(.ptype =), fix ptype finalization in list_combine() Fix list_combine()'s ptype finalization when default is involved Nov 12, 2025
@DavisVaughan DavisVaughan requested a review from lionel- November 12, 2025 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vec_case_when() type issue with NA and default = <character>

2 participants