Bug in out-of-bounds assignment of list object to expression() vector

When assigning a list to an out of bounds index (ex: the next, n+1
index), it errors the same but now changes the values of the vector
to NULL:

```
x <- expression(a,b,c)
x[[4]] <- list() # Error
x
#> expression(NULL, NULL, NULL)  
```

Curiously, this behavior disappears if a prior attempt is made at
assigning to the same index, using a different incompatible object
that does not share this bug (like a function)

There seems to be a bug in out-of-bounds assignment of list objects to an
expression() vector. Tested on release and devel. (Many thanks to folks
over at Mastodon for the help narrowing down this bug)

When assigning a list into an existing index, it correctly errors on
incompatible type, and the expression vector is unchanged:

```
x <- expression(a,b,c)
x[[3]] <- list() # Error
x
#> expression(a, b, c)
```

When assigning a list to an out of bounds index (ex: the next, n+1 index),
it errors the same but now changes the values of the vector to NULL:

```
x <- expression(a,b,c)
x[[4]] <- list() # Error
x
#> expression(NULL, NULL, NULL)
```

Curiously, this behavior disappears if a prior attempt is made at assigning
to the same index, using a different incompatible object that does not
share this bug (like a function):

```
x <- expression(a,b,c)
x[[4]] <- base::sum # Error
x[[4]] <- list() # Error
x
#> expression(a, b, c)
```

That "protection" persists until x[[4]] is evaluated, at which point the
bug can be produced again:

```
x[[4]] # Error
x[[4]] <- list() # Error
x
#> expression(NULL, NULL, NULL)
```

Note that `x` has remained a 3-length vector throughout.

Best,
June

	[[alternative HTML version deleted]]

On Fri, 5 Apr 2024 08:15:20 -0400
June Choe <jchoe001 at gmail.com> wrote:

When assigning a list to an out of bounds index (ex: the next, n+1
index), it errors the same but now changes the values of the vector
to NULL:

```
x <- expression(a,b,c)
x[[4]] <- list() # Error
x
#> expression(NULL, NULL, NULL)
```

Curiously, this behavior disappears if a prior attempt is made at
assigning to the same index, using a different incompatible object
that does not share this bug (like a function)

Here's how the problem happens:

1. The call lands in src/main/subassign.c, do_subassign2_dflt().

2. do_subassign2_dflt() calls SubassignTypeFix() to prepare the operand
for the assignment.

3. Since the assignment is "stretching", SubassignTypeFix() calls
EnlargeVector() to provide the space for the assignment.

The bug relies on `x` not being IS_GROWABLE(), which may explain
why a plain x[[4]] <- list() sometimes doesn't fail.

The future assignment result `x` is now expression(a, b, c, NULL), and
the old `x` set to expression(NULL, NULL, NULL) by SET_VECTOR_ELT(newx,
i, VECTOR_ELT(x, i)); CLEAR_VECTOR_ELT(x, i); during EnlargeVector().

4. But then the assignment fails, raising the error back in
do_subassign2_dflt(), because the assignment kind is invalid: there is
no way to put data.frames into an expression vector. The new resized
`x` is lost, and the old overwritten `x` stays there.

Not sure what the right way to fix this is. It's desirable to avoid
shallow_duplicate(x) for the overwriting assignments, but then the
sub-assignment must either succeed or leave the operand untouched.
Is there a way to perform the type check before overwriting the operand?

On Fri, 5 Apr 2024, Ivan Krylov via R-devel wrote:

On Fri, 5 Apr 2024 08:15:20 -0400
June Choe <jchoe001 at gmail.com> wrote:

When assigning a list to an out of bounds index (ex: the next, n+1
index), it errors the same but now changes the values of the vector
to NULL:

```
x <- expression(a,b,c)
x[[4]] <- list() # Error
x
#> expression(NULL, NULL, NULL)
```

Curiously, this behavior disappears if a prior attempt is made at
assigning to the same index, using a different incompatible object
that does not share this bug (like a function)

Here's how the problem happens:

1. The call lands in src/main/subassign.c, do_subassign2_dflt().

2. do_subassign2_dflt() calls SubassignTypeFix() to prepare the operand
for the assignment.

3. Since the assignment is "stretching", SubassignTypeFix() calls
EnlargeVector() to provide the space for the assignment.

The bug relies on `x` not being IS_GROWABLE(), which may explain
why a plain x[[4]] <- list() sometimes doesn't fail.

The future assignment result `x` is now expression(a, b, c, NULL), and
the old `x` set to expression(NULL, NULL, NULL) by SET_VECTOR_ELT(newx,
i, VECTOR_ELT(x, i)); CLEAR_VECTOR_ELT(x, i); during EnlargeVector().

4. But then the assignment fails, raising the error back in
do_subassign2_dflt(), because the assignment kind is invalid: there is
no way to put data.frames into an expression vector. The new resized
`x` is lost, and the old overwritten `x` stays there.

Not sure what the right way to fix this is. It's desirable to avoid
shallow_duplicate(x) for the overwriting assignments, but then the
sub-assignment must either succeed or leave the operand untouched.
Is there a way to perform the type check before overwriting the operand?

Yes. There are two places where there are some checks, one early and
the other late. The early one is explicitly letting this one through
and shouldn't. So a one line change would address this particular
problem. But it would be a good idea to review why we the late checks
are needed at all and maybe change that. I'll look into it.

Best,

luke

--
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa                  Phone:             319-335-3386
Department of Statistics and        Fax:               319-335-3017
    Actuarial Science
241 Schaeffer Hall                  email:   luke-tierney at uiowa.edu
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu

[You don't often get email from jchoe001 at gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

There seems to be a bug in out-of-bounds assignment of list objects to an
expression() vector. Tested on release and devel. (Many thanks to folks
over at Mastodon for the help narrowing down this bug)

When assigning a list into an existing index, it correctly errors on
incompatible type, and the expression vector is unchanged:

```
x <- expression(a,b,c)
x[[3]] <- list() # Error
x
#> expression(a, b, c)
```

When assigning a list to an out of bounds index (ex: the next, n+1 index),
it errors the same but now changes the values of the vector to NULL:

```
x <- expression(a,b,c)
x[[4]] <- list() # Error
x
#> expression(NULL, NULL, NULL)
```

Curiously, this behavior disappears if a prior attempt is made at assigning
to the same index, using a different incompatible object that does not
share this bug (like a function):

```
x <- expression(a,b,c)
x[[4]] <- base::sum # Error
x[[4]] <- list() # Error
x
#> expression(a, b, c)
```

That "protection" persists until x[[4]] is evaluated, at which point the
bug can be produced again:

```
x[[4]] # Error
x[[4]] <- list() # Error
x
#> expression(NULL, NULL, NULL)
```

Note that `x` has remained a 3-length vector throughout.

Best,
June

       [[alternative HTML version deleted]]