Re: [PATCH] devel: fix segfault by unset 'assoc[${x[0]}]'

[Koichi Murase]

> I'd like the default behavior to be closer to what it is when
> assoc_expand_once is enabled, as I said back in March. I think it's
> going to be better for users in the long run.

Does that mean the behavior with `assoc_expand_once' being disabled
also modified in a way incompatible with older Bash versions?  I have
been thinking that `shopt -s assoc_expand_once' would be the default
in the future keeping the behavior of `shopt -u assoc_expand_once'.

If the behavior of `shopt -u assoc_expand_once' would also be
modified, I would like to request another switch for the
backward-compatible behavior, in particular, a specific shopt switch
(but not a setting something like `BASH_COMPAT=51' which would involve
other behavior changes). Anyway, we need to maintain the code of the
backward-compatible behavior.

> Only that I'd like a more comprehensive behavioral change. Your fix
> is fine for the limited scope it tackles (resolving the discrepancy
> between valid_array_reference and unbind_array_element).

I see.

> > Yes, but I feel like this is another design issue which is irrelevant
> > for the fix of the present small problem.
>
> Sure, but that's why we're talking through the issue. Your fix is
> fine for the problem it intends to solve, now I'd like to go beyond
> it and figure out a better architectural solution.

I see.  In order to make such architectural changes, I feel we first
need to determine how the behavior should be changed.  I guess such a
discussion would be again as long as the one in March.  Maybe this
would become just a repetition of the discussion in March, but to
summarize,

* I still feel that the cleanest solution is to introduce a special
  the syntax-level rule for `unset arr[...]' where the part `arr[...]'
  is treated as if the right-hand side of a variable assignment (just
  like in other assignment builtins such as `declare', `local',
  `export', etc.), i.e., pathname expansions and word splitting is not
  performed on the arguments of the form `name[xxx]'.

  This might be also useful to distinguish the all-element unset «
  unset a[@] » from the unset of the element of key='@' « unset a['@']
  ».

  But the problem might be that this may require non-trivial changes
  to the existing codebase.

* I would like to request a backward-compatible mode where the extra
  expansions of array subscripts are performed the same as the older
  versions of Bash.  I would like to see a specific option for this
  mode rather than `BASH_COMPAT=51' which would involve other
  behavioral changes.

* I feel we need to care about the consistency with the extra
  expansions performed in other contexts:

  - printf -v 'a[$key]'
  - read 'a[$key]'
  - declare 'a[$key]=1'
  - vref='a[$key]'; echo "${!vref}"
  - declare -n nref='a[$key]'
  - etc.

> a better architectural solution.

This will not change the observable behavior, but if I would refactor
it, I'd make `valid_array_referecen' return the extracted subscript
and let `unbind_array_element' just receive the extracted subscript
rather than make `unbind_array_element' again extract the subscript.
I attach a patch `r0029-0002b-refactor-unset.patch' to illustrate this
strategy.  This patch bases on the current devel branch.  This patch
gives an alternative solution for the following patches (sorry if you
have already applied some of them):

- `0002-allow-nesting-and-quoting-in-assoc-subscripts-when-a.patch' in
  https://lists.gnu.org/archive/html/bug-bash/2021-10/msg00051.html
- `0001-arrayfunc.c-unset_array_element-fix-a-bug-that-the-f.patch' in
  https://lists.gnu.org/archive/html/bug-bash/2021-10/msg00059.html

--
Koichi

r0029-0002b-refactor-unset.patch
Description: Binary data