Skip to content

Commit

Permalink
Fix u[42]=99 semantics when u is undefined.
Browse files Browse the repository at this point in the history
It should magically turn into an array in that case.  We need to be
compatible with bash/mksh to run both 'bash_completion' and
'git-completion.bash'.

- You can opt in to the more sane behavior (an error) with set -o
  strict-array.
- Add spec tests.

This reverts the last commit (4ff4f25).
  • Loading branch information
Andy Chu committed Sep 22, 2018
1 parent 4ff4f25 commit 8781632
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 68 deletions.
15 changes: 5 additions & 10 deletions core/cmd_exec.py
Original file line number Diff line number Diff line change
Expand Up @@ -769,20 +769,15 @@ def _Dispatch(self, node, fork_external):
if pair.rhs:
val = self.word_ev.EvalRhsWord(pair.rhs)
assert isinstance(val, runtime.value), val
else:
# e.g. 'readonly x' or 'local x'
if var_flags_e.Array in flags:
val = runtime.StrArray([])
elif var_flags_e.AssocArray in flags:
# TODO: AssocArray
val = runtime.StrArray([])
else:
val = None # 'export a' -- only changing flags, may exist

else: # e.g. 'readonly x' or 'local x'
val = None

# NOTE: In bash and mksh, declare -a myarray makes an empty cell with
# Undef value, but the 'array' attribute.

self.mem.SetVar(lval, val, flags, lookup_mode)
self.mem.SetVar(lval, val, flags, lookup_mode,
strict_array=self.exec_opts.strict_array)

# Assignment always appears to have a spid.
if node.spids:
Expand Down
128 changes: 71 additions & 57 deletions core/state.py
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ def _FindCellAndNamespace(self, name, lookup_mode, writing=True):
else:
raise AssertionError(lookup_mode)

def SetVar(self, lval, value, new_flags, lookup_mode):
def SetVar(self, lval, value, new_flags, lookup_mode, strict_array=False):
"""
Args:
lval: lvalue
Expand Down Expand Up @@ -763,6 +763,7 @@ def SetVar(self, lval, value, new_flags, lookup_mode):
cell.readonly = True
else:
if value is None:
# set -o nounset; local foo; echo $foo # It's still undefined!
value = runtime.Undef() # export foo, readonly foo
cell = runtime.cell(value,
var_flags_e.Exported in new_flags ,
Expand All @@ -787,66 +788,79 @@ def SetVar(self, lval, value, new_flags, lookup_mode):
e_die("Can't assign array to array member", span_id=left_spid)

cell, namespace = self._FindCellAndNamespace(lval.name, lookup_mode)
if cell:
if cell.val.tag != value_e.StrArray:
# s=x
# s[1]=y # invalid
e_die("Entries in value of type %s can't be assigned to",
cell.val.__class__.__name__, span_id=left_spid)
if not cell:
self._BindNewArrayWithEntry(namespace, lval, value, new_flags)
return

# bash/mksh have annoying behavior of letting you do LHS assignment to
# Undef, which then turns into an array. (Undef means that set -o
# nounset fails.)
if (cell.val.tag == value_e.Str or
(cell.val.tag == value_e.Undef and strict_array)):
# s=x
# s[1]=y # invalid
e_die("Entries in value of type %s can't be assigned to",
cell.val.__class__.__name__, span_id=left_spid)

if cell.readonly:
e_die("Can't assign to readonly value", span_id=left_spid)

strs = cell.val.strs
try:
strs[lval.index] = value.s
except IndexError:
# Fill it in with None. It could look like this:
# ['1', 2, 3, None, None, '4', None]
# Then ${#a[@]} counts the entries that are not None.
#
# TODO: strict-array for Oil arrays won't auto-fill.
n = lval.index - len(strs) + 1
strs.extend([None] * n)
strs[lval.index] = value.s
else:
# When the array doesn't exist yet, it is created filled with None.
# Access to the array needs to explicitly filter those sentinel values.
# It also wastes memory. But indexed access is fast.

# What should be optimized for? Bash uses a linked list. Random access
# takes linear time, but iteration skips unset entries automatically.

# - Maybe represent as hash table? Then it's not an ASDL type?

# representations:
# - array_item.Str array_item.Undef
# - parallel array: val.strs, val.undefs
# - or change ASDL type checking
# - ASDL language does not allow: StrArray(string?* strs)
# - or add dict to ASDL? Didn't it support obj?
# - finding the max index is linear time?
# - also you have to sort the indices
if cell.readonly:
e_die("Can't assign to readonly value", span_id=left_spid)

if cell.val.tag == value_e.Undef:
self._BindNewArrayWithEntry(namespace, lval, value, new_flags)
return

strs = cell.val.strs
try:
strs[lval.index] = value.s
except IndexError:
# Fill it in with None. It could look like this:
# ['1', 2, 3, None, None, '4', None]
# Then ${#a[@]} counts the entries that are not None.
#
# array ops:
# a=(1 2)
# a[1]=x
# a+=(1 2)
# ${a[@]} - get all
# ${#a[@]} - length
# ${!a[@]} - keys
# That seems pretty minimal.

items = [None] * lval.index
items.append(value.s)
new_value = runtime.StrArray(items)
# arrays can't be exported
cell = runtime.cell(new_value, False,
var_flags_e.ReadOnly in new_flags)
namespace[lval.name] = cell
# TODO: strict-array for Oil arrays won't auto-fill.
n = lval.index - len(strs) + 1
strs.extend([None] * n)
strs[lval.index] = value.s

else:
raise AssertionError
raise AssertionError(lval.__class__.__name__)

def _BindNewArrayWithEntry(self, namespace, lval, value, new_flags):
# When the array doesn't exist yet, it is created filled with None.
# Access to the array needs to explicitly filter those sentinel values.
# It also wastes memory. But indexed access is fast.

# What should be optimized for? Bash uses a linked list. Random access
# takes linear time, but iteration skips unset entries automatically.

# - Maybe represent as hash table? Then it's not an ASDL type?

# representations:
# - array_item.Str array_item.Undef
# - parallel array: val.strs, val.undefs
# - or change ASDL type checking
# - ASDL language does not allow: StrArray(string?* strs)
# - or add dict to ASDL? Didn't it support obj?
# - finding the max index is linear time?
# - also you have to sort the indices
#
# array ops:
# a=(1 2)
# a[1]=x
# a+=(1 2)
# ${a[@]} - get all
# ${#a[@]} - length
# ${!a[@]} - keys
# That seems pretty minimal.

items = [None] * lval.index
items.append(value.s)
new_value = runtime.StrArray(items)
# arrays can't be exported
cell = runtime.cell(new_value, False,
var_flags_e.ReadOnly in new_flags)
namespace[lval.name] = cell


def InternalSetGlobal(self, name, new_val):
"""For setting read-only globals internally.
Expand Down
50 changes: 49 additions & 1 deletion spec/assign.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -407,11 +407,14 @@ None
#### syntax error in array assignment
a=x b[0+]=y c=z
echo $a $b $c
# status: 2
## status: 2
## stdout-json: ""
## BUG bash stdout: x
## BUG bash status: 0
## OK mksh stdout-json: ""
## OK mksh status: 1
## N-I dash stdout:
## N-I dash status: 0

#### dynamic local variables
f() {
Expand All @@ -430,3 +433,48 @@ f 'x=y a=b'
[y]
[b]
## END

#### 'local x' does not set variable
set -o nounset
f() {
local x
echo $x
}
f
## status: 1
## OK dash status: 2

#### 'local -a x' does not set variable
set -o nounset
f() {
local -a x
echo $x
}
f
## status: 1
## OK dash status: 2

#### 'local x' and then array assignment
f() {
local x
x[3]=foo
echo ${x[3]}
}
f
## status: 0
## stdout: foo
## N-I dash status: 2
## N-I dash stdout-json: ""

#### 'declare -A' and then dict assignment
set -o strict-arith
declare -A foo
key=bar
foo["$key"]=value
echo ${foo["bar"]}
## status: 0
## stdout: value
## N-I dash status: 2
## N-I dash stdout-json: ""
## N-I mksh status: 1
## N-I mksh stdout-json: ""
10 changes: 10 additions & 0 deletions test/runtime-errors.sh
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,16 @@ strict_array_3() {
local foo=${1:- "[$@]" }
}

strict_array_4() {
local -a x
x[42]=99
echo "x[42] = ${x[42]}"

set -o strict-array
local -a y
y[42]=99
}

array_assign_1() {
s=1
s[0]=x # can't assign value
Expand Down

0 comments on commit 8781632

Please sign in to comment.